[NONEVM-4760] [TXM] Classify "insufficient funds" as fatal in broadcastWithRetry#726
Conversation
701ccdb to
2cdcb4a
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the TON TxManager to stop retrying broadcasts when the failure indicates insufficient funds/zero balance, and refactors client usage to an interface so callers no longer access wallet/client fields directly.
Changes:
- Classify a specific “insufficient funds / cannot apply external message …” wallet error as fatal (no retry) in
broadcastWithRetry. - Introduce
SignedAPIClientInterfaceand migrate TXM/relay/CCIP transmitter call sites to useGetClient()/GetWallet(). - Expand TXM integration tests to cover “zero balance” and “insufficient balance” scenarios; add a new tracetracking transfer integration test; fix a docs typo.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/txm/txm.go | Adds fatal classification for insufficient-funds wallet errors; logs when no outgoing messages; migrates to client interface accessors. |
| pkg/ton/tracetracking/signed_api_client.go | Introduces SignedAPIClientInterface; wraps send failures in WalletTXError; adds GetClient/GetWallet. |
| pkg/relay/service.go | Updates relay Tx sending to use GetWallet() accessor. |
| pkg/relay/relay.go | Updates relay TxManager interface to return SignedAPIClientInterface. |
| pkg/relay/chain.go | Updates lazy client provider to return SignedAPIClientInterface and uses GetClient(). |
| pkg/ccip/ocr/contract_transmitter.go | Updates transmitter to use GetWallet() accessor. |
| integration-tests/txm/txm_test.go | Refactors/extends TXM integration tests; adds insufficient-funds coverage and log assertions. |
| integration-tests/tracetracking/transfer_test.go | Adds a new integration test around transfer/fees behavior. |
| docs/contracts/overview/ccip/offramp/arbitrary-msg.md | Fixes “transmiter” → “transmitter” typo in diagram note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
24e599b to
f9a1cd8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7bff969 to
1e769fd
Compare
| GetWallet() *wallet.Wallet | ||
| } | ||
|
|
||
| var _ SignedAPIClientInterface = (*SignedAPIClient)(nil) |
There was a problem hiding this comment.
nit: I can see you're trying to avoid duplication but adding interface suffix seems redundant
There was a problem hiding this comment.
I know, I haven't think of a better name yet. I could hide the struct as private, but tracetracking.SignedAPIClient has 43 uses in integration-tests and pkg has 10. I wanted to keep this fix contained instead of changing so many files.
…nto txm/fix/classify-insufficient-funds-as-fatal-error-in-broadcastWithRetry
… of SignedAPIClient Revert "ref: migrate txm to Interface" This reverts commit cb1394b.
NONEVM-4760
Depends on #721