Tooling deployment scaffolding #1252
Conversation
…tcontractkit/ccip into CCIP-2906-tooling-scaffolding-basic
## Motivation ## Solution
| return "" | ||
| } | ||
|
|
||
| func UBigInt(i uint64) *big.Int { |
There was a problem hiding this comment.
how about moving to a utils/helper ?
There was a problem hiding this comment.
I think in a follow up PR I'll dig around for a better home. Something in chainlink-common or existing tools should exist for this
| return new(big.Int).SetUint64(i) | ||
| } | ||
|
|
||
| func E18Mult(amount uint64) *big.Int { |
There was a problem hiding this comment.
how about moving to a utils/helper ?
| // OnchainPublicKey types.OnchainPublicKey | ||
| // PeerID string | ||
| // TransmitAccount types.Account | ||
| type Node struct { |
There was a problem hiding this comment.
| type Node struct { | |
| type OCRNode struct { |
There was a problem hiding this comment.
today I actually don't think there are non-OCR nodes anymore heh
| type OnchainClient interface { | ||
| // For EVM specifically we can use existing geth interface | ||
| // to abstract chain clients. | ||
| bind.ContractBackend |
There was a problem hiding this comment.
can we also include bind.DeployBackend ?
There was a problem hiding this comment.
idt its needed because Confirm is a separate function pointer which can enclose transactionReceipt?
| Name string | ||
| Chains map[uint64]Chain | ||
| Offchain OffchainClient | ||
| NodeIDs []string |
There was a problem hiding this comment.
Are these NodeIDs specific to JD ?
There was a problem hiding this comment.
right now I'm using the peerIDs to unblock but JD does have its own. Will have to move to theirs at some point or convince them to use peerIDs
|
|
||
| type AddressBook interface { | ||
| Save(chainSelector uint64, address string, typeAndVersion string) error | ||
| Addresses() (map[uint64]map[string]string, error) |
There was a problem hiding this comment.
Having comments on what the keys on this map mean might help. I'm guessing its chainSelector => (typeAndVersion => address) but not 100% sure.
Alternatively a type alias on ChainSelector/Address and maybe a concrete TypeAndVersion type e.g
type (
ChainSelector = uint64
TypeAndVersion struct {
Type, Version string
}
Address = string
)There was a problem hiding this comment.
yeah type alias sgtm. Can add in a follow up with further address book testing
| } | ||
| for chain, chainAddresses := range addresses { | ||
| for address, typeAndVersions := range chainAddresses { | ||
| return m.Save(chain, address, typeAndVersions) |
There was a problem hiding this comment.
Seems like a premature return here, is this intended?
There was a problem hiding this comment.
nope good catch, merge to be tested in a follow up / once we determine its use case a bit better
There was a problem hiding this comment.
added a quick merge test
|
|
||
| import "fmt" | ||
|
|
||
| type AddressBook interface { |
There was a problem hiding this comment.
Doc comment on this would be useful to explain significance and also to indicate whether implementations should be thread-safe
There was a problem hiding this comment.
yeah I think a full polish address book follow up PR needed
| AddressesByChain map[uint64]map[string]string | ||
| } | ||
|
|
||
| func (m *AddressBookMap) Save(chainSelector uint64, address string, typeAndVersion string) error { |
There was a problem hiding this comment.
Should we check for edge cases here e.g address or typeAndVersion being empty strings?
integration-tests/deployment/ccip/migrations/1_initial_deploy_test.go
Outdated
Show resolved
Hide resolved
integration-tests/deployment/ccip/migrations/1_initial_deploy_test.go
Outdated
Show resolved
Hide resolved
integration-tests/deployment/ccip/migrations/1_initial_deploy_test.go
Outdated
Show resolved
Hide resolved
integration-tests/deployment/ccip/migrations/1_initial_deploy_test.go
Outdated
Show resolved
Hide resolved
| m.AddressesByChain[chainSelector] = make(map[string]string) | ||
| } | ||
| if _, exists := m.AddressesByChain[chainSelector][address]; exists { | ||
| return fmt.Errorf("address %s already exists for chain %d", address, chainSelector) |
There was a problem hiding this comment.
It's okay if it has same value not to error, just a log should be enough.
There was a problem hiding this comment.
in this context I think it'd imply caller has a bug so should error
| ) | ||
|
|
||
| type Contracts interface { | ||
| *capabilities_registry.CapabilitiesRegistry | |
There was a problem hiding this comment.
TIL this feature in Go :D
| for node, jb := range js { | ||
| fmt.Println(node, jb) | ||
| } | ||
| // TODO: Add job assertions |
There was a problem hiding this comment.
Maybe later can we have a ticket for each TODO (or group relevant TODO's together)?
| CapabilityVersion: CapabilityVersion, | ||
| CapabilityLabelledName: CapabilityLabelledName, | ||
| OCRKeyBundleIDs: map[string]string{ | ||
| // TODO: Validate that that all EVM chains are using the same keybundle. |
There was a problem hiding this comment.
Offchain OCR key usage for CCIP capability is by definition one per family (this is enforced by the job). That's why we require the bundle IDs to be set explicitly in the job spec in case there is more than one per family
| wg.Add(1) | ||
| go func(src, dest uint64) { | ||
| defer wg.Done() | ||
| waitForCommitWithInterval(t, srcChain, dstChain, state.EvmOffRampsV160[dest], ccipocr3.SeqNumRange{1, 1}) |
There was a problem hiding this comment.
nit: you can just use src and dest here since they're passed in as args to the function - those are eval'd at the go statement callsite unlike closure vars
| wg.Add(1) | ||
| go func(src, dest uint64) { | ||
| defer wg.Done() | ||
| waitForExecWithSeqNr(t, srcChain, dstChain, state.EvmOffRampsV160[dest], 1) |
There was a problem hiding this comment.
Ditto here re: src and dest
| src.Client.(*backends.SimulatedBackend).Commit() | ||
| dest.Client.(*backends.SimulatedBackend).Commit() |
There was a problem hiding this comment.
There a reason we don't have these commits in a background goroutine?
|
- NOTE ~5k is generated protobuf code to unblock. That will be imported once exposed. - We put the deployment/configuration logic in integration-tests module for a few reasons: - Keeps the chain dependencies out of the core module, in particular helpful for eventual cross family tests - It can become the canonical deployment logic to be used for CRIB envs as well (eventually can replace the actions + contracts dirs) - To accomplish the lightweight tests (chainlink.Application + simulated.Backend) we expose some test utilities in util/testutils/ - integration-tests/deployment holds product agnostic deployment utilities including a general purpose environment structure to write environment abstracted code against and migration output components (address books, proposals etc) - integration-tests/deployment/ccip holds all product specific deployment code including - Top level migrations and migration tests where a "migration" is defined to be a function which operates against an environment and outputs a MigrationOutput structure with one or more artifacts (MCMS proposals, job specs). Notably migration tests can apply those outputs to an ephemeral environment to ensure correctness. These migrations are intended for export and use against real environments (testnet/mainnet). - Re-usable product specific components of top level migrations and associated tests Next steps / follow up PRs: - Port testutils export to chainlink repo - Example solana setup - Once cross family validated, start deeper testing and real CCIP use cases --------- Co-authored-by: Adam Hamrick <[email protected]> Co-authored-by: AnieeG <[email protected]>
Next steps / follow up PRs: