RMNHome #1469
RMNHome #1469
Conversation
RensR
requested review from
makramkd,
elatoskinas,
RayXpub and
a team
as code owners
LCOV of commit
|
RensR
force-pushed
the
rmn-home-final
branch
from
98b61fa
to
d8e1df6
Compare
| /// in case one of the configs is too large to be returnable by one of the other getters. | ||
| /// @param configDigest The digest of the config to fetch. | ||
| /// @return versionedConfig The config and its version. | ||
| /// @return ok True if the config was found, false otherwise. |
There was a problem hiding this comment.
Just wondering if this is needed, won't configDigest on the VersionedConfig be ZERO_DIGEST which is always an invalid digest and hence config does not exist?
There was a problem hiding this comment.
That is true, but using the bool is more explicit. It's not strictly needed but I kinda like it.
| (PREFIX & PREFIX_MASK) | ||
| | ( | ||
| uint256( | ||
| keccak256(bytes.concat(abi.encode(bytes32("EVM"), block.chainid, address(this), version), staticConfig)) |
There was a problem hiding this comment.
Why is staticConfig bytes.concat'd and not abi.encode'd?
There was a problem hiding this comment.
Both components are already abi.encoded. Afaik this should not compromise anything as the first section is all static length anyway
| /// - Init: promoteCandidateAndRevokeActive(), as there is no config to promote. | ||
| /// - Init: revokeCandidate(), as there is no config to revoke | ||
| /// - Active: revokeCandidate(), as there is no candidate to revoke | ||
| /// Note that we explicitly do allow promoteCandidateAndRevokeActive() to be called when there is an active config but |
There was a problem hiding this comment.
I'd maybe add one more note (for completeness), that the other state transitions ActiveAndCandidate->Init, Candidate -> Init are not possible in one step given the transition functions available. Alternatively would be more clear to have a little ascii diagram
There was a problem hiding this comment.
+1 to cute ascii diagram with state transitions.
|
New PR to have this be standalone