feat: bridge with hoodi tt2 non-custodial contract#1298
Conversation
Time Submission Status
|
📝 WalkthroughWalkthroughThe PR renames the Hoodi ERC20 bridge extension from Changes
Sequence Diagram(s)(silently skipped — conditions for diagram generation not met) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
🔇 Additional comments (6)
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://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
internal/migrations/erc20-bridge/001-actions.sql (1)
59-115: Addhoodi_tt2_get_withdrawal_proofaction to maintain parity withhoodi_tt.The
hoodi_tt2bridge is missing the withdrawal proof action. Whilehoodi_tt_get_withdrawal_proofexists in004-withdrawal-proof-action.sql, there is no correspondinghoodi_tt2_get_withdrawal_proof. Since both test tokens have identical bridge action patterns, TT2 users will need withdrawal proof functionality. Add the missing action following the same pattern ashoodi_tt_get_withdrawal_proof, replacinghoodi_ttnamespace references withhoodi_tt2.tests/streams/hoodi_withdrawal_fee_test.go (5)
127-141: Inconsistent naming in log message.Line 127 correctly uses
hoodi_tt_bridge_tokens, but line 140's log message still referenceshoodi_bridge_tokens. Update for consistency.Suggested fix
- t.Logf("✅ hoodi_bridge_tokens correctly deducted 50 TRUF (10 withdrawal + 40 fee)") + t.Logf("✅ hoodi_tt_bridge_tokens correctly deducted 50 TRUF (10 withdrawal + 40 fee)")
167-172: Inconsistent naming in log message.Line 167 correctly uses
hoodi_tt_bridge_tokens, but line 171's log message still referenceshoodi_bridge_tokens.Suggested fix
- t.Logf("✅ hoodi_bridge_tokens correctly rejects insufficient balance (30 TRUF < 50 TRUF needed)") + t.Logf("✅ hoodi_tt_bridge_tokens correctly rejects insufficient balance (30 TRUF < 50 TRUF needed)")
214-216: Inconsistent naming in require message.Line 215's require message still references
hoodi_bridge_tokenswhile other test functions have been updated to usehoodi_tt_bridge_tokens.Suggested fix
- require.NoError(t, err, "hoodi_bridge_tokens with leader should succeed") + require.NoError(t, err, "hoodi_tt_bridge_tokens with leader should succeed")
354-385: Inconsistent function comment.The function
callHoodiWithdrawalActioncomment at line 354 still referenceshoodi_bridge_tokenswhile line 374 correctly callshoodi_tt_bridge_tokens.Suggested fix
-// callHoodiWithdrawalAction calls the hoodi_bridge_tokens action +// callHoodiWithdrawalAction calls the hoodi_tt_bridge_tokens action
99-100: Inconsistent test function comment.The comment at line 99 still references
hoodi_bridge_tokens. Update tohoodi_tt_bridge_tokensfor consistency with the renamed action.Suggested fix
-// Test 1: hoodi_bridge_tokens pays 40 TRUF fee +// Test 1: hoodi_tt_bridge_tokens pays 40 TRUF fee
🤖 Fix all issues with AI agents
In @tests/extensions/erc20/erc20_bridge_withdrawal_proof_test.go:
- Around line 27-35: Update the stale comment in the
TestHoodiGetWithdrawalProofAction block: replace the phrase
"migration-registered hoodi_bridge instance" with "migration-registered hoodi_tt
instance" so the comment correctly reflects the renamed instance (refer to the
test function TestHoodiGetWithdrawalProofAction to locate the comment).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/migrations/erc20-bridge/000-extension.sqlinternal/migrations/erc20-bridge/001-actions.sqlinternal/migrations/erc20-bridge/003-disable-hoodi.prod.sqlinternal/migrations/erc20-bridge/004-withdrawal-proof-action.sqltests/extensions/erc20/erc20_bridge_withdrawal_proof_test.gotests/streams/hoodi_withdrawal_fee_test.go
🧰 Additional context used
🪛 Gitleaks (8.30.0)
internal/migrations/erc20-bridge/000-extension.sql
[high] 24-24: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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 (12)
internal/migrations/erc20-bridge/000-extension.sql (1)
22-41: LGTM! New hoodi_tt2 bridge extension and rename look good.The static analysis warnings about "Generic API Key" on lines 24 and 34 are false positives — these are public Ethereum contract addresses (token and proxy contracts) that are intentionally documented here and publicly visible on block explorers.
internal/migrations/erc20-bridge/001-actions.sql (2)
1-17: LGTM! Renamed hoodi_tt actions are correctly updated.The namespace migration from
hoodi_*tohoodi_tt_*is consistent, and all internal calls now correctly referencehoodi_tt.info().
24-57: LGTM! Bridge tokens action properly handles fee collection and event recording.The logic correctly:
- Validates balance against total required (withdrawal + fee)
- Transfers fee to leader before bridge operation
- Records the transaction event after successful bridge
internal/migrations/erc20-bridge/004-withdrawal-proof-action.sql (1)
1-27: LGTM! Withdrawal proof action correctly renamed to hoodi_tt namespace.The action properly:
- Uses
hoodi_tt.list_wallet_rewardsfor the internal call- Returns all required fields including validator signatures
- Uses
RETURN NEXTfor multiple row supportinternal/migrations/erc20-bridge/003-disable-hoodi.prod.sql (1)
1-7: LGTM! Disable migration correctly updated for hoodi_tt.The
UNUSE hoodi_ttdirective and accompanying comments are consistent with the extension rename.tests/extensions/erc20/erc20_bridge_withdrawal_proof_test.go (4)
19-25: LGTM! Test constants correctly updated.The
MigrationHoodiAliasconstant is properly updated to"hoodi_tt"matching the extension rename.
38-228: LGTM! TestTestHoodiGetWithdrawalProofActionproperly validates the renamed action.The test comprehensively validates:
- Merkle proof structure (32-byte root and proof elements)
- Validator signatures (65 bytes each)
- All 10 return columns
- Correct chain/contract/recipient matching
231-301: LGTM! Pending epoch exclusion test correctly updated.Test name and action calls properly reference
hoodi_tt_get_withdrawal_proof.
303-448: LGTM! Multiple users test correctly validates independent withdrawal proofs.The test properly verifies that each user receives their own withdrawal proof with correct amounts.
tests/streams/hoodi_withdrawal_fee_test.go (3)
23-30: LGTM!The extension name constant is correctly updated to
hoodi_tt, aligning with the renamed extension.
47-61: LGTM!Comment correctly updated to reference the renamed
hoodi_tt_bridge_tokensaction.
63-97: LGTM!Setup function comment correctly updated to reference the
hoodi_ttinstance name.
1 participant
resolves: https://github.com/truflation/website/issues/3117
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.