-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat(protocol): add more events in automata #17195
feat(protocol): add more events in automata #17195
Conversation
feat(protocol): add more events in automata
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
@@ -70,14 +70,22 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract { | |||
pemCertLib = PEMCertChainLib(pemCertLibAddr); | |||
} | |||
|
|||
event SetMrSigner(bytes32); |
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.
please give variabls 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.
please also run pnpm lint:sol
event ConfigQeIdentity(bytes32); | ||
event ToggleLocalReportCheck(bool); | ||
event AddRevokedCertSerialNum(uint256, bytes); | ||
event RemoveRevokedCertSerialNum(uint256, bytes); |
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.
you may also want to add indexed
keyword to certain events.
@@ -92,7 +100,7 @@ contract AutomataDcapV3Attestation is IAttestation, EssentialContract { | |||
continue; | |||
} | |||
serialNumIsRevoked[index][serialNumBatch[i]] = true; | |||
// TODO(yue): emit an event | |||
emit AddRevokedCertSerialNum(index, serialNumBatch[i]); |
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.
how about rename addRevokedCertSerialNum
to revokeCertSerialNum
, then rename event to CertSerialNumRevoked
.
I also suggest to rename other events to xxxAdded, xxxRemoved, xxxConfigured, xxxSet
ee37226
into
suggest_changes_in_AutomataDcapV3Attestation
No description provided.