-
Notifications
You must be signed in to change notification settings - Fork 3
Ton analyzer #629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ton analyzer #629
Changes from 29 commits
e0f9aab
a51337a
09fc3c2
fe17bf1
a7aa1a2
d6f8113
d33bd19
751007d
c6e01f1
93c385e
a840aea
c56332e
7294c52
ccad05a
6f521dc
933d0b8
0d0ee38
ed21211
19647e5
163c1f0
324486c
a06f6eb
264318f
4d80162
a005805
c3530bf
46fb259
bd55ade
d7cf8ae
c2b5193
b23b872
cb31ba4
4601b1a
1b01990
0e98af6
d69e6a7
082ed38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "chainlink-deployments-framework": minor | ||
| --- | ||
|
|
||
| Adds TON blockchain analyzer support |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -195,33 +195,44 @@ func TestChainNameOrUnknown(t *testing.T) { | |||||||
| require.Equal(t, "<chain unknown>", chainNameOrUnknown(" ")) | ||||||||
| } | ||||||||
|
|
||||||||
| func TestBuildProposalReport_FamilyBranches(t *testing.T) { | ||||||||
| func TestBuildProposalReport_FamilyErrors(t *testing.T) { | ||||||||
| t.Parallel() | ||||||||
|
|
||||||||
| tests := []struct { | ||||||||
| name string | ||||||||
| selector uint64 | ||||||||
| expectedError string | ||||||||
| name string | ||||||||
| selector uint64 | ||||||||
| expectedMsg string | ||||||||
| wantErr bool // if true, expect returned error; if false, error is in Method field (TON behavior) | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "EVM_missing_registry", | ||||||||
| selector: chainsel.ETHEREUM_TESTNET_SEPOLIA.Selector, | ||||||||
| expectedError: "EVM registry is not available", | ||||||||
| name: "EVM_missing_registry", | ||||||||
| selector: chainsel.ETHEREUM_TESTNET_SEPOLIA.Selector, | ||||||||
| expectedMsg: "EVM registry is not available", | ||||||||
| wantErr: true, | ||||||||
|
krebernisak marked this conversation as resolved.
Outdated
|
||||||||
| }, | ||||||||
| { | ||||||||
| name: "Solana_missing_registry", | ||||||||
| selector: chainsel.SOLANA_DEVNET.Selector, | ||||||||
| expectedError: "failed to analyze solana transaction 0: solana decoder registry is not available", | ||||||||
| name: "Solana_missing_registry", | ||||||||
| selector: chainsel.SOLANA_DEVNET.Selector, | ||||||||
| expectedMsg: "failed to analyze solana transaction 0: solana decoder registry is not available", | ||||||||
| wantErr: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Aptos_unmarshal_additional_fields", | ||||||||
| selector: chainsel.APTOS_TESTNET.Selector, | ||||||||
| expectedError: "failed to unmarshal Aptos additional fields: unexpected end of JSON input", | ||||||||
| name: "Aptos_unmarshal_additional_fields", | ||||||||
| selector: chainsel.APTOS_TESTNET.Selector, | ||||||||
| expectedMsg: "failed to unmarshal Aptos additional fields: unexpected end of JSON input", | ||||||||
| wantErr: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Sui_unmarshal_additional_fields", | ||||||||
| selector: chainsel.SUI_TESTNET.Selector, | ||||||||
| expectedError: "failed to unmarshal Sui additional fields: unexpected end of JSON input", | ||||||||
| name: "Sui_unmarshal_additional_fields", | ||||||||
| selector: chainsel.SUI_TESTNET.Selector, | ||||||||
| expectedMsg: "failed to unmarshal Sui additional fields: unexpected end of JSON input", | ||||||||
| wantErr: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "TON_decode_failure", | ||||||||
| selector: chainsel.TON_TESTNET.Selector, | ||||||||
| expectedMsg: "failed to decode TON transaction", | ||||||||
| wantErr: false, // TON doesn't unmarshal AdditionalFields, so decode errors go to Method field | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't diverge from other test cases
|
||||||||
| wantErr: false, // TON doesn't unmarshal AdditionalFields, so decode errors go to Method field | |
| wantErr: false, // TON returns decode errors in Method field to avoid blocking proposal processing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure why is here a difference for TON? There shouldn't be one...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't SUI impl also surfaces errors via .Method member:
| Method: errStr.Error(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is for default unhappy pass, where the analyze function suppose to fail immediately. Unlike SUI we don't need extra field Unmarshal, so the first error will be hiding in Method field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but what's the reason for diverge in this and other tests?
Where other analyzers fail, TON doesn't - why do we need to catch this error specifically for TON?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TON analyzer diverges from other analyzers because it reaches the decode stage, whereas EVM and Solana fail earlier due to missing registry, and Aptos and
Sui fail during additional field decoding. TON handles decode failures gracefully by placing the error in the method field rather than returning an error.
For better readability and more context there, I split the TON case into a separate test with comments explaining this distinction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! Thanks for the info.
Still think we should follow up on this: (1) explicitly add error to DecodeCall (vs. using Method), (2) think about unifying other integrations to follow the same approach
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as Comment 4 - the comment is misleading about why decode errors go to the Method field. The reason is to avoid blocking proposal processing, not specifically about AdditionalFields unmarshaling.
| wantErr: false, // TON doesn't unmarshal AdditionalFields, so decode errors go to Method field | |
| wantErr: false, // decode errors are reported in the Method field so proposal processing is not blocked |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package analyzer | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/smartcontractkit/chainlink-ton/pkg/bindings" | ||
| "github.com/smartcontractkit/mcms/sdk" | ||
| "github.com/smartcontractkit/mcms/sdk/ton" | ||
| "github.com/smartcontractkit/mcms/types" | ||
| ) | ||
|
|
||
| // AnalyzeTONTransactions decodes a slice of TON transactions and returns their decoded representations. | ||
| func AnalyzeTONTransactions(ctx ProposalContext, txs []types.Transaction) ([]*DecodedCall, error) { | ||
| decoder := ton.NewDecoder(bindings.Registry) | ||
| decodedTxs := make([]*DecodedCall, len(txs)) | ||
| for i, op := range txs { | ||
| analyzedTransaction, err := AnalyzeTONTransaction(ctx, decoder, op) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to analyze TON transaction %d: %w", i, err) | ||
| } | ||
| decodedTxs[i] = analyzedTransaction | ||
| } | ||
|
|
||
| return decodedTxs, nil | ||
| } | ||
|
|
||
| // AnalyzeTONTransaction decodes a single TON transaction using the MCMS TON decoder. | ||
| // | ||
| // Unlike Aptos/Sui analyzers, this function does not unmarshal AdditionalFields because | ||
| // the TON decoder only requires tx.Data (BOC cell) and tx.ContractType (metadata). | ||
| // AdditionalFields in TON is only used by the encoder/timelock_converter for the Value field. | ||
| // | ||
| // On decode failure, this function returns a DecodedCall with the error in the Method field | ||
| // instead of returning an error. This allows the proposal to continue processing even if | ||
| // a single transaction fails to decode. | ||
| func AnalyzeTONTransaction(_ ProposalContext, decoder sdk.Decoder, mcmsTx types.Transaction) (*DecodedCall, error) { | ||
| decodedOp, err := decoder.Decode(mcmsTx, mcmsTx.ContractType) | ||
| if err != nil { | ||
| // Don't return an error to not block the whole proposal decoding because of a single transaction decode failure. | ||
| // Instead, put the error message in the Method field so it's visible in the report. | ||
| errStr := fmt.Errorf("failed to decode TON transaction: %w", err) | ||
|
|
||
| return &DecodedCall{ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we should double check this with security. From one side I agree that a failure to decode should not be a blocker for operational activity, but on the other side I know security wants to push for us to reduce blind signing of proposals as much as possible. Will it be common to see decode of operations failing? |
||
| Address: mcmsTx.To, | ||
| Method: errStr.Error(), | ||
|
huangzhen1997 marked this conversation as resolved.
|
||
| }, nil | ||
| } | ||
|
|
||
| namedArgs, err := toNamedFields(decodedOp) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to convert decoded operation to named arguments: %w", err) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we do the same thing of not returning an error and instead putting it on the Decoded call for the same reasons as the comment above?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| return &DecodedCall{ | ||
| Address: mcmsTx.To, | ||
| Method: decodedOp.MethodName(), | ||
| Inputs: namedArgs, | ||
| Outputs: []NamedField{}, | ||
| }, nil | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.