feat: deposit and withdraw using hoodi#1287
Conversation
Time Submission Status
|
📝 WalkthroughWalkthroughAdds Hoodi testnet support for the ERC20 bridge: bumps kwil-db pseudo-versions, adds a Hoodi extension block, introduces Hoodi-specific SQL actions (info, balance, withdrawal with fee handling), and adds integration tests validating fee, reward, and error flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (wallet)
participant Leader as Leader (signer/fee recipient)
participant DB as Database (SQL actions)
participant Bridge as Hoodi Bridge Engine
Note over User,Leader: Initiate withdrawal with fee flow
User->>Leader: submit withdrawal request (recipient?, amount)
Leader->>DB: CALL hoodi_bridge_tokens(recipient, amount) with leader signer
DB->>DB: check wallet balance >= amount + fee
alt insufficient balance
DB-->>Leader: RETURN error
else sufficient
DB->>Leader: transfer leader fee (internal ledger entry)
DB->>Bridge: invoke hoodi_bridge.withdraw(recipient, amount)
Bridge-->>DB: withdrawal result (success/failure)
DB->>DB: record transaction_event (withdrawal, fee)
DB-->>Leader: RETURN success + event ID
end
Leader-->>User: response (success/error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/migrations/erc20-bridge/000-extension.sql (1)
16-26: Parameter inconsistency between sepolia_bridge and hoodi_bridge.The
hoodi_bridgeincludesdistribution_period: '30m'but thesepolia_bridgeon lines 16-19 omits this parameter. Ifdistribution_periodis intentionally different (relying on a default for sepolia), this is fine, but the inconsistency may cause confusion during maintenance.Consider adding a comment explaining why sepolia uses the default period or add the explicit parameter for consistency.
🔎 Suggested fix for consistency
USE erc20 { chain: 'sepolia', - escrow: '0x502430eD0BbE0f230215870c9C2853e126eE5Ae3' + escrow: '0x502430eD0BbE0f230215870c9C2853e126eE5Ae3', + distribution_period: '30m' } AS sepolia_bridge;tests/extensions/erc20/erc20_bridge_hoodi_test.go (2)
76-80: Consider adding bounds check or column name verification.Direct index access to
row.Values[6]androw.Values[8]is fragile if the schema changes. While acceptable in tests, consider adding a comment documenting the expected column order or using a more robust approach.🔎 Suggested improvement for clarity
resInfo, errInfo := platform.Engine.Call(engineCtx, platform.DB, TestExtensionAliasHoodi, "info", []any{}, func(row *common.Row) error { if len(row.Values) < 9 { return nil } + // Column order: chain(0), escrow(1), epoch_period(2), erc20(3), decimals(4), balance(5), synced(6), synced_at(7), enabled(8) syncedResult = row.Values[6].(bool) // synced column enabledResult = row.Values[8].(bool) // enabled column return nil })
276-278: Fragile column index access for recipient verification.Accessing
row.Values[4]assumes a specific column order inlist_wallet_rewards. If the return schema changes, this will break silently or cause a panic.🔎 Suggested defensive check
r, err = platform.Engine.Call(engineCtx, platform.DB, TestExtensionAliasHoodi, "list_wallet_rewards", []any{TestUserHoodiB, false}, func(row *common.Row) error { rewardRows++ // Verify the recipient column contains User B's address + require.GreaterOrEqual(t, len(row.Values), 5, "expected at least 5 columns in list_wallet_rewards result") recipientValue, ok := row.Values[4].(string) require.True(t, ok, "recipient should be string") require.True(t, strings.EqualFold(TestUserHoodiB, recipientValue), "recipient should be User B") return nil })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modinternal/migrations/erc20-bridge/000-extension.sqlinternal/migrations/erc20-bridge/001-actions.sqltests/extensions/erc20/erc20_bridge_hoodi_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
tests/extensions/erc20/erc20_bridge_hoodi_test.go (3)
deployments/infra/config/node/values.go (1)
Values(96-110)tests/streams/utils/erc20/inject.go (1)
InjectERC20Transfer(24-85)tests/streams/utils/erc20/helper.go (1)
GetUserBalance(49-74)
⏰ 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 (8)
internal/migrations/erc20-bridge/001-actions.sql (3)
30-43: Verify cross-chain fee collection is intentional.The
hoodi_bridge_tokensaction checks balance and collects fees fromethereum_bridge(lines 30, 43) but performs the withdrawal fromhoodi_bridge(line 49). This pattern is consistent withsepolia_bridge_tokens, suggesting a cross-chain fee model where fees are always collected on Ethereum mainnet.If this is intentional, consider adding a comment explaining this design decision. If users are expected to have funds on both chains, the error message on line 33-36 should clarify which chain the balance is checked on.
Please confirm this cross-chain fee collection model is correct:
- User must have sufficient balance on
ethereum_bridge(nothoodi_bridge) to cover withdrawal + fee- Fee is transferred on Ethereum mainnet
- Withdrawal is executed on Hoodi testnet
1-22: Hoodi info and balance actions look good.The
hoodi_get_erc20_bridge_infoandhoodi_wallet_balanceactions correctly usehoodi_bridgeand follow the same pattern as the existing Sepolia counterparts.
59-59: Minor: Comment label change.The comment was updated from "TESTNET" to "SEPOLIA TESTNET (kept for reference)" which improves clarity now that there are multiple testnets.
tests/extensions/erc20/erc20_bridge_hoodi_test.go (4)
1-42: Well-structured test constants and configuration.Using fake test addresses (0x333... and 0x444...) to avoid conflicts with migration-created instances is a good practice. The constants are well-documented and consistent with the Sepolia test pattern.
58-188: Comprehensive end-to-end test for Hoodi bridge.The test covers the complete flow: seed instance → inject deposit → verify balance → bridge action → verify pending rewards → finalize epoch → verify wallet rewards. Good use of intermediate assertions to catch failures early.
318-347: Good practice: chaining deposits with prev pointer.Properly linking the second deposit (pointB) to the first (pointA) via
prevPointensures correct ordered-sync processing. This accurately simulates production deposit ordering.
201-289: Custom recipient and multiple deposit tests provide good coverage.Both tests validate important edge cases:
- Custom recipient ensures withdrawal proofs are generated for the correct address
- Multiple deposits verifies per-user balance tracking and partial withdrawals
Also applies to: 305-424
go.mod (1)
22-23: Dependency versions for Hoodi testnet support are confirmed.Both
kwil-dbandkwil-db/coreare updated to the same commit (278c1930ac63a4b3a6c4f7967766e0227b7b5f13), ensuring consistency. The versions exist in the Go module proxy and align with the Hoodi bridge extension tests in the repository.
| // TestERC20BridgeEndToEndHoodi tests the complete bridge flow on Hoodi testnet. | ||
| // | ||
| // Test flow: | ||
| // 1) Initialize Hoodi bridge instance with test alias | ||
| // 2) Inject deposit to give user a balance (simulates Hoodi → Kwil deposit) | ||
| // 3) Call bridge action to lock tokens and create withdrawal request | ||
| // 4) Finalize and confirm epoch (makes withdrawal ready) | ||
| // 5) Verify user has wallet rewards (withdrawal proof available) | ||
| // | ||
| // This validates: | ||
| // - Hoodi chain is recognized by kwil-db | ||
| // - Deposit listener would work (simulated via injection) | ||
| // - Withdrawal flow works end-to-end | ||
| // - Epoch management functions correctly |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for tests that call hoodi_bridge_tokens or similar SQL actions
rg -n "hoodi_bridge_tokens|hoodi_get_erc20_bridge_info|hoodi_wallet_balance" --type go tests/Repository: trufnetwork/node
Length of output: 42
🏁 Script executed:
# Also check the structure of the test file and related test files
find tests/ -name "*hoodi*" -o -name "*bridge*" | head -20Repository: trufnetwork/node
Length of output: 662
🏁 Script executed:
# Check what SQL files exist and what actions they define
find . -name "*.sql" | grep -i "hoodi\|bridge" | head -20Repository: trufnetwork/node
Length of output: 230
🏁 Script executed:
# Check the content of 001-actions.sql to see what SQL actions are defined
cat ./internal/migrations/erc20-bridge/001-actions.sql | head -100Repository: trufnetwork/node
Length of output: 3648
🏁 Script executed:
# Search for patterns where test files execute SQL actions
rg -n "hoodi|bridge" tests/extensions/erc20/ --type go | grep -i "action\|exec\|call" | head -30Repository: trufnetwork/node
Length of output: 3580
🏁 Script executed:
# Check if the test files actually call SQL-based actions (look for ExecuteAction or similar patterns)
rg -n "ExecuteAction|ExecAction|CallAction" tests/extensions/erc20/erc20_bridge_hoodi_test.goRepository: trufnetwork/node
Length of output: 42
🏁 Script executed:
# Search more broadly for any calls to *_bridge_tokens in all test files
rg -n "_bridge_tokens" tests/ --type goRepository: trufnetwork/node
Length of output: 511
🏁 Script executed:
# Check what TestExtensionAliasHoodi refers to - see if it maps to SQL action or extension
rg -n "TestExtensionAliasHoodi" tests/extensions/erc20/ --type go -A 3 -B 3 | head -50Repository: trufnetwork/node
Length of output: 4631
🏁 Script executed:
# Check the hoodi_test.go file to see the complete test structure
cat tests/extensions/erc20/erc20_bridge_hoodi_test.go | head -150Repository: trufnetwork/node
Length of output: 6131
🏁 Script executed:
# Search for any tests that might call hoodi_bridge_tokens explicitly or via action execution
rg -n "hoodi_bridge_tokens|hoodi.*bridge.*tokens" . --type goRepository: trufnetwork/node
Length of output: 42
🏁 Script executed:
# Check if there are withdrawal_fee_test.go or transaction_events_ledger_test.go that test bridge token actions for other chains
# and verify they also test hoodi
cat tests/streams/withdrawal_fee_test.go | head -300Repository: trufnetwork/node
Length of output: 11798
🏁 Script executed:
# Look at what the engine.Call with "bridge" method does - check the extension definition
find . -name "*.go" -type f | xargs grep -l "func.*bridge.*Hoodi\|HoodiInstance" | head -5Repository: trufnetwork/node
Length of output: 42
🏁 Script executed:
# Check if there are any hoodi-specific fee or withdrawal tests
rg -n "hoodi.*fee|hoodi.*withdrawal" tests/ --type goRepository: trufnetwork/node
Length of output: 42
🏁 Script executed:
# Check the transaction_events_ledger_test.go to see if it tests hoodi_bridge_tokens
rg -n "hoodi|bridge_tokens" tests/streams/transaction_events_ledger_test.go --type go | head -30Repository: trufnetwork/node
Length of output: 186
🏁 Script executed:
# Verify what SQL actions exist for each chain
rg -n "CREATE OR REPLACE ACTION.*bridge_tokens|CREATE OR REPLACE ACTION.*get_erc20_bridge_info" ./internal/migrations/erc20-bridge/ --type sqlRepository: trufnetwork/node
Length of output: 855
SQL action hoodi_bridge_tokens is not tested directly.
The hoodi_test.go file exercises the underlying bridge extension method (via TestExtensionAliasHoodi), not the SQL action hoodi_bridge_tokens defined in 001-actions.sql which includes fee collection logic (40 TRUF withdrawal fee). Other chains (Sepolia, Ethereum) have dedicated test coverage for their SQL actions in withdrawal_fee_test.go and transaction_events_ledger_test.go. Add similar tests for hoodi_bridge_tokens to ensure fee collection and balance validation work correctly at the SQL action level.
🤖 Prompt for AI Agents
tests/extensions/erc20/erc20_bridge_hoodi_test.go around lines 44-57: the review
notes that the SQL action hoodi_bridge_tokens (defined in 001-actions.sql) is
not exercised by the current end-to-end test and must be covered directly to
validate the 40 TRUF withdrawal fee and balance logic; add a unit/integration
test that invokes the hoodi_bridge_tokens SQL action directly (via the test
harness method that executes SQL actions or db.Exec against the test database),
set up a user/account with a deposit balance, call the action with parameters
that trigger a withdrawal, assert the resulting balance decrement equals
amount+40 TRUF fee, verify fee is recorded in the expected fee/treasury ledger
table and any withdrawal record is created, and add at least one failure case
asserting action rejects insufficient-balance attempts; keep test scaffolding
consistent with existing withdrawal_fee_test.go and
transaction_events_ledger_test.go patterns.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/streams/hoodi_withdrawal_fee_test.go (2)
32-36: Package-level mutable state may cause test flakiness.
hoodiPointCounterandhoodiPrevPointare shared across all tests. If tests run in parallel (viat.Parallel()) or in an unexpected order, this shared state could cause race conditions or unexpected behavior. Consider passing these values through a test context struct or resetting them in each test's setup.
185-187: Consider removing debug logging.The
DEBUG test3:log statement appears to be leftover from development. Either remove it or, if intentionally kept for troubleshooting, ensure consistency by adding similar logging to other tests that reset state.🔎 Proposed fix
// Reset deposit chain for this test to avoid cross-test interference - oldPrevPoint := hoodiPrevPoint hoodiPrevPoint = nil - t.Logf("DEBUG test3: Reset prev point from %v to nil", oldPrevPoint)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/migrations/erc20-bridge/001-actions.sqltests/streams/hoodi_withdrawal_fee_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/migrations/erc20-bridge/001-actions.sql
🧰 Additional context used
🧬 Code graph analysis (1)
tests/streams/hoodi_withdrawal_fee_test.go (4)
tests/streams/utils/runner.go (1)
RunSchemaTest(37-116)internal/migrations/migration.go (1)
GetSeedScriptStatements(60-109)tests/streams/utils/erc20/inject.go (1)
InjectERC20Transfer(24-85)tests/streams/utils/erc20/helper.go (1)
GetUserBalance(49-74)
⏰ 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). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (8)
tests/streams/hoodi_withdrawal_fee_test.go (8)
1-21: LGTM!Build tag and imports are properly structured for a test file under the
kwiltestbuild constraint.
44-58: LGTM!The test suite structure follows the established pattern from
RunSchemaTestand properly sequences setup before test functions.
60-94: LGTM!The setup function properly initializes the Hoodi bridge test environment with clear comments explaining the critical initialization steps. Error handling is consistent with proper context wrapping.
96-140: LGTM!Test properly validates the fee deduction logic with clear assertions and informative log messages. The arithmetic using
big.Intensures precision.
142-171: LGTM!Good negative test case validating that insufficient balance is correctly rejected with an appropriate error message.
173-227: LGTM on test logic!The test correctly validates that the leader receives the 40 TRUF fee. Good use of key generation and address derivation for the leader.
292-334: LGTM!Helper functions
giveHoodiBalanceandgetHoodiBalanceare well-implemented with proper error handling and deposit chaining logic.
336-372: LGTM!The withdrawal action helpers properly set up the transaction context with the leader as proposer and handle both call errors and result errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/streams/hoodi_withdrawal_fee_test.go (1)
32-36: Consider consistent global state reset across all tests.The global
hoodiPrevPointis reset insetupHoodiWithdrawalTestEnvironmentand in tests 3 and 4, but not in tests 1 and 2. While the current sequential execution works, inconsistent reset patterns can make test behavior harder to reason about.Consider either:
- Resetting
hoodiPrevPointat the start of every test function for explicit isolation, or- Documenting why certain tests need fresh chains while others can reuse the global state
Also applies to: 67-67, 189-189, 244-244
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/streams/hoodi_withdrawal_fee_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
tests/streams/hoodi_withdrawal_fee_test.go (5)
tests/streams/utils/runner.go (1)
RunSchemaTest(37-116)internal/migrations/migration.go (1)
GetSeedScriptStatements(60-109)deployments/infra/config/node/values.go (1)
Values(96-110)tests/streams/utils/erc20/inject.go (1)
InjectERC20Transfer(24-85)tests/streams/utils/erc20/helper.go (1)
GetUserBalance(49-74)
⏰ 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 (6)
tests/streams/hoodi_withdrawal_fee_test.go (6)
38-45: Previous review comments have been addressed.Both issues flagged in the previous review have been fixed:
mustParseHoodiBigIntnow checks theSetStringreturn value and panics with a descriptive message on parse failure.- Type assertions in the ledger query use safe comma-ok checks and return errors for unexpected types.
Also applies to: 272-295
99-143: LGTM!The test correctly validates that withdrawing 10 TRUF deducts 50 TRUF total (10 withdrawal + 40 fee). The balance assertions and error messages are clear.
145-174: LGTM!The test properly validates the insufficient balance error path. The error message assertion ensures the failure reason is clear.
176-230: LGTM!The test correctly validates that the block leader receives the 40 TRUF withdrawal fee. The explicit state reset and debug logging demonstrate good test hygiene.
232-308: LGTM!The test validates that withdrawal fees are properly recorded in the transaction ledger. The SQL query explicitly casts
fee_amountto TEXT and the type assertions are safe.
312-390: LGTM!The helper functions are well-structured:
giveHoodiBalanceproperly manages deposit chaining with the global point trackergetHoodiBalancesafely parses balance stringscallHoodiWithdrawalActioncorrectly sets up the transaction context with block height 2 (post-setup)
1 participant
resolves: https://github.com/truflation/website/issues/3041
prepare for testnet. Also contain go mod upgrades
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.