-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Ton analyzer #629
Changes from all 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
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, | ||||||||
| }, | ||||||||
| { | ||||||||
| 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 | ||||||||
|
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.
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 |
krebernisak marked this conversation as resolved.
Show resolved
Hide resolved
| 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.
Show resolved
Hide 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.
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.
nit: Can't this be simplified as
wantErr: stringwhich if set is the expected error string?