Extend cldf timelock converter logic for TON #628
Conversation
|
|
👋 huangzhen1997, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR extends the CLDF (Chainlink Deployments Framework) proposal conversion logic to support the TON (The Open Network) blockchain. The changes enable TON chain integration by adding necessary converters, executors, and inspectors while also updating various dependencies.
Key changes:
- Add TON support to MCMS v2 CLI with timelock converter, executor, and inspector implementations
- Introduce
TxOpsstruct to encapsulate TON transaction operation configuration - Update multiple dependencies including Go version, chainlink-ccip, mcms, and tonutils-go
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates Go version to 1.25.3 and upgrades multiple dependencies including TON-related libraries |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Adds TON chain family support with converter, executor, and inspector implementations; refactors variable naming |
| chain/ton/ton_chain.go | Introduces TxOps struct to hold wallet and amount configuration for transactions |
| chain/ton/provider/rpc_provider.go | Initializes TxOps with default transaction amount in RPC provider |
| chain/ton/provider/ctf_provider_test.go | Updates test to verify TxOps instead of Wallet field |
| chain/ton/provider/ctf_provider.go | Initializes TxOps with default amount; fixes wallet method call to use WalletAddress() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
huangzhen1997
changed the title
chain/ton/provider/ctf_provider.go
Outdated
| URL: url, | ||
| TxOps: cldf_ton.TxOps{ | ||
| Wallet: tonWallet, | ||
| Amount: tlb.MustFromTON(defaultTxTONAmount), // default amount for transactions |
There was a problem hiding this comment.
Good for now, too simple to scale - we need to track this item.
We will need to be able to load gas configuration (from somewhere) which will need to tell us how much on average something costs to do (per contract call), and something a CLD user should be able to easily override.
cc @patricios-space @vicentevieytes re: per call gas configuration
There was a problem hiding this comment.
@huangzhen1997 I suggest we drop the TxOps idea for now (from this PR), and just use 3x static defaults for value amounts we need:
- MCMS configurer: 0.25 TON
- MCMS executor: 0.25 TON
- Timelock executor: 0.25 TON
That way we will simplify this integration PR, and work on gas configuration/estimation provider in a follow up as Amount: tlb.MustFromTON(defaultTxTONAmount), // default amount for transactions will not be enough to support our requirements.
We will need to build a more dynamic gas estimator and use it instead of static values.
There was a problem hiding this comment.
Updated the default to be 0.25 for now. Created a ticket for the gas estimator
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Client: nodeClient, | ||
| Wallet: tonWallet, | ||
| WalletAddress: tonWallet.Address(), | ||
| WalletAddress: tonWallet.WalletAddress(), |
There was a problem hiding this comment.
The change from tonWallet.Address() to tonWallet.WalletAddress() lacks test coverage. The existing test in ctf_provider_test.go only checks that WalletAddress is not empty but doesn't verify the correct method is being called or that the address value is as expected.
There was a problem hiding this comment.
@copilot This is not true, it's tested in rpc_provider_test.go line 276
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Client: nodeClient, | ||
| Wallet: tonWallet, | ||
| WalletAddress: tonWallet.Address(), | ||
| WalletAddress: tonWallet.WalletAddress(), |
There was a problem hiding this comment.
The code previously called tonWallet.Address() and now calls tonWallet.WalletAddress(). If these methods return different values or have different behaviors, this could break existing functionality. Verify that WalletAddress() returns the same address format as the previous Address() method.
| WalletAddress: tonWallet.WalletAddress(), | |
| WalletAddress: tonWallet.Address(), |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
huangzhen1997
force-pushed
the
NONEVM-3069/extend-cldf-proposal-logic-support-ton-timelock-converter
branch
from
d3318f4 to
120f23b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
4 participants
Jira.
Adding TON timelock converter support. Note the TON support for executor override is also added here, but there's another PR for supporting the migration to MCMS-Tools repo: https://github.com/smartcontractkit/mcms-tools/pull/261
Need to add test coverage