Repository navigation
Restructure TP flow with onlyRouter access - #850
Conversation
94657e2 to
8d7a746
Compare
Make the Router the single access point of every token pool for inbound release/mint, mirroring the outbound LockOrBurn flow and EVM's _onlyOffRamp. - OffRamp relays OffRamp_ReleaseOrMint to the Router as Router_RelayReleaseOrMint instead of calling the pool directly. The pool's reply (Finished/Failure) still goes straight to the executor via replyTo. - Router authenticates the registered OffRamp for the source lane, enforces its RMN policy, then dispatches TokenPool_ReleaseOrMint to the pool. - On relay rejection (cursed) or a pool bounce, the Router reports a Router_TokenPoolReleaseOrMintFailed to the owning OffRamp, which is the executor's only authority. The OffRamp forwards it to the executor through the existing mintOrReleaseFailed rail. - ReceiveExecutor storage is unchanged (no exclusive-router-read needed). Also fix stale pool unauthorized exit-code expectations (14910 -> 51710).
- Fix stale comment on the OffRamp bounce handler (failure is reported to the OffRamp, not proxied to the executor). - Trim verbose comments on the relay/failure helpers. - Add tests: OffRamp rejects Router_TokenPoolReleaseOrMintFailed from a non-router sender; Router rejects a relay from an unregistered OffRamp and one whose request selector mismatches the source chain.
Mirror the feat/tp-only-router contract changes in the hand-maintained Go bindings and the TON CCIP deployment sequences: - tokenpool: replace MirroredPolicy with LocalPolicy (pool-local RMN cursed-subjects only); ramp access is no longer pool state because the Router is the pool's sole entrypoint (onlyRouter). - tokenpool: model allowedDepositNamespaces as a unit-value set (map<uint32,()>) via *cell.Dictionary, matching the Tolk contract. - tokenpool: drop the removed UpdateRampAccess / RampAccessUpdatesApplied surfaces and the GetOnRamp/GetOffRamp/GetMirroredPolicy readers, and nil-check getRMNProxy's optional address. - Correct four pre-existing opcode drifts (SetDeployableCode, DeployableCodeSet, SetAllowedDepositNamespaces, AllowedDepositNamespacesSet) verified against the Tolk message prefixes. - deployment/1_6_0: stop writing ramp access on the pool; the OffRamp is registered in the Router's offRamps map by the lane's ApplyRampUpdates. - token-pools IMPLEMENTATION doc: describe the Router chokepoint and local RMN policy instead of the mirrored onRamp/offRamp maps.
The MockTokenPool test contract and its generated wrapper were deleted on main in "[NONEVM-5813] Delete mock Token Pool contract (#840)" but came back through the rebase onto feat/tp-only-router. Nothing references them (no test, script, or manifest entry), and the contract merely re-exported the real pool lib. Drop the Tolk contract and the generated wrapper again so the branch does not reintroduce a fixture main intentionally removed.
30e62e1 to
5e44641
Compare
Replace the raw *cell.Dictionary ("dict 32" tag) for the pool's
AllowedDepositNamespaces with *tlbe.Dict[uint32, struct{}] (tlb:"." so it
routes through tlbe.Dict's cell marshaller rather than tonutils-go's hard
*dict N* type assertion).
The value is a unit-value set (map<uint32,()> on-chain). A struct{} value
encodes to an empty, 0-bit inline leaf, so the wire form is byte-identical
to the previous dict 32 encoding (verified by a new round-trip test that
hashes both encodings). This also gives the field explicit set semantics
and tlbe's JSON tooling support.
The nil *cell.Dictionary trick for "empty map" no longer applies on the
"." tag path (a nil *Dict errors), so deployment init data now passes
tlbe.NewEmptyDict[uint32, struct{}]() and the getter decodes via
tlbe.NewDictFromDictionary (nil-safe), matching the deposit Beneficiaries
precedent.
CursedSubjects stays a raw *cell.Dictionary: its values are not unit
(they carry an uint128 timestamp) and it is serialized through the
"dict N" tag path.
Add tlbe.Uint128, a 128-bit unsigned integer that is a comparable value
type (two uint64 words), so it can key a tlbe.Dict directly and compare
by value.
This fixes a class of bug present in Uint160/Uint256 (and any big.Int
alias): they are not comparable, so pointer-keyed dicts (e.g.
Dict[*Uint256, *cell.Cell]) mis-key on pointer identity, and decoding
them via Dict.LoadFromDictionary panics in tonutils' reflection path
because a **T does not satisfy tlb.Unmarshaler when T has a pointer
receiver. Uint128 avoids both: value receiver marshalling and value
equality.
Use it for the pool's CursedSubjects, which is map<uint128, ()>
on-chain: replace the raw *cell.Dictionary ("dict 128") with
*tlbe.Dict[tlbe.Uint128, struct{}] (tlb:"."). A struct{} value encodes
to a 0-bit inline leaf, so the wire form is unchanged (verified by a new
test hashing both encodings). Deployment init now passes an empty,
non-nil dict (the "." tag errors on nil).
Also drops the incorrect "values are not unit" comment, which
contradicted the on-chain map<uint128, ()>.
Uint160/Uint256 were declared as `type Uint160 big.Int`. That is misleading:
the result is a struct containing a slice, so it is NOT comparable and has
none of big.Int's methods. Two consequences:
- They cannot be dictionary keys by value, which is why call sites used
*tlbe.Uint256 / *tlbe.Uint160 pointer keys.
- Pointer keys do not decode. A **T does not satisfy tlb.Unmarshaler when
T.LoadFromCell takes a pointer receiver, so Dict.LoadFromDictionary
panics inside tonutils' reflection path. RemoteChainConfig.RemotePools,
mcms Signers/SeenSignedHashes/Timestamps/OpPendingCalls and rbac Roles
were all affected (latent: none were decoded yet).
Rework all three boxed widths (uint128/uint160/uint256) as comparable value
types backed by fixed-size big-endian byte arrays, sharing one codec
(uintBytes* helpers). Byte storage is required so uint160 is exactly 160
bits (a uint64-word layout would produce 192).
- Uint128/Uint160/Uint256 are now comparable => proper value equality and
usable as tlbe.Dict keys, matching Tolk map<uintN, T>.
- Pointer-keyed dicts switched to value keys; added a RemotePools-shaped
decode regression test.
- Constructors stay pointer-returning (*Uint128 etc.) because the external
mcms v0.52.0 SDK assigns them into pointer fields; the API is unchanged.
- Wire format is unchanged (verified by hashing the boxed encoding against
the previous raw fixed-width big-endian store).
- Uint160/Uint256 gain Cmp; Value() is retained as an alias for ToBigInt().
The dead BigUint helper is left in place for now (no in-repo callers).
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved local curse-policy, Router bounce-handling, sender-attribution, and pending-OffRamp issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR centralizes Token Pool authorization in the Router and updates inbound relay flows, local curse policy, failure handling, bindings, wrappers, deployment, and tests.
Changes:
- Adds Router-authenticated inbound and outbound pool operations.
- Replaces mirrored ramp policy with local pool policy.
- Updates failure propagation, storage bindings, wrappers, and test coverage.
File summaries
| File | Summary |
|---|---|
pkg/ton/tlbe/tlbe.go |
TLB type re-exports |
pkg/ccip/bindings/tokenpool/types.go |
Token Pool bindings |
pkg/ccip/bindings/tokenpool/types_test.go |
Binding serialization tests |
pkg/ccip/bindings/tokenpool/reader.go |
Token Pool readers |
pkg/ccip/bindings/tokenpool/lockreleaselockbox/reader.go |
Lockbox reader updates |
pkg/ccip/bindings/tokenpool/lockrelease/reader.go |
Lock-release reader updates |
pkg/ccip/bindings/tokenpool/burnmint/reader.go |
Burn-mint reader updates |
pkg/bindings/mcms/timelock/types.go |
Timelock types |
pkg/bindings/mcms/timelock/storage.go |
Timelock storage |
pkg/bindings/mcms/mcms/types.go |
MCMS types |
pkg/bindings/mcms/mcms/storage.go |
MCMS storage |
pkg/bindings/lib/access/rbac/types.go |
RBAC types |
docs/contracts/overview/ccip/token-pools/IMPLEMENTATION.md |
Token Pool documentation |
deployment/ccip/1_6_0/sequences/tokens.go |
Pool deployment initialization |
contracts/wrappers/gen/ccip/pools/TokenPool.ts |
Generated pool wrapper |
contracts/wrappers/gen/ccip/OffRamp.ts |
Generated OffRamp wrapper |
contracts/wrappers/gen/ccip/CCIPSendExecutor.ts |
Generated executor wrapper |
contracts/tests/ccip/router/Router.cursing.spec.ts |
Router curse tests |
contracts/tests/ccip/router/Router.ccipReceive.spec.ts |
Router receive tests |
contracts/tests/ccip/pools/TokenPool.ccvFees.behavior.ts |
Pool fee behavior tests |
contracts/tests/ccip/pools/TokenPool.behavior.ts |
Core pool behavior tests |
contracts/tests/ccip/pools/TokenPool.asyncHook.behavior.ts |
Async hook tests |
contracts/tests/ccip/pools/LockReleaseTokenPool.spec.ts |
Lock-release pool tests |
contracts/tests/ccip/pools/LockReleaseLockboxTokenPool.spec.ts |
Lockbox pool tests |
contracts/tests/ccip/pools/BurnMintTokenPool.spec.ts |
Burn-mint pool tests |
contracts/tests/ccip/offramp/OffRamp.Setup.ts |
OffRamp test setup |
contracts/tests/ccip/offramp/OffRamp.execute.spec.ts |
OffRamp execution tests |
contracts/tests/ccip/e2e/CCIPSendWithTokenTransfer.spec.ts |
End-to-end transfer tests |
contracts/tests/ccip/accounts/OnRampAccount.spec.ts |
OnRamp account tests |
contracts/tests/ccip/accounts/DepositAccount.spec.ts |
Deposit account tests |
contracts/contracts/ccip/router/messages.tolk |
Router messages |
contracts/contracts/ccip/router/errors.tolk |
Router errors |
contracts/contracts/ccip/router/contract.tolk |
Router relay and bounce handling |
contracts/contracts/ccip/pools/TokenPool.tolk |
Token Pool contract |
contracts/contracts/ccip/pools/lock_release/storage.tolk |
Lock-release storage |
contracts/contracts/ccip/pools/lock_release/contract.tolk |
Lock-release contract |
contracts/contracts/ccip/pools/lock_release_lockbox/storage.tolk |
Lockbox storage |
contracts/contracts/ccip/pools/lock_release_lockbox/contract.tolk |
Lockbox contract |
contracts/contracts/ccip/pools/lib/token_pool/types.tolk |
Local policy types |
contracts/contracts/ccip/pools/lib/token_pool/messages.tolk |
Pool messages |
contracts/contracts/ccip/pools/lib/token_pool/events.tolk |
Pool events |
contracts/contracts/ccip/pools/lib/token_pool/entrypoint.tolk |
Pool authorization and policy flow |
contracts/contracts/ccip/pools/burn_mint/storage.tolk |
Burn-mint storage |
contracts/contracts/ccip/pools/burn_mint/contract.tolk |
Burn-mint contract |
contracts/contracts/ccip/offramp/messages.tolk |
OffRamp messages |
contracts/contracts/ccip/offramp/contract.tolk |
OffRamp relay flow |
contracts/contracts/ccip/ccipsend_executor/messages.tolk |
Executor messages |
contracts/contracts/ccip/ccipsend_executor/errors.tolk |
Executor errors |
contracts/contracts/ccip/ccipsend_executor/contract.tolk |
Executor failure handling |
cciplib/ton/tlbe/uint128_test.go |
Uint128 tests |
cciplib/ton/tlbe/boxeduint_test.go |
Boxed integer tests |
cciplib/ton/tlbe/biguint.go |
Fixed-width integer types |
cciplib/ton/tlbe/biguint_test.go |
Integer type tests |
Review details
Suppressed comments (2)
contracts/contracts/ccip/pools/lib/token_pool/entrypoint.tolk:1426
senderis now the Router, so the subsequent assignment records the Router asTokenPool_LockOrBurnForwardPayload.originalSender; that value is later emitted asTokenPool_LockedOrBurnedDetails.senderand carried through the withdrawal flow. The request already contains the actual source-chain account inrequest.transfer.details.originalSender, so outbound events and hook payloads currently attribute every lock/burn to the Router instead of the user.
self.onlyRouter(sender);
contracts/contracts/ccip/router/contract.tolk:277
- This bounce handler looks up the current
offRampsentry instead of preserving the OffRamp that authenticated the original relay. If governance removes the lane while the pool request is in flight,mustGetreverts and the ReceiveExecutor never receives a failure; if the lane is replaced, the failure is sent to the wrong OffRamp. Preserve the original OffRamp in the relay/pending state and use that address for bounce delivery.
val request = lazy richMsg.request.load();
val details = lazy request.transfer.details.load();
val st = lazy Storage.load();
val offRamp = st.offRamps.mustGet(details.remoteChainSelector, Router_Error.SourceChainNotEnabled);
- Files reviewed: 56/57 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // pool cannot accidentally bypass the Router boundary. | ||
| self.onlyRouter(sender); | ||
|
|
||
| if (self.hooks != null && self.hooks.onLockOrBurn != null) { |
There was a problem hiding this comment.
Fixed in be2eb3e
Actually moved back to be more consistent with EVM and not hoist the self.onlyRouter(sender); check outside the validateLockOrBurn fun.
| // pool cannot accidentally bypass the Router boundary. | ||
| self.onlyRouter(sender); | ||
|
|
||
| if (self.hooks != null && self.hooks.onReleaseOrMint != null) { |
There was a problem hiding this comment.
Fixed in be2eb3e
Actually moved back to be more consistent with EVM and not hoist the self.onlyRouter(sender); check outside the validateReleaseOrMint fun.
| fun onBouncedMessage(in: InMessageBounced) { | ||
| if (in.bouncedBody.preloadUint(32) == 0xfffffffe) { | ||
| val rich = lazy RichBounceBody.fromSlice(in.bouncedBody); | ||
| val richMsg = lazy Router_RichBouncedMessage.fromCell(rich.originalBody); |
There was a problem hiding this comment.
Fixed in be2eb3e
Added AskToTransfer handling with a TODO note that this is in the process of moving to deposit acc flow:
// TODO: propagate the failure to the OnRamp so it can fail the send and the
// executor can settle. Requires a failure message plus an executor `replyTo` for
// this leg, which the Router does not carry. The withdrawal is being reworked to
// route through an on-ramp deposit account, which will own this failure path.
…e all rich bounces Token-pool operation hooks returned before the only code path performing the Router, curse, and rate-limit checks, so a pool overriding `onLockOrBurn` or `onReleaseOrMint` admitted requests without them. This contradicted the documented contract on `ensureNotCursed` and on the `validateLockOrBurn` hook. Follow EVM: `_validateLockOrBurn` / `_validateReleaseOrMint` are virtual and own the curse check and `_onlyOnRamp` / `_onlyOffRamp`. An override replaces them and takes on every check. Mirror that placement by moving `onlyRouter` out of the dispatcher and into `validateLockOrBurn` / `validateReleaseOrMint`, alongside `ensureNotCursed`, and drop the "cannot bypass" comments. Hook docs now state that setting an operation hook replaces the base admission checks. In-repo pools leave both hooks unset, so the default path is unchanged. Separately, `onBouncedMessage` treated every rich bounce as a token-pool relay and decoded `Router_RichBouncedMessage` unconditionally. `onWithdrawToTokenPool` sends `AskToTransfer` with rich bounce, which failed that union match and reverted inside the bounce handler. Add `AskToTransfer` to the union and handle it explicitly. The withdrawal failure is not yet propagated to the OnRamp, so the send stalls and the executor keeps waiting. Emit `TokenPoolWithdrawBounced` for visibility and leave a TODO; routing the withdrawal through an on-ramp deposit account is tracked separately.
Wrap single-field reads of adminConfig/localPolicy/prepared with `lazy` so TVM deserializes only the prefix up to the accessed field instead of full config structs. Applies to onlyRouter, onlyAdvancedPoolHooks, onlyOwnerOr*, curse checks, validators, finalize/refund paths, and preflight/postflight checks.
Merge boxeduint/uint128 tests into biguint_test.go, remove the unused dynamic-width BigUint (superseded by comparable Uint128/160/256), and fix checkstyle (G115, unused, testifylint, revive).
18cb546
Summary
Reworks the Token Pool (TP) access-control model so that all inbound/outbound token operations are authenticated solely by the Router, rather than by per-pool onRamp/offRamp mappings. The pool no longer performs ramp authentication — the Router becomes the single chokepoint that verifies the sender is the correct OnRamp/OffRamp and applies its own RMN curse policy before dispatching to the pool. The pool keeps only an independent, owner-managed local curse policy.
Motivation
Previously each token pool stored a
mirroredPolicywithonRamps/offRampsmaps and ranensureOutboundAccess/ensureInboundAccessguards (with overridable hooks) to authenticate callers. This duplicated ramp authentication that the Router already owns, and the overridable access hooks let a custom pool bypass the Router boundary if it mishandled the check. Centralizing authentication in the Router removes this class of bug and simplifies pool storage/config.Key changes
Access control (
entrypoint.tolk)onLockOrBurnandonReleaseOrMintnow call a newonlyRouter(sender)guard before invoking any overridable operation hook, so a custom pool cannot bypass the Router boundary.onlyAdvancedPoolHooks(sender)guards the four async hook continuations (onPreflightCheckFinished/Failed,onPostflightCheckFinished/Failed), preventing a forger from injecting a fake success/failure continuation after the Router-authenticated op returns to the async flow.ensureOutboundAccess/ensureInboundAccesshooks and their base ramp-lookup implementations.ensureNotCursednow always enforces the pool's local curse policy first (hooks may add restriction but cannot bypass it).rmnProxybecomes nullable (address?); the owner can now disable RMN-proxy updates by settingaddr_none.setRMNProxy/getRMNProxyandonlyOwnerOrRMNProxyupdated accordingly.Storage/types (
types.tolk, concrete pools)TokenPool_MirroredPolicy(onRamps + offRamps + cursedSubjects) with a slimmerTokenPool_LocalPolicy(cursedSubjects only). RemovedTokenPool_RampUpdateand theTokenPool_UpdateRampAccessmessage/flow, itsRampAccessUpdatedevent, and theonRamp/offRamp/getMirroredPolicygetters.burn_mint,lock_release,lock_release_lockbox) updated: removed null access hooks and ramp getters, switched storage init tolocalPolicy.setDynamicConfignow asserts the router address is non-zero.Router (
contract.tolk,messages.tolk,errors.tolk)Router_LockOrBurnnow performs a Router-side curse check (isCursedon the destination selector) and returns a structuredRouter_TokenPoolLockOrBurnFailedto the executor when rejected.Router_RelayReleaseOrMint— the registered OffRamp relays the release-or-mint request to the Router, which asserts the sender is that OffRamp, bindsdetails.remoteChainSelector == sourceChainSelector(newSourceChainSelectorMismatcherror), applies its RMN curse policy, and forwards aTokenPool_ReleaseOrMintto the pool.Router_RichBouncedMessagehandling inonBouncedMessage: pre-admission pool bounces (viaRichBounce) are caught and surfaced as canonicalRouter_TokenPool…Failedmessages.Send executor (
ccipsend_executor)TokenPool_LockOrBurnFailure(structured pool failure, may arrive before or during withdrawal) andRouter_TokenPoolLockOrBurnFailed(Router policy rejection / pre-admission bounce), both drivingexitWithError(newTokenPoolBounceerror). Validates sender is the pool/router and thattokenPoolmatches state.Tests & wrappers
Router.ts, pools,MockTokenPool,CCIPSendExecutor).