-
Notifications
You must be signed in to change notification settings - Fork 5
ton devenv #470
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 devenv #470
Conversation
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.
Pull request overview
This PR introduces a TON development environment implementation with comprehensive CCIP contract deployment and configuration capabilities. The primary focus is adding a new devenv-impl module that integrates TON blockchain support into the chainlink-ccip devenv infrastructure.
Key Changes:
- Created new
devenv-implmodule implementing CCIP16TON interface for TON chain integration - Refactored deployment helpers to use type-safe
Transactionswrapper instead of raw byte slices - Updated function naming for consistency (e.g.,
LoadMCMSOnchainState→LoadMCMSOnChainState)
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| devenv-impl/impl.go | Core implementation of CCIP16TON interface with message sending, event monitoring, and chain operations |
| devenv-impl/utils.go | Helper function to derive node addresses from key bundles |
| devenv-impl/ton.toml | Configuration file defining TON blockchain and EVM test environments |
| devenv-impl/README.md | Documentation for running the devenv implementation |
| deployment/ccip/helpers/utils.go | New Transactions type wrapping serialized messages with type-safe operations |
| deployment/ccip/helpers/execute.go | Updated to use new Transactions type |
| deployment/ccip/operation/*.go | Refactored operations to return *Transactions instead of [][]byte |
| deployment/ccip/sequence/*.go | Updated sequences to use Transactions type and append operations |
| deployment/state/state.go | Renamed functions for consistency and added new loader for MCMS state |
| integration-tests/deployment/ccip/cs_deployer_test.go | New test validating contract deployment and OCR3 configuration via deployer API |
| integration-tests/deployment/mcms/cs_deployer_test.go | New test for MCMS deployment with idempotency verification |
| .github/workflows/test-smoke.yml | New CI workflow for running TON smoke tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| // WaitOneExecEventBySeqNo wait and fetch strictly one ExecutionStateChanged event by sequence number and selector. | ||
| func (m *CCIP16TON) WaitOneExecEventBySeqNo(ctx context.Context, from, to, seq uint64, timeout time.Duration) (any, 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.
Can we somehow structure this infra more and split up the three different components?
- ENV setup/spinup (infrastructure)
- Protocol deployments + configuration (product)
- Protocol interactions + tests (running a scenario)
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.
yeah will give that a shot
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.
Tried splitting into 3 files. offchain, onchain, and test_framework
| return tonChain, nil | ||
| } | ||
|
|
||
| func CreateClient(ctx context.Context, url string) (*ton.APIClient, 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 code is also duplicated here, and probably in few other places: https://github.com/smartcontractkit/chainlink-ton/blob/main/staging-monitor/lib/ton/client.go#L92
@jadepark-dev can you support Terry here and try to figure out a single package in chainlink-ton which exposes an utility fn like this to connect a client, which can be reused in other modules (avoid duplication)?
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.
I can try updating the staging-monitor ref to the deployment utils ref; the staging-monitor module already has a dependency on the deployment module. Whether that's the best place for it to live, we can figure out
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.
Scoped out a follow up ticket here: https://smartcontract-it.atlassian.net/browse/NONEVM-3451
Should live in the root chainlink-ton module so others can import.
| env: | ||
| GH_TOKEN: ${{ steps.setup-github-token.outputs.access-token }} | ||
| # Deployment configuration | ||
| WORKFLOW_FILE: .github/workflows/test_smoke.yml |
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.
Wait, is this workflow TON-specific? I checked it out and has TON-specific steps like build TON contracts and other - why is that, what's the goal/structure here?
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.
We're trying to follow a model proposed by https://docs.google.com/document/d/18ZgE45u7XkSSZgiKsxDKgoWeQSRIzaNPv8WbH-pyxig/edit?tab=t.0#heading=h.1f85iu2b3snb
The re-usable flow is not TON specific and any chain should be able to trigger the devenv flow. The reason we had to add the build for TON contracts is when we're trying to source "local" contracts. The other reason we need a ref to the triggering repo is that the env configs are going to live in that repo (e.g. the ton.toml we have defined for blockchains here) so the test runner needs a way to refer to that config.
The scaffolding has tradeoffs. If we define a re-usable flow in chainlink-ccip, it will end up needing to be refs to the calling repo so we can perform any chain specific setup required. If we don't have a re-usable flow and have each chain's repo implement their own action, we end up duplicating the ccip devenv setup steps
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.
Thanks for the context!
is when we're trying to source "local" contracts
But the local contracts are here, as is other context. Can I use your reusable component here vs. triggering it there? Why this can't be an module we import hook into and run here?
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.
Yes that is certainly possible. I tried to get this working with workflow_call rather than workflow_dispatch earlier in development which would have been the action running here rather than chainlink-ccip. 5d7cb4f
https://github.com/smartcontractkit/chainlink-ton/actions/runs/20666861690/job/59340834271
The issue was that workflow is tightly coupled to other actions in chainlink-ccip so we would either have to duplicate actions (and secrets) across repos, or refactor the workflow to actually be standalone and capable of import. If you'd like, we can investigate it in a followup. It should be possible but will require some rework is all.
|
tests still passing with refactor https://github.com/smartcontractkit/chainlink-ccip/actions/runs/20791922163/job/59715893808 |
huangzhen1997
left a comment
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.
looks good, just needs to fix CI lint errors and update go.md graph (./modgraph > go.md)
| return FundWalletsWithCtx(t.Context(), client, recipients, amounts) | ||
| } | ||
|
|
||
| func FundWalletsNoT(client ton.APIClientWrapped, recipients []*address.Address, amounts []tlb.Coins) 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.
These "NoT" and "WithCtx" aren't idiomatic Go. We should just keep FundWallets that takes context, and you can pass context.Background() for offchain.go.
Then callers use:
- Tests: FundWallets(t.Context(), ...)
- Other: FundWallets(ctx or context.Background(), ...)
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.
Yeah that's fair. I was trying to avoid too big of a blast radius with these changes, but there aren't that many calls it looks like
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.
So it looks like the code in core uses this as is, which is causing the tests to fail. I'm going to revert back to the original change as I really don't want to incur a 3+ repo PR for this
{"ImportPath":"github.com/smartcontractkit/chainlink/deployment/ccip/changeset/testhelpers","Action":"build-output","Output":"../deployment/ccip/changeset/testhelpers/test_environment.go:505:22: cannot use t (variable of type *\"testing\".T) as \"context\".Context value in argument to utils.FundWallets: *\"testing\".T does not implement \"context\".Context (missing method Done)\n"}
|
refactor still passes e2e tests https://github.com/smartcontractkit/chainlink-ccip/actions/runs/20820927683 |
| return nil | ||
| } | ||
|
|
||
| func (m *CCIP16TON) PostDeployContractsForSelector(ctx context.Context, env *deployment.Environment, cls []*simple_node_set.Input, selector uint64, ccipHomeSelector uint64, crAddr string) 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.
But where do we actually deploy? Where do we integrate the product level adapters we implemented in this devenv?
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 neat thing is that because every chain implements the same API, we can share most of the deployment/configuration setup https://github.com/smartcontractkit/chainlink-ccip/blob/0be702ef3ff57ec1fd258e96151c1f5e4bb372fc/devenv/common/implcommon.go#L55. Every impl actually calls this shared function to deploy. The pre- and post- methods are just for things that might be chain specific, like price updates
No description provided.