feat(sdk): add round-robin test example for Go and TS SDKs#789
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds runnable Go and TypeScript round‑robin channel lifecycle examples (asset discovery, preflight, iterative deposit/transfer/close/withdraw, optional session keys), updates TypeScript packaging/tsconfig, tweaks EVM simulate/account and ERC‑20 approve behavior, and adds cross‑chain home‑ledger blockchain ID guards with tests. ChangesRound‑Robin Channel Lifecycle Examples + Guards
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
sdk/ts/examples/round_robin/lifecycle.ts (1)
288-291: 💤 Low valueAvoid
anytypes for signer parameters.The
stateSignerandtxSignerparameters are typed asany, which bypasses TypeScript's type checking. The SDK should have proper types for these signers (e.g.,ChannelSigneror similar interfaces).Suggested typing improvement
-async function newClient(stateSigner: any, txSigner: any): Promise<Client> { +async function newClient(stateSigner: EthereumMsgSigner | ChannelSessionKeyStateSigner, txSigner: EthereumRawSigner): Promise<Client> {Alternatively, import and use the proper signer interface types from the SDK if available.
As per coding guidelines, "Use strict TypeScript with no
anytypes unless unavoidable (e.g., RPC wire types)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/ts/examples/round_robin/lifecycle.ts` around lines 288 - 291, The function newClient currently types stateSigner and txSigner as any which disables TypeScript checks; change the signature to use the SDK signer interfaces (e.g., replace stateSigner: any, txSigner: any with the appropriate types such as ChannelSigner or WalletSigner exported by the SDK), import those signer types at top of the file, and update any callers to pass objects matching those interfaces; ensure the types are used in the call to Client.create so the compiler validates the signers passed to Client.create(wsURL, stateSigner, txSigner, ...opts).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sdk/ts/examples/round_robin/lifecycle.ts`:
- Around line 288-291: The function newClient currently types stateSigner and
txSigner as any which disables TypeScript checks; change the signature to use
the SDK signer interfaces (e.g., replace stateSigner: any, txSigner: any with
the appropriate types such as ChannelSigner or WalletSigner exported by the
SDK), import those signer types at top of the file, and update any callers to
pass objects matching those interfaces; ensure the types are used in the call to
Client.create so the compiler validates the signers passed to
Client.create(wsURL, stateSigner, txSigner, ...opts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8b58cc0-d53f-4ad9-8223-3a71a61d3d3d
📒 Files selected for processing (4)
sdk/go/examples/round_robin/main.gosdk/ts/examples/round_robin/lifecycle.tssdk/ts/examples/round_robin/package.jsonsdk/ts/examples/round_robin/tsconfig.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/ts/examples/round_robin/lifecycle.ts (1)
87-88: ⚡ Quick winLoad the example private keys from env instead of editing the file.
These placeholders keep the example compiling, but the intended workflow is still “paste secrets into source.” Reading them from env and validating at startup is safer and gives a clearer failure than an
as Hexcast.🔐 Suggested direction
-const privA = '0x7d6071...' as Hex; -const privB = '0xf63695...' as Hex; +const privA = requiredHexEnv('ROUND_ROBIN_PRIV_A'); +const privB = requiredHexEnv('ROUND_ROBIN_PRIV_B');function requiredHexEnv(name: string): Hex { const value = process.env[name]; if (!value?.startsWith('0x')) { throw new Error(`${name} must be set to a 0x-prefixed private key`); } return value as Hex; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/ts/examples/round_robin/lifecycle.ts` around lines 87 - 88, Replace hard-coded example private keys privA and privB with environment-driven validation: add a helper like requiredHexEnv(name) that reads process.env[name], verifies it exists and starts with "0x", and returns it typed as Hex; then call requiredHexEnv('PRIV_A') and requiredHexEnv('PRIV_B') to initialize privA and privB so the example fails fast with a clear error instead of using an as Hex cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/go/examples/round_robin/main.go`:
- Around line 322-346: The polling loops in waitForTxReceipt, closeAndWait,
checkpointAndWait, and waitForOnChain ignore ctx cancellation because they call
time.Sleep(2 * time.Second) directly; replace those sleeps with a cancellable
helper (e.g., sleepOrDone(ctx, 2*time.Second)) that returns early when
ctx.Done() is closed (or use select { case <-time.After(...): case <-ctx.Done():
return/continue } inline) so each loop honors context cancellation and deadlines
while keeping the existing retry logic.
---
Nitpick comments:
In `@sdk/ts/examples/round_robin/lifecycle.ts`:
- Around line 87-88: Replace hard-coded example private keys privA and privB
with environment-driven validation: add a helper like requiredHexEnv(name) that
reads process.env[name], verifies it exists and starts with "0x", and returns it
typed as Hex; then call requiredHexEnv('PRIV_A') and requiredHexEnv('PRIV_B') to
initialize privA and privB so the example fails fast with a clear error instead
of using an as Hex cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 916661ab-5658-4b27-8d3f-7f15bee30946
⛔ Files ignored due to path filters (1)
sdk/ts/examples/round_robin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
sdk/go/examples/round_robin/main.gosdk/ts/examples/round_robin/lifecycle.tssdk/ts/src/blockchain/evm/client.tssdk/ts/src/blockchain/evm/erc20.ts
Exercises deposit, transfer, close, and withdraw across every supported chain for one asset. Funds circulate through the loop: deposit on chain[i], off-chain transfer to B, close, B transfers back to A (off-chain credit), withdraw on chain[i+1]. Last iteration wraps to chain[0]. Preflight checks per-chain native gas balance and seed-chain asset balance before running. Optional channel session key path mirrored from the channel_session_key example. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The blockchain client and ERC20 wrapper were stripping the wallet's LocalAccount down to its `.address` before passing it into simulateContract. viem parses a bare address as a JsonRpcAccount, which then propagates into writeContract and routes through eth_sendTransaction — a wallet-namespaced RPC method that public HTTP providers (DRPC, Alchemy, etc.) reject because they do not hold the private key. Pass the full account so viem retains the LocalAccount on the returned request and writeContract signs locally + calls eth_sendRawTransaction. Browser flow (JsonRpcAccount from window.ethereum) is unchanged — same account type flows through; MetaMask still receives eth_sendTransaction over the custom transport. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Wait for approve tx receipt before deposit (Go side; the TS SDK's approve already waits internally). - Replace post-close checkpointAndWait with a dedicated closeAndWait that polls for channel == null or status == Void after the on-chain close, rather than waiting for a state_version bump that never lands. - Special-case checkpointAndWait for expectedVersion == 0 (the channel-creation case after Withdraw on a void state): wait for status == Open instead of a stale state_version comparison. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… channel Deposit and Withdraw silently created channel-creation transitions on the caller-supplied blockchainID even when an open home channel for that asset already existed on a different chain. The mismatch surfaced later as a confusing on-chain failure rather than at the SDK call site. Add an early guard in both methods (Go and TS) that returns an error when the latest state has an active (non-final) home channel whose HomeLedger.BlockchainID does not match the requested chain. The "no channel" and "final channel" paths still create a fresh channel on the requested chain, so the new rule only applies while an existing channel is in use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Non-standard tokens such as Tether USDT on Ethereum L1 declare `approve` without a `returns (bool)`, so the pre-flight simulateContract path blew up with ContractFunctionZeroDataError when viem tried to decode the empty return value against the standard ERC-20 ABI. The transaction itself would succeed because writeContract never decodes return data on non-view calls. Drop the simulate step in the ERC20 wrapper's approve and submit the write directly. Loses the pre-flight revert check (acceptable for approve specifically — failures cost only the failed-tx gas) but unblocks every non-standard ERC-20. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ound-robin - waitForTxReceipt in the Go example now waits approveConfirmations (=3) additional blocks past the approve receipt, and the TS SDK's ERC-20 approve waits the same on its post-receipt block. Both target the load-balanced public-RPC read-after-write race where a downstream allowance/eth_call read could hit a node that has not yet indexed the approve. - setupSessionKeyClient in both examples now looks up the latest stored state for the configured session key (including inactive/expired) and resumes at version+1 instead of always submitting v1. Re-running the example against the same wallet+session-key no longer fails with "version must be monotonically increasing". - Go example also honors sessionKeyPriv when provided (TS already did), matching the documented "If sessionKeyPriv is non-empty, register it" behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
philanton
force-pushed
the
feat/round-robin-test
branch
from
c32d868 to
e01cfc1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
sdk/ts/test/unit/client.test.ts (1)
346-394: ⚡ Quick winRefactor these guard tests into a data-driven
.forEach()table.The new deposit/withdraw guard cases are structurally duplicated and can drift. A single table-driven block will keep coverage equivalent with less maintenance.
As per coding guidelines
sdk/ts/test/**/*.test.ts: "Use data-driven tests with.forEach()pattern and manual mocks instead ofjest.mock()."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/ts/test/unit/client.test.ts` around lines 346 - 394, Refactor the duplicated deposit/withdraw guard tests into a single data-driven table using Array.forEach so both operations (Client.deposit and Client.withdraw) are tested uniformly: build a test table of cases with method name ('deposit'|'withdraw'), targetChain values (8453n vs 137n), expected throw messages, and expectations for signAndSubmitState/requestChannelCreation; for each row create the latestState via openChannelState(), adjust latestState.homeLedger.blockchainId and homeLedger.tokenAddress as needed, createHighLevelClient(latestState), then invoke client[method](...) and assert either rejects.toThrow(expectedMessage) and that signAndSubmitState/requestChannelCreation were not called, or that signAndSubmitState was called once and requestChannelCreation was not called. Keep references to Client.deposit/Client.withdraw, createHighLevelClient, openChannelState, TOKEN_ADDRESS, signAndSubmitState, and requestChannelCreation to locate code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/go/channel.go`:
- Around line 109-113: The chain-mismatch guard (checking
state.HomeLedger.BlockchainID vs blockchainID) must run before any call to
getTokenAddress to avoid returning token lookup errors for existing channels;
move the early return that compares state.HomeLedger.BlockchainID and
blockchainID to execute prior to getTokenAddress(...) in the functions where
it's currently after token resolution (references:
state.HomeLedger.BlockchainID, blockchainID, asset, and getTokenAddress),
ensuring both the code paths around the current checks at the locations
corresponding to lines ~109 and ~227-231 perform the blockchain ID check first
and only call getTokenAddress if the chain matches.
In `@sdk/ts/examples/round_robin/lifecycle.ts`:
- Around line 306-309: Change the newClient function signature to use the SDK
signer interfaces instead of any: replace stateSigner: any and txSigner: any
with stateSigner: StateSigner and txSigner: TransactionSigner, and add imports
for StateSigner and TransactionSigner from ../../src/signers; ensure the types
align with the Client.create(wsURL, stateSigner, txSigner, ...opts) call so
TypeScript type-checks correctly.
In `@sdk/ts/src/client.ts`:
- Around line 425-431: The active-channel chain-binding check (compare
state.homeLedger.blockchainId with blockchainId) must run before any call to
getTokenAddress to ensure the cross-chain guard throws first and to avoid
unnecessary token lookups; update the code paths that call getTokenAddress (the
block around the current check using state.homeLedger.blockchainId and the
similar block around the getTokenAddress usage at the later section) so the
mismatch check executes immediately and only if it passes then call
getTokenAddress(asset, ...). Ensure both occurrences (the one referencing
state.homeLedger.blockchainId vs blockchainId and the later similar block) are
reordered accordingly.
---
Nitpick comments:
In `@sdk/ts/test/unit/client.test.ts`:
- Around line 346-394: Refactor the duplicated deposit/withdraw guard tests into
a single data-driven table using Array.forEach so both operations
(Client.deposit and Client.withdraw) are tested uniformly: build a test table of
cases with method name ('deposit'|'withdraw'), targetChain values (8453n vs
137n), expected throw messages, and expectations for
signAndSubmitState/requestChannelCreation; for each row create the latestState
via openChannelState(), adjust latestState.homeLedger.blockchainId and
homeLedger.tokenAddress as needed, createHighLevelClient(latestState), then
invoke client[method](...) and assert either rejects.toThrow(expectedMessage)
and that signAndSubmitState/requestChannelCreation were not called, or that
signAndSubmitState was called once and requestChannelCreation was not called.
Keep references to Client.deposit/Client.withdraw, createHighLevelClient,
openChannelState, TOKEN_ADDRESS, signAndSubmitState, and requestChannelCreation
to locate code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1656155e-1c34-4582-9f28-767a8d5b54ec
⛔ Files ignored due to path filters (1)
sdk/ts/examples/round_robin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
sdk/go/channel.gosdk/go/client_test.gosdk/go/examples/round_robin/main.gosdk/ts/examples/round_robin/lifecycle.tssdk/ts/examples/round_robin/package.jsonsdk/ts/examples/round_robin/tsconfig.jsonsdk/ts/src/blockchain/evm/client.tssdk/ts/src/blockchain/evm/erc20.tssdk/ts/src/client.tssdk/ts/test/unit/client.test.ts
✅ Files skipped from review due to trivial changes (1)
- sdk/ts/examples/round_robin/package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sdk/go/examples/round_robin/main.goandsdk/ts/examples/round_robin/{lifecycle.ts,package.json,tsconfig.json}.sessionKeyPriv) mirrored from the existingchannel_session_keyexample — registers a v1 key for asset, builds a session-key-backed client, and runs the entire loop through it.Test plan
go vet ./sdk/go/examples/round_robin/...go build ./sdk/go/examples/round_robin/...tsc --noEmitagainstsdk/ts/examples/round_robin/lifecycle.ts(skipLibCheck, ES2022+DOM lib)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests