Update ownership Ops and Changesets #116
Conversation
RodrigoAD
force-pushed
the
update-mcms-cs
branch
3 times, most recently
from
66e3ee1 to
7a7122e
Compare
|
|
||
| mcmsPackageId := state.MCMSPackageID | ||
| // If MCMS is not deployed, deploy it | ||
| if mcmsPackageId == "" { |
There was a problem hiding this comment.
Adding to be backwards compatible with core integration tests, should be removed after those tests are updated
There was a problem hiding this comment.
Why is MCMS deployment going to be removed?
There was a problem hiding this comment.
It doesn't need to, but doesn't makes sense to wait to deploy Sui to start deploying MCMS. MCMS deployments can happen before that
| } | ||
|
|
||
| // init transfer ownership to MCMS | ||
| _, err = cld_ops.ExecuteOperation( |
There was a problem hiding this comment.
Every CCIP sequence inits the ownership transfer to MCMS. The next step is to execute the MCMS accept ownership proposal
| ) | ||
|
|
||
| // Exports every operation available so they can be registered to be used in dynamic changesets | ||
| var AllOperationsCCIP = []cld_ops.Operation[any, any, any]{ |
There was a problem hiding this comment.
Each package will expose their operations in a registry Op collection. This operations can later be dynamically pulled by the MCMS proposal generator sequence
| Results map[ContractType]string // Transaction digests | ||
| } | ||
|
|
||
| var ExecuteOwnershipTransferToMcmsSequence = cld_ops.NewSequence( |
There was a problem hiding this comment.
After the Accept Ownership proposal is executed, a last execute_ownership_transfer call is needed to confirm the transfer. This sequence wraps every package ExecuteOwnershipTransfer operation
| ) | ||
|
|
||
| // Exports every operation available so they can be registered to be used in dynamic changesets | ||
| var AllOperations = func() []cld_ops.Operation[any, any, any] { |
There was a problem hiding this comment.
All operations available in chainlnk-sui will be collected here
| const ( | ||
| // DefaultGasBudget is the default gas budget for transactions | ||
| DefaultGasBudget uint64 = 10_000_000 | ||
| DefaultGasBudget uint64 = 500_000_000 |
There was a problem hiding this comment.
Are we actually increasing this default to 500_000_000? If that's the case we should probably remove the 50_000_000s we have in mcms repo.
There was a problem hiding this comment.
This is probably temporary until a version of MCMS is cut, right?
| @@ -0,0 +1,1300 @@ | |||
| // Code generated - DO NOT EDIT. | |||
There was a problem hiding this comment.
Why this one hasn't been generated before? What changed?
There was a problem hiding this comment.
Just saw the updates to scripts/generate_bindings.sh
There was a problem hiding this comment.
offramp and onramp call encoding need to go through the ownable now. We didn't need ownable before this
|
|
||
| mcmsPackageId := state.MCMSPackageID | ||
| // If MCMS is not deployed, deploy it | ||
| if mcmsPackageId == "" { |
There was a problem hiding this comment.
Why is MCMS deployment going to be removed?
|
|
||
| // VerifyPreconditions implements deployment.ChangeSetV2. | ||
| func (d MCMSExecuteTransferOwnership) VerifyPreconditions(e cldf.Environment, config MCMSExecuteTransferOwnershipInput) error { | ||
| return nil |
There was a problem hiding this comment.
Can we check that at least one of the inputs is true? Just to early return in this case.
RodrigoAD
force-pushed
the
update-mcms-cs
branch
from
7a7122e to
d0873f4
Compare
2 participants
Needs #129 merged