Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add deployer preset #467
Add deployer preset #467
Changes from 21 commits
48a9a1e
2eb5299
c2bd445
9dcb55d
705b3fe
e7f1777
c9a6524
0217b62
d2e9d37
10201cb
368a01c
8c7ff36
c411ec1
ea81740
824572f
acb85f2
dd4e1e5
13bdaaa
2a6f095
8632ef0
78c6dc9
63b2a91
ad7992a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to increment ap here with tempvar when a reference is enough:
Actually, the reference shouldn't even be necessary, from my understanding, if you rename
_perdersen
topedersen_ptr
rebinding the reference (because in both branches we should have [ap - 1]). But, for some reason, when doing this, the line emitting the event seems to revoke the reference. This is an expected behavior @bbrandtom ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, this works (as I expect):
While this doesn't (I would expect it to work too):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericnordelo what do you mean by it doesn't work? what exact error are you getting? and i'm not sure the point you're trying to make by removing the event emission
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point is that the event emission is revoking the pedersen_ptr reference in the example I'm presenting when I think it shouldn't. And when you rebind the reference to the same ap reference, it works again (with the event). This looks like a potential compiler issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericnordelo What's the rebinding that works with the event? My guess is that the revocation happens due to an arbitrary change in ap caused by a function call. Usually (after 0.5.0 IIRC), such revocations are solved implicitly by adding a local variable definition prior to the call. However, here the function itself is generated by the compiler (emit is replaced by a code using the emit_event syscall) so it might be missed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ArielElp, the rebinding that works with the event is the one after the
else
block in this example:If you remove that rebinding, the reference is revoked by the
emit
call. My question is, why is it not being revoked when we just rebind the reference from ap to itself? (the same reference to the exact ap location).That rebinding to itself is avoiding the reference revocation in the emit call, and I think it should work without that line too, but is being revoked without it, what am I missing?