feat: transfer token within network#120
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Go Transfer binding and Python TNClient.transfer, splits and normalizes bridge identifier allowlists, updates bridge-using callers (balance, withdraw, proof, history, market creation), updates API docs with client.transfer, and bumps the sdk-go dependency. ChangesTransfer Method Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
|
@holdex pr submit-time 4h |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/trufnetwork_sdk_py/client.py`:
- Around line 394-403: The global VALID_BRIDGES list mixes plain bridge names
and action-specific extension aliases, causing false-valid inputs; split it into
per-operation allowlists (e.g., VALID_BRIDGES for core names like
"eth_truf","eth_usdc","hoodi_tt","hoodi_tt2" and VALID_BRIDGE_EXTENSIONS for
"sepolia_bridge","ethereum_bridge") and add a normalization step in dispatching
functions (the methods that consume VALID_BRIDGES) to map accepted aliases
("sepolia" <-> "sepolia_bridge", "ethereum" <-> "ethereum_bridge") to the
correct operation-specific identifier before making downstream calls; update
validation checks to use the appropriate allowlist (or normalized value) so
methods only accept names valid for that operation.
🪄 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: 73ecf6a6-711e-4a97-abe2-7a463bba2be4
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
bindings/bindings.godocs/api-reference.mdgo.modsrc/trufnetwork_sdk_py/client.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/trufnetwork_sdk_py/client.py (1)
2914-2956: 💤 Low valueAPI surface nit: argument order diverges from
withdraw, and the docstring under-documents accepted bridge values.Two small things on the new public method:
Parameter order vs.
withdraw.transfer(bridge_identifier, recipient, amount)follows ERC-20'stransfer(to, amount)convention, which is reasonable. Butwithdraw(bridge_identifier, amount, recipient)(line 2891) putsamountbeforerecipient. Users alternating between the two will hit a footgun where positional calls type-check (both arestr) but silently swap the values —recipient="1000..."andamount="0x..."won't surface until on-chain. Consider adding a one-line note in each docstring cross-referencing the other to make the divergence intentional and discoverable. (Reordering an existing public API is the riskier alternative.)Docstring is incomplete vs.
VALID_BRIDGES. Line 2935 lists only"eth_truf","eth_usdc","sepolia", omittinghoodi_tt,hoodi_tt2, andethereum— all of which pass validation. The docstring also doesn't mention that extension-namespace aliases (sepolia_bridge,ethereum_bridge) are silently accepted via_normalize_bridge_for_action, even though that's an explicit design choice in this PR. Worth surfacing so callers don't avoid passing what they think is the "wrong" identifier.📝 Suggested docstring tweak
Args: bridge_identifier: Bridge / action namespace prefix - (e.g. ``"eth_truf"``, ``"eth_usdc"``, ``"sepolia"``). + (e.g. ``"eth_truf"``, ``"eth_usdc"``, ``"hoodi_tt"``, + ``"hoodi_tt2"``, ``"sepolia"``, ``"ethereum"``). Extension + aliases ``"sepolia_bridge"`` / ``"ethereum_bridge"`` are + also accepted and normalized to their action-prefix form. recipient: The destination wallet address (Ethereum 0x… format). Must already exist as an in-network address. amount: The transfer amount in wei (as a string to preserve precision). wait: If True, wait for the transaction to be confirmed on-chain. + + Note: + Argument order intentionally follows ERC-20 (``recipient`` before + ``amount``); ``withdraw`` uses the opposite order + (``amount`` before ``recipient``).🤖 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 `@src/trufnetwork_sdk_py/client.py` around lines 2914 - 2956, The transfer method's parameter ordering diverges from withdraw and its docstring omits valid bridge identifiers/aliases; update the transfer and withdraw docstrings to include a one-line note pointing out the intentional parameter-order difference between transfer(bridge_identifier, recipient, amount) and withdraw(bridge_identifier, amount, recipient) to avoid positional footguns, and expand the transfer docstring to list all entries in VALID_BRIDGES (including hoodi_tt, hoodi_tt2, ethereum) and mention that _normalize_bridge_for_action also accepts extension-namespace aliases (e.g. sepolia_bridge, ethereum_bridge) so callers know those identifiers are accepted.
🤖 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 `@src/trufnetwork_sdk_py/client.py`:
- Around line 2914-2956: The transfer method's parameter ordering diverges from
withdraw and its docstring omits valid bridge identifiers/aliases; update the
transfer and withdraw docstrings to include a one-line note pointing out the
intentional parameter-order difference between transfer(bridge_identifier,
recipient, amount) and withdraw(bridge_identifier, amount, recipient) to avoid
positional footguns, and expand the transfer docstring to list all entries in
VALID_BRIDGES (including hoodi_tt, hoodi_tt2, ethereum) and mention that
_normalize_bridge_for_action also accepts extension-namespace aliases (e.g.
sepolia_bridge, ethereum_bridge) so callers know those identifiers are accepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0f92645-fea6-42ed-a0ab-d83c07870e97
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modsrc/trufnetwork_sdk_py/client.py
resolves: https://github.com/truflation/website/issues/3849
Summary by CodeRabbit
New Features
Documentation