chore: add internal transfer fee#1245
Conversation
|
Warning Rate limit exceeded@williamrusdyputra has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 45 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
WalkthroughAdds a fixed 1 TRUF on‑chain fee to internal transfer actions ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bridge
participant Leader
participant Recipient
Note right of Bridge: sepolia_transfer / ethereum_transfer (updated)
User->>Bridge: sepolia_transfer(amount)
activate Bridge
Bridge->>Bridge: Read caller balance (balance)
alt balance >= amount + 1 TRUF
Bridge->>Leader: Transfer 1 TRUF fee (leader_sender)
Bridge->>Bridge: Deduct amount from caller balance
Bridge->>Recipient: Credit amount
Note right of Bridge: Original transfer logic continues
else insufficient
Bridge-->>User: Error: Insufficient balance (requires extra 1 TRUF fee)
end
deactivate Bridge
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/migrations/erc20-bridge/002-public-transfer-actions.sql (1)
16-26: Guard balance reads against NULL and compare with total requirement
sepolia_bridge.balance(@caller)/ethereum_bridge.balance(@caller)can come back NULL for first-time callers, which makes the subtraction/comparison evaluate to NULL and skip the friendly error branch. Coalescing to 0 and comparing againstamount + feekeeps the custom message and avoids depending on downstream transfer failures. Please wrap both balance reads inCOALESCEand compare directly against the summed requirement.- $caller_balance := sepolia_bridge.balance(@caller); - IF ($caller_balance - $amount::NUMERIC(78, 0)) < $fee { + $caller_balance := COALESCE(sepolia_bridge.balance(@caller), 0::NUMERIC(78, 0)); + IF ($caller_balance < ($amount::NUMERIC(78, 0) + $fee)) { ERROR('Insufficient balance for transfer. Requires an extra 1 TRUF fee on top of the transfer amount'); } @@ - $caller_balance := ethereum_bridge.balance(@caller); - IF ($caller_balance - $amount::NUMERIC(78, 0)) < $fee { + $caller_balance := COALESCE(ethereum_bridge.balance(@caller), 0::NUMERIC(78, 0)); + IF ($caller_balance < ($amount::NUMERIC(78, 0) + $fee)) { ERROR('Insufficient balance for transfer. Requires an extra 1 TRUF fee on top of the transfer amount'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/migrations/erc20-bridge/002-public-transfer-actions.sql(2 hunks)tests/extensions/erc20/common_test.go(3 hunks)tests/extensions/erc20/erc20_bridge_transfer_actions_test.go(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-12T01:19:04.545Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1147
File: tests/extensions/erc20/erc20_bridge_actions_smoke_test.go:40-47
Timestamp: 2025-09-12T01:19:04.545Z
Learning: In ERC20 bridge tests, the maintainer (outerlook) prefers fail-fast behavior through panics when schema changes occur. Direct indexing and type assertions in test row handlers should be maintained to immediately catch unexpected schema changes, rather than using graceful error handling.
Applied to files:
tests/extensions/erc20/erc20_bridge_transfer_actions_test.go
📚 Learning: 2025-10-23T07:00:26.796Z
Learnt from: williamrusdyputra
Repo: trufnetwork/node PR: 1228
File: internal/migrations/erc20-bridge/000-extension.sql:16-19
Timestamp: 2025-10-23T07:00:26.796Z
Learning: In internal/migrations/erc20-bridge/000-extension.sql, the active sepolia_bridge configuration is intentionally for test environments only and does not include the distribution_period parameter. The commented sepolia_bridge and ethereum_bridge blocks with distribution_period are production configurations.
Applied to files:
internal/migrations/erc20-bridge/002-public-transfer-actions.sql
🧬 Code graph analysis (1)
tests/extensions/erc20/erc20_bridge_transfer_actions_test.go (2)
tests/streams/utils/erc20/inject.go (1)
InjectERC20Transfer(24-85)tests/extensions/erc20/common_test.go (10)
TestChain(20-20)TestERC20(24-24)TestUserA(25-25)TestUserB(26-26)TestAmount1(29-29)TestUserC(27-27)TestAmount2(30-30)TestEscrowA(22-22)TestExtensionAlias(21-21)TestUserD(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (2)
tests/extensions/erc20/common_test.go (1)
65-82: Leader bootstrap change looks goodGenerating a provisional proposer and wiring
EthPersonalSignAuthkeeps @leader_sender populated for the fee transfer path without touching the rest of the harness.tests/extensions/erc20/erc20_bridge_transfer_actions_test.go (1)
250-354: Thorough coverage on the new fee scenariosReally appreciate the trio of exact/insufficient/multiple transfer cases—these lock in the sender-fee semantics and the error string.
Time Submission Status
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/migrations/erc20-bridge/002-public-transfer-actions.sql(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-23T07:00:26.796Z
Learnt from: williamrusdyputra
Repo: trufnetwork/node PR: 1228
File: internal/migrations/erc20-bridge/000-extension.sql:16-19
Timestamp: 2025-10-23T07:00:26.796Z
Learning: In internal/migrations/erc20-bridge/000-extension.sql, the active sepolia_bridge configuration is intentionally for test environments only and does not include the distribution_period parameter. The commented sepolia_bridge and ethereum_bridge blocks with distribution_period are production configurations.
Applied to files:
internal/migrations/erc20-bridge/002-public-transfer-actions.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (2)
internal/migrations/erc20-bridge/002-public-transfer-actions.sql (2)
24-25: Verify SQL variable declaration and address encoding approach.Lines 24–25 declare and initialize
$leader_addrin a single statement usingencode(@leader_sender, 'hex')::TEXT. Confirm:
- The SQL dialect supports this declaration/assignment syntax
encode(@leader_sender, 'hex')::TEXTcorrectly converts the leader's public key/identity into a valid Ethereum address format compatible with the bridge'stransfer()methodIf
@leader_senderis a public key or binary identifier, ensure the hex encoding produces a string in the format expected bysepolia_bridge.transfer()(likely0x...prefixed).
44-53: Verify consistency of address encoding inethereum_transfer.The
ethereum_transferaction mirrors the fee-transfer pattern fromsepolia_transfer(after the fix on line 20). Lines 52–53 use the sameencode(@leader_sender, 'hex')::TEXTencoding strategy. Once you've verified the encoding approach insepolia_transfer, confirm the same strategy is correct here as well.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
2 participants
resolves: https://github.com/trufnetwork/truf-network/issues/1314
Summary by CodeRabbit
New Features
Tests