feat(deployment): add view for new mcms state #16290
Conversation
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
gustavogama-cll
force-pushed
the
ggama/feat-add-view-for-new-mcms-state
branch
2 times, most recently
from
93ac134 to
4b0609b
Compare
gustavogama-cll
requested review from
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang and
0xnogo
gustavogama-cll
force-pushed
the
ggama/feat-add-view-for-new-mcms-state
branch
from
4b0609b to
977b46f
Compare
|
sorry @ecPablo , I had to push an update removing an old, commented out block of code in the tests. |
|
| @@ -19,7 +20,7 @@ func DeployMCMSWithTimelockProgramsSolana( | |||
| config commontypes.MCMSWithTimelockConfigV2, | |||
| ) (*state.MCMSWithTimelockStateSolana, error) { | |||
| addresses, err := e.ExistingAddresses.AddressesForChain(chain.Selector) | |||
| if err != nil { | |||
| if err != nil && !errors.Is(err, deployment.ErrChainNotFound) { | |||
There was a problem hiding this comment.
if the chain is not found, is it still worth continuing in this function? Wouldnt MaybeLoadMCMSWithTimelockChainStateSolana fail too?
There was a problem hiding this comment.
no, I don't think so. Our integration tests pass at the moment.
What this is doing in practice is accepting an empty address book. I believe in other places we require the address book to have an entry with the chain selector (pointing to an empty list of addresses). But I honestly don't see why we require that and added this workaround (which, tbh, was copied from one of the ccip changesets).
If you think this may break something, please let me know and I'll drop it.
There was a problem hiding this comment.
I dont know enough here too, but just tracing the code, if chain is not found, address will be nil and this will fail here when it tries to load the state?
There was a problem hiding this comment.
I don't think so. The only way we use the addresses variable in AddressesContainsBundle (and MaybeLoadMCMSWithTimelockChainStateSolana) is in a for loop.
There was a problem hiding this comment.
But I'll create an empty addressbook to be on the safe side. Your point is valid.
There was a problem hiding this comment.
i mean the end result is probably the same regardless if that error check is there or not. So we can continue with the change if it is consistent with the other code?
This PR updates the MCMS state view by (1) migrating the existing types and functions to the new MCMS library and (2) adding a view for the MCMS Solana state.