Skip to content

chore: remove withdrawal fee from non-custodial bridge#1299

Merged
MicBun merged 1 commit intomainfrom
hoodiNoWithdrawal
Jan 13, 2026
Merged

chore: remove withdrawal fee from non-custodial bridge#1299
MicBun merged 1 commit intomainfrom
hoodiNoWithdrawal

Conversation

@MicBun
Copy link
Member

@MicBun MicBun commented Jan 13, 2026

resolves: https://github.com/truflation/website/issues/3115

Summary by CodeRabbit

  • New Features
    • Multiple ERC20 bridge instances now operate with full isolation and independence. Each bridge maintains separate balance records, and transactions in one instance do not affect balances, deposits, or withdrawals in another.
    • Withdrawal fees have been eliminated from all ERC20 bridge transactions. Users receive the complete requested withdrawal amount without any deductions, charges, or additional fees.

✏️ Tip: You can customize this high-level summary in your review settings.

@MicBun MicBun self-assigned this Jan 13, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 13, 2026

📝 Walkthrough

Walkthrough

The pull request removes withdrawal fee collection logic from Hoodi ERC20 bridge token actions, simplifying balance checks and transaction processing. Associated test files are updated to validate no-fee behavior, and a new coexistence test ensures two bridge instances operate independently.

Changes

Cohort / File(s) Summary
Fee logic removal
internal/migrations/erc20-bridge/001-actions.sql
Removed fee calculation, leader fee transfer, and transaction event recording from hoodi_tt_bridge_tokens and hoodi_tt2_bridge_tokens actions; balance check now validates only withdrawal amount required
Test updates for no-fee semantics
tests/streams/hoodi_withdrawal_fee_test.go
Renamed test suite and helpers from fee-based to no-fee naming; removed 40 TRUF fee constant and associated deduction logic; updated assertions to verify no fee transfer and leader balance remains unchanged
New coexistence test
tests/streams/hoodi_bridge_coexistence_test.go
Added comprehensive test validating isolation between two Hoodi bridge instances (hoodi_tt and hoodi_tt2) with deposit, withdrawal, and cross-contamination scenarios

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • williamrusdyputra
  • outerlook
  • pr-time-tracker

Poem

🐰 Fees now vanish into the ether,
No leader takes the toll,
Two bridges stand in harmony,
Each one controls its soul.
Testing proves they coexist,
Independent, strong, and true! 🌉

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and directly describes the main change: removing withdrawal fees from the non-custodial bridge, which is reflected across all modified files (migration SQL, tests).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@holdex
Copy link

holdex bot commented Jan 13, 2026

Time Submission Status

Member Status Time Action Last Update
MicBun ✅ Submitted 3h 30min Update time Jan 13, 2026, 1:59 PM

@MicBun MicBun marked this pull request as ready for review January 13, 2026 13:56
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
tests/streams/hoodi_bridge_coexistence_test.go (2)

38-43: Package-level mutable state may cause issues in parallel test execution.

The package-level variables ttPointCounter, tt2PointCounter, ttPrevPoint, and tt2PrevPoint are modified during test execution. While the tests currently run sequentially via RunSchemaTest, this pattern could lead to data races if tests are ever parallelized or if multiple test functions in this package run concurrently.

Consider encapsulating this state in a test struct or resetting these values atomically in setupBridgeCoexistenceEnvironment. The current reset of ttPrevPoint and tt2PrevPoint in setupBridgeCoexistenceEnvironment (lines 64-65) helps, but the counters are not reset.


301-370: Consider extracting common helper logic to reduce duplication.

The giveTTBalance/executeTTWithdrawal functions are nearly identical to giveTT2Balance/executeTT2Withdrawal, differing only in the chain/escrow/ERC20 constants and action name. This is acceptable for test code clarity, but could be refactored into parameterized helpers if the pattern expands further.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55461dc and c473ea7.

📒 Files selected for processing (3)
  • internal/migrations/erc20-bridge/001-actions.sql
  • tests/streams/hoodi_bridge_coexistence_test.go
  • tests/streams/hoodi_withdrawal_fee_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
tests/streams/hoodi_withdrawal_fee_test.go (2)
tests/streams/utils/runner.go (1)
  • RunSchemaTest (37-116)
internal/migrations/migration.go (1)
  • GetSeedScriptStatements (60-109)
