feat(canton): integrate Canton support#613
Conversation
|
- Adapts to latest contract changes - Removes DAML client to use raw pb client
- E2e tests, contract updates
## Desc Resolve targetCids execution time - Added TargetTemplateID field to AdditionalFields - Added ResolveTargetContractID resolver function - Updated `TimelockExecutor.Execute` and `Executor.ExecuteOperation` for auto-resolution
# Conflicts: # go.mod # go.sum # types/chain_selector.go
Resolve go.mod/go.sum conflicts by keeping main dependency updates alongside Canton-specific modules (chainlink-canton, go-daml, dazl-client, chainlink-deployments-framework). Co-authored-by: Cursor <cursoragent@cursor.com>
gustavogama-cll
left a comment
There was a problem hiding this comment.
Submitting a few thoughts after a shallow initial pass because I think there is one request (the last comment) that might require a bigger refactoring.
There was a problem hiding this comment.
I don't see canton changes to the chainwrappers/executors_test.go in this PR, nor in #628.
There was a problem hiding this comment.
this is what I have in mind:
ad7ee04
I don't think we need a separate test case for canton. And this exercises the code paths more thoroughly.
| AddChainMetadata(s.chainSelector, cancellerMetadata). | ||
| SetAction(types.TimelockActionCancel). | ||
| SetDelay(delay). | ||
| SetSalt((*common.Hash)(new(scheduleProposal.Salt()))). |
| } | ||
|
|
||
| return types.TransactionResult{ | ||
| Hash: "tx.Digest", |
There was a problem hiding this comment.
or maybe we should use transaction.GetExternalTransactionHash()?
Hash: "0x" + hex.EncodeToString(transaction.GetExternalTransactionHash()),There was a problem hiding this comment.
follow up question: I see the other methods are returning commandID, as mentioned by Copilot. So, if "commandID" makes more sense for Canton than the transaction hash, we should use it (though I'd be curious to understand why "commandID" is the right choice and not the tx hash).
Either way, a hardcoded "tx.Digest" seems wrong.
| case chainsel.FamilyCanton: | ||
| ch, ok := chains.CantonChain(rawSelector) | ||
| if !ok || len(ch.Participants) == 0 { | ||
| return nil, fmt.Errorf("missing Canton chain participant for selector %d", rawSelector) | ||
| } | ||
| participant := ch.Participants[0] | ||
| mcmsParties := lo.Map(ch.Participants, func(p cantonsdk.Participant, _ int) string { return p.PartyID }) | ||
|
|
||
| return cantonsdk.NewTimelockExecutor( | ||
| participant.LedgerServices.Command, | ||
| participant.LedgerServices.State, | ||
| participant.PartyID, | ||
| mcmsParties, | ||
| ), nil |
| func PadLeft32(hexStr string) string { | ||
| if len(hexStr) >= hexWordLen { | ||
| return hexStr[:hexWordLen] | ||
| } | ||
|
|
||
| return strings.Repeat("0", hexWordLen-len(hexStr)) + hexStr | ||
| } |
| // Build exercise command using generated bindings | ||
| mcmsContract := mcmscore.MCMS{} | ||
| exerciseCmd := mcmsContract.SetConfig(mcmsContractID, input) | ||
|
|
||
| // Parse template ID | ||
| packageID, moduleName, entityName, err := parseTemplateIDFromString(mcmsContract.GetTemplateID()) | ||
| if err != nil { | ||
| return types.TransactionResult{}, fmt.Errorf("failed to parse template ID: %w", err) | ||
| } | ||
|
|
||
| // Convert input to choice argument | ||
| choiceArgument := ledger.MapToValue(input) | ||
|
|
||
| commandID := uuid.Must(uuid.NewUUID()).String() | ||
| submitResp, err := c.client.SubmitAndWaitForTransaction(ctx, &apiv2.SubmitAndWaitForTransactionRequest{ | ||
| Commands: &apiv2.Commands{ | ||
| WorkflowId: "mcms-set-config", | ||
| CommandId: commandID, | ||
| ActAs: []string{c.mcmsParties[0]}, | ||
| ReadAs: c.mcmsParties, | ||
| Commands: []*apiv2.Command{{ | ||
| Command: &apiv2.Command_Exercise{ | ||
| Exercise: &apiv2.ExerciseCommand{ | ||
| TemplateId: &apiv2.Identifier{ | ||
| PackageId: packageID, | ||
| ModuleName: moduleName, | ||
| EntityName: entityName, | ||
| }, | ||
| ContractId: exerciseCmd.ContractID, | ||
| Choice: exerciseCmd.Choice, | ||
| ChoiceArgument: choiceArgument, | ||
| }, | ||
| }, | ||
| }}, | ||
| }, | ||
| }) |
| // parseTemplateIDFromString parses a template ID string like "#package:Module:Entity" into its components | ||
| func parseTemplateIDFromString(templateID string) (packageID, moduleName, entityName string, err error) { | ||
| if !strings.HasPrefix(templateID, "#") { | ||
| return "", "", "", fmt.Errorf("template ID must start with #") | ||
| } | ||
| parts := strings.Split(templateID, ":") | ||
| if len(parts) != templateIDPartCount { | ||
| return "", "", "", fmt.Errorf("template ID must have format #package:module:entity, got: %s", templateID) | ||
| } | ||
|
|
||
| return parts[0], parts[1], parts[2], nil | ||
| } |
| commandID := uuid.Must(uuid.NewUUID()).String() | ||
| submitResp, err := c.client.SubmitAndWaitForTransaction(ctx, &apiv2.SubmitAndWaitForTransactionRequest{ | ||
| Commands: &apiv2.Commands{ | ||
| WorkflowId: "mcms-set-config", | ||
| CommandId: commandID, | ||
| ActAs: []string{c.mcmsParties[0]}, | ||
| ReadAs: c.mcmsParties, | ||
| Commands: []*apiv2.Command{{ | ||
| Command: &apiv2.Command_Exercise{ | ||
| Exercise: &apiv2.ExerciseCommand{ | ||
| TemplateId: &apiv2.Identifier{ | ||
| PackageId: packageID, | ||
| ModuleName: moduleName, | ||
| EntityName: entityName, | ||
| }, | ||
| ContractId: exerciseCmd.ContractID, | ||
| Choice: exerciseCmd.Choice, | ||
| ChoiceArgument: choiceArgument, | ||
| }, | ||
| }, | ||
| }}, | ||
| }, | ||
| }) |
| return types.TransactionResult{ | ||
| Hash: "tx.Digest", | ||
| ChainFamily: cselectors.FamilyCanton, | ||
| RawData: rawDataFromMCMSTx(newMCMSContractID, newMCMSTemplateID, submitResp), | ||
| }, nil |
| cancelProposal, err := mcms.NewTimelockProposalBuilder(). | ||
| SetVersion("v1"). | ||
| SetValidUntil(validUntil). | ||
| SetDescription("Canton timelock - cancel scheduled batch"). | ||
| AddTimelockAddress(s.chainSelector, s.mcmsInstanceAddress). | ||
| AddChainMetadata(s.chainSelector, cancellerMetadata). | ||
| SetAction(types.TimelockActionCancel). | ||
| SetDelay(delay). | ||
| SetSalt((*common.Hash)(new(scheduleProposal.Salt()))). | ||
| AddOperation(bop). | ||
| Build() |
| data, err := hex.DecodeString(sb.String()) | ||
| if err != nil { | ||
| panic("HashTimelockOpId: invalid hex encoding: " + err.Error()) | ||
| } |
There was a problem hiding this comment.
I understand the current code paths ensure predecessor and salt are hex, but it's hard to keep track of this type of thing long term. I feels this is making a bad tradeoff just to save ~10 lines of error handling.
| return types.TransactionResult{ | ||
| Hash: "tx.Digest", | ||
| ChainFamily: cselectors.FamilyCanton, | ||
| RawData: rawDataFromMCMSTx(newMCMSContractID, newMCMSTemplateID, submitResp), | ||
| }, nil |
| return "", "", "", fmt.Errorf("template ID must have format #package:module:entity, got: %s", templateID) | ||
| } | ||
|
|
||
| return parts[0], parts[1], parts[2], nil |
| func encodeOperationDataForHash(operationData string) string { | ||
| if isValidHex(operationData) { | ||
| return operationData | ||
| } | ||
|
|
||
| return asciiToHex(operationData) | ||
| } |
| func ValidateAdditionalFields(additionalFields json.RawMessage) error { | ||
| if len(additionalFields) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| var fields AdditionalFields | ||
| if err := json.Unmarshal(additionalFields, &fields); err != nil { | ||
| return fmt.Errorf("failed to unmarshal Canton additional fields: %w", err) | ||
| } | ||
|
|
||
| return fields.Validate() | ||
| } |
| // AdditionalFieldsMetadata represents the Canton-specific metadata fields. | ||
| // MultisigId must be makeMcmsId(instanceId, role) e.g. "mcms-001-proposer" (DAML SetRoot/ExecuteOp). | ||
| // InstanceId is the base MCMS instanceId for self-dispatch TargetInstanceId (DAML E_NOT_SELF_DISPATCH). |
| func PadLeft32(hexStr string) string { | ||
| if len(hexStr) >= hexWordLen { | ||
| return hexStr[:hexWordLen] | ||
| } | ||
|
|
||
| return strings.Repeat("0", hexWordLen-len(hexStr)) + hexStr | ||
| } |
| // Build cancel proposal - reuse the same batch operation (the converter extracts operationId) | ||
| // Use the same salt as the schedule proposal to derive the same operation ID | ||
| cancelProposal, err := mcms.NewTimelockProposalBuilder(). | ||
| SetVersion("v1"). | ||
| SetValidUntil(validUntil). | ||
| SetDescription("Canton timelock - cancel scheduled batch"). | ||
| AddTimelockAddress(s.chainSelector, s.mcmsInstanceAddress). | ||
| AddChainMetadata(s.chainSelector, cancellerMetadata). | ||
| SetAction(types.TimelockActionCancel). | ||
| SetDelay(delay). | ||
| SetSalt((*common.Hash)(new(scheduleProposal.Salt()))). | ||
| AddOperation(bop). | ||
| Build() |
|
gustavogama-cll
left a comment
There was a problem hiding this comment.
Need to take a break and will resume later. As a general comment, I find the test coverage pretty lacking. According to Sonarqube we're at ~9%. We have the e2e tests, which are definitely important but I do feel like there's a lot of missing unit tests which would help us maintain and validate the scenarios that deviate from the happy paths.
| ChainId int64 `json:"chainId"` | ||
| MultisigId string `json:"multisigId"` | ||
| InstanceId string `json:"instanceId,omitempty"` // base instanceId; converter uses for TargetInstanceId in ScheduleBatch etc. | ||
| PreOpCount uint64 `json:"preOpCount"` |
There was a problem hiding this comment.
why are PreOpCount, PostOpCount and OverridePreviousRoot part of "AdditionalFieldsMetadata"? The AdditionalFields attribute should be reserved for data that the mcms clients need to manually add.
PreOpCount seems to be the same as StartingOpCount, OverridePreviousRoot is already available as an attribute of the proposal itself, and PostOpCount can be calculated on the fly when needed using txcount and startingOpCount.
There was a problem hiding this comment.
I spent a few minutes commenting out these fields to check my understanding and see it something would break: b6da3ed
Looks ok to my eyes. Let me know if I'm missing something.
| } | ||
|
|
||
| return types.TransactionResult{ | ||
| Hash: "tx.Digest", |
There was a problem hiding this comment.
or maybe we should use transaction.GetExternalTransactionHash()?
Hash: "0x" + hex.EncodeToString(transaction.GetExternalTransactionHash()),There was a problem hiding this comment.
this is what I have in mind:
ad7ee04
I don't think we need a separate test case for canton. And this exercises the code paths more thoroughly.
| if n < 0 { | ||
| panic("intToHex: negative numbers not supported") | ||
| } | ||
| if n == 0 { |
There was a problem hiding this comment.
nit: fmt.Sprintf("%x", n) returns "0". I don't understand why we need the if n == 0.
|
|
||
| // Build the encoded data with domain separator and length prefix for multisigId | ||
| encoded := metadataLeafDomainSeparator + | ||
| padLeft32(intToHex(int(metadataFields.ChainId))) + |
There was a problem hiding this comment.
since intToHex and and padLeft32 can panic, should we add checks to the metadata validation? I see we do chainId != 0 but not chainId < 0. Maybe we could add similar checks to MultisigId and any other fields which we indirectly pass to functions that can panic.
| ) | ||
|
|
||
| const ( | ||
| MCMSTemplateKey = "MCMS.Main:MCMS" |
There was a problem hiding this comment.
nit: it feels like some of these constants should be defined closer to where they are used. A dedicated constants.go file is ok as well, but placing everything in helpers.go is confusing.
| } | ||
|
|
||
| // ParseTemplateIDFromString is the exported version of parseTemplateIDFromString | ||
| func ParseTemplateIDFromString(templateID string) (packageID, moduleName, entityName string, err error) { |
There was a problem hiding this comment.
why do we need the unexported version?
| } | ||
|
|
||
| // FormatTemplateID is the exported version of formatTemplateID | ||
| func FormatTemplateID(id *apiv2.Identifier) string { |
There was a problem hiding this comment.
same question here: why do we need an unexported version?
| } | ||
|
|
||
| return types.TransactionResult{ | ||
| Hash: "tx.Digest", |
There was a problem hiding this comment.
follow up question: I see the other methods are returning commandID, as mentioned by Copilot. So, if "commandID" makes more sense for Canton than the transaction hash, we should use it (though I'd be curious to understand why "commandID" is the right choice and not the tx hash).
Either way, a hardcoded "tx.Digest" seems wrong.
gustavogama-cll
left a comment
There was a problem hiding this comment.
another quick round, will resume later
| // Parse the root from hex string | ||
| rootStr := string(expiringRoot.Root) | ||
| rootStr = strings.TrimPrefix(rootStr, "0x") | ||
| rootBytes, err := hex.DecodeString(rootStr) | ||
| if err != nil { | ||
| return common.Hash{}, 0, fmt.Errorf("failed to decode root hash: %w", err) | ||
| } | ||
|
|
||
| root := common.BytesToHash(rootBytes) |
There was a problem hiding this comment.
| // Parse the root from hex string | |
| rootStr := string(expiringRoot.Root) | |
| rootStr = strings.TrimPrefix(rootStr, "0x") | |
| rootBytes, err := hex.DecodeString(rootStr) | |
| if err != nil { | |
| return common.Hash{}, 0, fmt.Errorf("failed to decode root hash: %w", err) | |
| } | |
| root := common.BytesToHash(rootBytes) | |
| root := common.HexToHash(string(expiringRoot.Root)) |
| } | ||
|
|
||
| // For Canton, MCMAddress is the InstanceAddress hex (stable across SetRoot/ExecuteOp) | ||
| mcmAddress := contracts.InstanceID(string(mcmsContract.InstanceId)).RawInstanceAddress(mcmsContract.Owner).InstanceAddress().Hex() |
There was a problem hiding this comment.
I'm not sure I understand this. It seems we return a different address than the one passed as input. I see aptos and sui doing the same thing but I do want to flag it because this is counterintuitive to me and a potential source of bugs.
| } | ||
|
|
||
| quorum := uint8(0) | ||
| if i < len(bindConfig.GroupQuorums) { |
There was a problem hiding this comment.
nit: you can drop this condition and simplify the code a bit if you check that len(bindConfig.GroupQuorums) <= maxMCMSGroups before the for.
| // Process in reverse order to build the tree from leaves to root | ||
| for i := maxMCMSGroups - 1; i >= 0; i-- { | ||
| parent := uint8(0) | ||
| if i < len(bindConfig.GroupParents) { |
There was a problem hiding this comment.
same thing here; you can check that len(bindConfig.GroupParents) <= maxMCMSGroups before the for.
| @@ -0,0 +1,200 @@ | |||
| //go:build e2e | |||
There was a problem hiding this comment.
appreciate the unit tests (for a change 😛 ) but why does it need the go:build e2e?
There was a problem hiding this comment.
can't we merge this into the helpers.go file? I don't see a good reason to have two files for generic helpers.
There was a problem hiding this comment.
the chain_metadata.go file might also be a good option since the CantonRoleFromAction function is related to the TimelockRole that is defined there.
| } | ||
|
|
||
| // Extract metadata fields for chainId and multisigId | ||
| var metadataFields struct { |
There was a problem hiding this comment.
don't we have AdditionalFieldsMetadata and parseAdditionalFieldsMetadata already?
| return types.TransactionResult{}, fmt.Errorf("failed to parse template ID: %w", err) | ||
| } | ||
|
|
||
| commandID := uuid.Must(uuid.NewUUID()).String() |
There was a problem hiding this comment.
| commandID := uuid.Must(uuid.NewUUID()).String() | |
| commandID := uuid.NewString() |
(unless you need uuid v1)
| // Convert input to choice argument | ||
| choiceArgument := ledger.MapToValue(input) | ||
|
|
||
| commandID := uuid.Must(uuid.NewUUID()).String() |
There was a problem hiding this comment.
same suggestion:
| commandID := uuid.Must(uuid.NewUUID()).String() | |
| commandID := uuid.NewString() |
| }, nil | ||
| } | ||
|
|
||
| func PadLeft32(hexStr string) string { |
There was a problem hiding this comment.
this isn't used; you already have padLeft32 in encoder.go.
| ChainSelector: bop.ChainSelector, | ||
| Transaction: types.Transaction{ | ||
| To: mcmAddress, | ||
| Data: []byte{0x00}, // placeholder for validators; Canton uses AdditionalFields.OperationData |
There was a problem hiding this comment.
why use OperationData and not Data?
I'm also curious as to why we need to duplicate mcmAddress as TargetCid. I tried to follow the execution flow but it seems TargetCid isn't really used (other than check for "TargetCid == ""` in executor.go).
|
|
||
| // TimelockCallForHash is used for computing the operation ID hash. | ||
| // Field semantics match mcms.TimelockCall (TargetInstanceAddress, FunctionName, OperationData). | ||
| type TimelockCallForHash struct { |
There was a problem hiding this comment.
this can probably be made private
| // HashTimelockOpId computes the operation ID for timelock operations. | ||
| // Matches Canton's hashTimelockOpId: keccak256(encodedCalls || predecessor || salt). | ||
| // predecessor and salt should be hex-encoded (e.g. 64-char hex for 32-byte hashes). | ||
| func HashTimelockOpId(calls []TimelockCallForHash, predecessor, salt string) string { |
| data, err := hex.DecodeString(sb.String()) | ||
| if err != nil { | ||
| panic("HashTimelockOpId: invalid hex encoding: " + err.Error()) | ||
| } |
There was a problem hiding this comment.
I understand the current code paths ensure predecessor and salt are hex, but it's hard to keep track of this type of thing long term. I feels this is making a bad tradeoff just to save ~10 lines of error handling.
| return asciiToHex(operationData) | ||
| } | ||
|
|
||
| func isValidHex(s string) bool { |
There was a problem hiding this comment.
there's some redundancy with IsInstanceAddressHex in resolver.go. Maybe it should call isValidHex.


No description provided.