-
Notifications
You must be signed in to change notification settings - Fork 0
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
CS6.2 - Event improvements #225
Conversation
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.
LGTM, might want to look into the gas snapshot regression. I don't think it's important. The test with a gas use regression is apparently testClaimsSequentialNonces
, which honestly doesn't even seem like a relevant test anymore based on its name?
// Note: even with a valid submission token, we always set non-replayables to exhausted (e.g. for cancellations) | ||
submissions[msg.sender][nonce] = isReplayable ? submissionToken : EXHAUSTED; | ||
// Note: Even with a valid submission token, we always set non-replayables to exhausted (e.g. for cancellations) | ||
submissionToken = isReplayable ? submissionToken : EXHAUSTED; |
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.
Can you call this a new variable name?
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.
Renamed to finalSubmissionToken
.
87241b0
to
e9691d8
Compare
e9691d8
to
96a33a8
Compare
@@ -19,7 +19,7 @@ contract QuarkNonceManager { | |||
error InvalidNonce(address wallet, bytes32 nonce); | |||
error InvalidSubmissionToken(address wallet, bytes32 nonce, bytes32 submissionToken); | |||
|
|||
event NonceSubmitted(address wallet, bytes32 nonce, bytes32 submissionToken); | |||
event NonceSubmitted(address indexed wallet, bytes32 indexed nonce, bytes32 submissionToken); |
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 also just added an index on nonce
since it could be useful for replayable operations.
Some small improvements to events:
wallet
andnonce
fields in theNonceSubmitted
eventEXHAUSTED
as thesubmissionToken
value in theNonceSubmitted
event