⏰ 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: slow-integration-tests
  • GitHub Check: acceptance-test
🔇 Additional comments (15)
internal/migrations/erc20-bridge/001-actions.sql (4)

24-37: LGTM! Fee removal from hoodi_tt_bridge_tokens is correctly implemented.

The balance check now correctly validates only against $withdrawal_amount, and the error message clearly indicates the required token amount. The logic flow is clean: check balance → resolve recipient → execute bridge.


62-75: LGTM! hoodi_tt2_bridge_tokens mirrors hoodi_tt_bridge_tokens correctly.

Both Hoodi bridge actions follow the same no-fee pattern, ensuring consistency across the two bridge instances.


100-133: Verify that retaining fees for Sepolia is intentional.

The Sepolia bridge (sepolia_bridge_tokens) still includes the 40 TRUF withdrawal fee, @leader_sender check, and record_transaction_event call. This differs from the Hoodi bridges which have had fees removed.

Is retaining fees for Sepolia intentional? If the goal is to remove fees from all non-custodial bridges, this action may need similar updates.


158-191: Verify that retaining fees for Ethereum mainnet is intentional.

The Ethereum mainnet bridge (ethereum_bridge_tokens) retains the same fee structure as Sepolia. If this PR is specifically targeting Hoodi testnet only, this is fine, but please confirm this is the intended scope.

tests/streams/hoodi_bridge_coexistence_test.go (6)

60-105: LGTM! Setup function correctly initializes both bridge instances.

The setup properly:

  1. Resets point trackers for deposit chaining
  2. Sets the system admin deployer
  3. Syncs both hoodi_tt and hoodi_tt2 bridge instances
  4. Initializes the extension to load instances into the singleton

This ensures a clean state for each test run.


107-133: LGTM! Test 1 correctly validates TT deposit isolation.

The test verifies that depositing to the TT bridge only affects the TT balance while leaving TT2 balance at zero. Good use of distinct user addresses.


135-161: LGTM! Test 2 correctly validates TT2 deposit isolation.

Mirrors Test 1's pattern for the TT2 bridge, ensuring symmetrical coverage.


163-223: LGTM! Test 3 thoroughly validates withdrawal isolation.

The test:

  1. Resets deposit chains to ensure clean state
  2. Gives equal balances to both bridges
  3. Verifies TT withdrawal doesn't affect TT2 balance and vice versa

Good coverage of the no-fee withdrawal logic.


225-297: LGTM! Test 4 provides comprehensive cross-contamination validation.

Tests that operations by different users on different bridges remain completely isolated. This is the most thorough test in the suite.


341-370: LGTM! executeTTWithdrawal helper correctly constructs the transaction context.

The helper properly:

  1. Creates a TxContext with the signer and leader proposer
  2. Calls the hoodi_tt_bridge_tokens action
  3. Handles both direct errors and result errors
tests/streams/hoodi_withdrawal_fee_test.go (5)

45-58: LGTM! Test suite correctly renamed to reflect no-fee semantics.

The test suite and identifier updates are consistent with the PR objectives. The function tests properly validate:

  1. Setup environment
  2. No fee is charged on withdrawal
  3. Insufficient balance rejection
  4. Leader receives no fees

96-139: LGTM! testHoodiWithdrawalNoFee correctly validates no-fee withdrawal.

The test verifies that withdrawing 10 TRUF from a 100 TRUF balance results in exactly 90 TRUF remaining, confirming no fee is deducted.


141-170: LGTM! Insufficient balance test updated correctly.

The test now gives the user 5 TRUF and attempts a 10 TRUF withdrawal, which correctly fails. This aligns with the no-fee logic where only the withdrawal amount is checked.


172-225: LGTM! Leader fee test correctly updated to verify no fees are transferred.

The test now asserts that the leader's balance remains unchanged after a withdrawal, confirming no fee transfer occurs.


271-307: LGTM! Helper functions remain consistent with the no-fee implementation.

The callHoodiWithdrawalAction helper correctly calls hoodi_tt_bridge_tokens with just the recipient and amount parameters, and the comment at line 291 accurately reflects "no fees".

@MicBun MicBun merged commit 1687fbb into main Jan 13, 2026
13 checks passed
@MicBun MicBun deleted the hoodiNoWithdrawal branch January 13, 2026 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant