-
Notifications
You must be signed in to change notification settings - Fork 74
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: As a developer, I or portal owner should be able to revoke or replace the attestation #547
Conversation
…is own attestation
*/ | ||
function bulkReplace( | ||
bytes32[] calldata attestationIds, | ||
AttestationPayload[] calldata attestationPayloads, | ||
address attester | ||
address replacer |
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.
Will this introduce breaking changes for existing portals?
*/ | ||
function revoke(bytes32 attestationId) public { | ||
function revoke(bytes32 attestationId, address revoker) public { |
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.
If the revoker
argument is being specified by the contract calling this method, then surely it can be spoofed? Am I wrong?
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.
Actually, the Attestation registry will always be called from the portal that has implemented the abstract portal. The abstract portal has revoke and replace methods that cannot be overridden. The abstract portal has getAttester method which returns msg sender.
As it is recommended not to use tx origin, the best way is to get the attester from the Abstract portal.
I will still have a look if there is any possibility of it.
581c2b6
to
f607329
Compare
Stale |
What does this PR do?
Along with the portal owner, an attester who attested the attestation should be able to revoke or replace his attestation too.
Related ticket
Fixes #546
Type of change
Check list