[WIP] RMNHome #1443
[WIP] RMNHome #1443
Conversation
Co-authored-by: Makram <[email protected]>
LCOV of commit
|
|
RyanRHall
force-pushed
the
CCIP-3379-productionize-rmn-remote-and-rmn-home-contracts
branch
from
2804858 to
f3454c4
Compare
| uint64 minObservers; // required to agree on an observation for this source chain | ||
| uint64 chainSelector; // ─────╮ The Source chain selector. | ||
| uint64 minObservers; // ──────╯ Required number of observers to agree on an observation for this source chain. | ||
| uint256 observerNodesBitmap; // ObserverNodesBitmap & (1<<i) == (1<<i) iff nodes[i] is an observer for this source chain. |
There was a problem hiding this comment.
small nit to aid in understanding
| uint256 observerNodesBitmap; // ObserverNodesBitmap & (1<<i) == (1<<i) iff nodes[i] is an observer for this source chain. | |
| uint256 observerNodesBitmap; // ObserverNodesBitmap & (1<<i) == (1<<i) iff Config.nodes[i] is an observer for this source chain. |
| bytes32 s_latestConfigDigest; | ||
| string public constant override typeAndVersion = "RMNHome 1.6.0-dev"; | ||
|
|
||
| /// @notice The max number of configs that can be active at the same time. |
There was a problem hiding this comment.
Can place the reasoning for 3 under a @dev comment?
There was a problem hiding this comment.
Would be helpful for the auditors too
There was a problem hiding this comment.
3 is not final, still figuring out the number
| /// @notice Revokes past configs, so that only the latest config remains. Call to promote staging to production. If | ||
| /// the latest config that was set through setConfig, was subsequently revoked through revokeConfig, this function | ||
| /// will revoke _all_ configs. |
There was a problem hiding this comment.
Trying to understand what is staging and what is production when the ring buffer size is 3 - is staging s_latestConfigIndex and production s_latestConfigIndex - 1 if s_latestConfigIndex > 0?
There was a problem hiding this comment.
// s_latestConfigIndex == 0
setConfig(...); // v1. s_latestConfigIndex == 0, this is the production config because its the only one
setConfig(...); // v2. s_latestConfigIndex == 1, this is the staging config because there's already a config from before thats not revoked
setConfig(...); // v3. s_latestConfigIndex == 2, this is the staging staging config? only used if staging sucks?
revokeAllConfigsButLatest(); // v1 and v2 revoked, only v3 is valid. There was a problem hiding this comment.
There's no set index for each, it can be whatever you make it. Which is maybe a good and maybe a bad thing. Let's discuss
| /// @dev this is included in the preimage of the digest that RMN nodes sign | ||
| bytes32 constant RMN_V1_6_ANY2EVM_REPORT = keccak256("RMN_V1_6_ANY2EVM_REPORT"); |
There was a problem hiding this comment.
Would it make sense to add a getter for this? That way we don't have to duplicate this constant offchain and nodes can fetch it as part of reading from the remote.
RyanRHall
force-pushed
the
CCIP-3379-productionize-rmn-remote-and-rmn-home-contracts
branch
3 times, most recently
from
3e57295 to
f39440a
Compare
| /// @param offset setting to 0 will put the newest config in the last position of the returned array, setting to 1 | ||
| /// will put the second newest config in the last position, and so on | ||
| /// @param limit len(versionedConfigsWithDigests) <= limit, set to 1 to get just the latest config, | ||
| /// set to CONFIG_RING_BUFFER_SIZE to ensure that all configs are returned |
There was a problem hiding this comment.
Is it just me or does this seem unnecessarily complicated? Why not just return it in a specific order and offchain can re-order if needed?
There was a problem hiding this comment.
Might even simplify the code below.
RyanRHall
force-pushed
the
CCIP-3379-productionize-rmn-remote-and-rmn-home-contracts
branch
from
4d52586 to
57da48d
Compare
Make RMNHome production ready