Allow validators to specify an attestationKey with which they sign attestations #1313
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1313 +/- ##
=======================================
Coverage 66.41% 66.41%
=======================================
Files 264 264
Lines 7745 7745
Branches 513 444 -69
=======================================
Hits 5144 5144
- Misses 2500 2502 +2
+ Partials 101 99 -2
Continue to review full report at Codecov.
|
…ssuer in Attestations
| @@ -116,6 +133,9 @@ contract Attestations is IAttestations, Ownable, Initializable, UsingRegistry, R | |||
| mapping(bytes32 => IdentifierState) identifiers; | |||
| mapping(address => Account) accounts; | |||
|
|
|||
| // Mapping of attestationKey addresses to account addresses | |||
| mapping(address => address) keysToAccount; | |||
There was a problem hiding this comment.
Renamed it to authorizedAttestors in line with your comment below, wdyt?
There was a problem hiding this comment.
That name makes me think the mapping will be in the other direction. authorizedBy?
| @@ -51,6 +60,11 @@ contract Attestations is IAttestations, Ownable, Initializable, UsingRegistry, R | |||
| uint256 value | |||
| ); | |||
|
|
|||
| event AccountAuthorizedAttestorSet( | |||
There was a problem hiding this comment.
consider AttestorAuthorized for consistency with https://github.com/celo-org/celo-monorepo/pull/1177/files
| @@ -116,6 +133,9 @@ contract Attestations is IAttestations, Ownable, Initializable, UsingRegistry, R | |||
| mapping(bytes32 => IdentifierState) identifiers; | |||
| mapping(address => Account) accounts; | |||
|
|
|||
| // Mapping of authorizedAttestor addresses to account addresses | |||
| mapping(address => address) authorizedAttestors; | |||
There was a problem hiding this comment.
Consider authorizedBy for consistency with https://github.com/celo-org/celo-monorepo/pull/1177/files
| bytes32 r, | ||
| bytes32 s | ||
| ) | ||
| public |
| * @return The address of the authorized attestor | ||
| */ | ||
| function getAuthorizedAttestor(address account) external view returns (address) { | ||
| return accounts[account].authorizedAttestor; |
There was a problem hiding this comment.
What happens if an account hasn't authorized an attestor? Do we require that they do? If we follow the pattern from lockedGold, this would be named getAttestorFromAccount and would return the account in the event that an attestor was not authorized
* master: Integration deploy, don't overwrite genesis block if upgrading testnet (#1315) Fix broken Discord link (#1317) [Wallet] Add check for $ENVFILE to pre-dev script (#1324) Add GRADLE_OPTS note to SETUP.md (#1311) Allow validators to specify an attestationKey with which they sign attestations (#1313) Add step to install typescript and other minor edits (#1256) [Wallet] Add script to build sdk for env before running yarn dev (#1312) [Wallet] Fix ListView deprecation warning (#1293) Set IN_MEMORY_DISCOVERY_TABLE=true for integration (#1296) Add ability to generate addresses directly from env file in celotool (#1294)
* master: Integration deploy, don't overwrite genesis block if upgrading testnet (#1315) Fix broken Discord link (#1317) [Wallet] Add check for $ENVFILE to pre-dev script (#1324) Add GRADLE_OPTS note to SETUP.md (#1311) Allow validators to specify an attestationKey with which they sign attestations (#1313) Add step to install typescript and other minor edits (#1256) [Wallet] Add script to build sdk for env before running yarn dev (#1312) [Wallet] Fix ListView deprecation warning (#1293) Set IN_MEMORY_DISCOVERY_TABLE=true for integration (#1296) Add ability to generate addresses directly from env file in celotool (#1294)
Description
This PR implements #1079. Previously, validators were expected to sign attestations using their consensus key which is much more important for consensus than for attestations. Thus, they can now specify a separate attestation key. If they don't specify a key, their consensus key is used by default.
This PR also implements #1304 to use the LockedGold account address as the canonical address to reference Validators/Issuers
Tested
Related issues