Skip to content

chore: update erc20 extension queries#1228

Merged
MicBun merged 14 commits into
mainfrom
chore/update-erc20-actions
Oct 23, 2025
Merged

chore: update erc20 extension queries#1228
MicBun merged 14 commits into
mainfrom
chore/update-erc20-actions

Conversation

@williamrusdyputra
Copy link
Copy Markdown
Contributor

@williamrusdyputra williamrusdyputra commented Oct 23, 2025

current version migrated on mainnet

Summary by CodeRabbit

  • New Features

    • Richer Ethereum mainnet bridge info (chain, escrow, epoch period, token decimals, balances, sync status) and 30-minute distribution periods added for Sepolia and Ethereum; Sepolia escrow address updated.
  • Refactor

    • Bridge endpoints separated/renamed to explicitly target Sepolia (testnet) and Ethereum (mainnet).
  • Tests

    • Tests updated to use the Sepolia-specific endpoint and align with Ethereum bridge queries.

@williamrusdyputra williamrusdyputra self-assigned this Oct 23, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 23, 2025

Walkthrough

Split ERC20 bridge actions into sepolia- and ethereum-specific variants, added commented USE erc20 blocks for sepolia and ethereum with distribution_period: '30m', updated the sepolia escrow address, renamed mainnet_transferethereum_transfer, and updated tests to call the sepolia-specific info action.

Changes

Cohort / File(s) Summary
Bridge extension
internal/migrations/erc20-bridge/000-extension.sql
Added commented USE erc20 blocks for sepolia and ethereum (both with distribution_period: '30m'); added a TESTING ONLY comment block; updated sepolia_bridge escrow from 0x2D4f4358...0909Ad2 to 0x502430eD0BbE0f230215870c9C2853e126eE5Ae3.
Bridge actions
internal/migrations/erc20-bridge/001-actions.sql
Renamed get_erc20_bridge_info()sepolia_get_erc20_bridge_info() (TESTNET); added ethereum_get_erc20_bridge_info() (MAINNET) with extended return columns (chain, escrow, epoch_period, erc20, decimals, balance, synced, synced_at, enabled); switched several internal calls to use ethereum_bridge.* and reorganized TESTNET/MAINNET annotations.
Public transfer actions
internal/migrations/erc20-bridge/002-public-transfer-actions.sql
Renamed exported mainnet_transfer($to_address TEXT, $amount TEXT)ethereum_transfer($to_address TEXT, $amount TEXT) and updated implementation to call ethereum_bridge.transfer(...).
Tests
tests/extensions/erc20/erc20_bridge_transfer_actions_test.go
Updated test helper to call sepolia_get_erc20_bridge_info (was get_erc20_bridge_info) to obtain escrow address; validation and error handling unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SQL_Actions
  participant SepoliaBridge
  rect rgb(240,248,255)
    Client->>SQL_Actions: sepolia_get_erc20_bridge_info()
    SQL_Actions->>SepoliaBridge: sepolia_bridge.info()
    SepoliaBridge-->>SQL_Actions: (escrow, epoch_period, erc20, decimals, balance, synced, synced_at, enabled)
    SQL_Actions-->>Client: return sepolia info
  end
Loading
sequenceDiagram
  participant Client
  participant SQL_Actions
  participant EthereumBridge
  rect rgb(245,255,240)
    Client->>SQL_Actions: ethereum_get_erc20_bridge_info()
    SQL_Actions->>EthereumBridge: ethereum_bridge.info()
    EthereumBridge-->>SQL_Actions: (chain, escrow, epoch_period, erc20, decimals, balance, synced, synced_at, enabled)
    SQL_Actions-->>Client: return ethereum info
  end
Loading
sequenceDiagram
  participant Client
  participant SQL_Actions
  participant EthereumBridge
  rect rgb(255,250,240)
    Client->>SQL_Actions: ethereum_transfer(to, amount)
    SQL_Actions->>EthereumBridge: ethereum_bridge.transfer(to, amount)
    EthereumBridge-->>SQL_Actions: transfer result / error
    SQL_Actions-->>Client: return result
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • outerlook

Poem

🐇 I hopped across the ledger bright,
Sepolia's escrow gleamed anew tonight,
Ethereum's call split off in tune,
Epochs tick beneath a silver moon,
I nibbled code and cheered — transfers right.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: update erc20 extension queries" is related to the changeset and accurately reflects that the erc20 extension queries are being updated. However, it captures only a surface-level aspect of the changes rather than the primary refactoring. The core changes involve separating ethereum and sepolia bridge concerns through significant renaming: mainnet-related functions are renamed to ethereum (e.g., mainnet_transfer → ethereum_transfer), and the main query is split into sepolia_get_erc20_bridge_info and ethereum_get_erc20_bridge_info. The title's generic phrasing doesn't clearly communicate this architectural restructuring to someone scanning the git history.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/update-erc20-actions

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9995732 and 6aec304.

📒 Files selected for processing (4)
  • internal/migrations/erc20-bridge/000-extension.sql (1 hunks)
  • internal/migrations/erc20-bridge/001-actions.sql (2 hunks)
  • internal/migrations/erc20-bridge/002-public-transfer-actions.sql (2 hunks)
  • tests/extensions/erc20/erc20_bridge_transfer_actions_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: williamrusdyputra
PR: trufnetwork/node#1228
File: internal/migrations/erc20-bridge/000-extension.sql:16-19
Timestamp: 2025-10-23T07:00:26.182Z
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.
📚 Learning: 2025-10-23T07:00:26.182Z
Learnt from: williamrusdyputra
PR: trufnetwork/node#1228
File: internal/migrations/erc20-bridge/000-extension.sql:16-19
Timestamp: 2025-10-23T07:00:26.182Z
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/001-actions.sql
  • internal/migrations/erc20-bridge/002-public-transfer-actions.sql
  • internal/migrations/erc20-bridge/000-extension.sql
🔇 Additional comments (6)
internal/migrations/erc20-bridge/001-actions.sql (3)

1-17: LGTM! Clear network-specific naming convention.

The rename from get_erc20_bridge_info to sepolia_get_erc20_bridge_info with the TESTNET marker provides excellent clarity. The implementation correctly references sepolia_bridge.info() and maintains the same return structure.


28-44: LGTM! MAINNET action follows consistent pattern.

The ethereum_get_erc20_bridge_info action mirrors the sepolia variant with identical signature and structure. References ethereum_bridge.info() which aligns with the production configuration blocks in 000-extension.sql.

Based on learnings, the ethereum_bridge binding is commented out in the test-only configuration, so this action becomes active when deploying to production.


46-53: LGTM! Data source references correctly updated.

The updates from mainnet_bridge to ethereum_bridge (lines 47, 52) are correct and align with the new binding nomenclature. Consistent with the broader migration to network-specific naming.

tests/extensions/erc20/erc20_bridge_transfer_actions_test.go (1)

247-265: LGTM! Test correctly updated for renamed action.

The update from get_erc20_bridge_info to sepolia_get_erc20_bridge_info (line 251) correctly targets the renamed TESTNET action. The validation logic and escrow extraction remain unchanged and appropriate.

internal/migrations/erc20-bridge/002-public-transfer-actions.sql (1)

22-35: LGTM! Action correctly renamed and references proper binding.

The rename from mainnet_transfer to ethereum_transfer (line 22) with the corresponding update from mainnet_bridge.transfer to ethereum_bridge.transfer (line 34) is correct and consistent with the network-specific naming convention. The validation logic remains appropriate.

internal/migrations/erc20-bridge/000-extension.sql (1)

1-19: LGTM! Configuration correctly separates production and test environments.

The structure clearly delineates production configurations (commented blocks with distribution_period: '30m' at lines 2-6 and 8-12) from the active test configuration (lines 16-19). The updated sepolia escrow address and the TESTING ONLY comment section provide clear guidance.

Based on learnings, the active sepolia_bridge configuration is intentionally for test environments and correctly omits the distribution_period parameter that production deployments would include.


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.

@williamrusdyputra williamrusdyputra marked this pull request as ready for review October 23, 2025 03:53
@holdex
Copy link
Copy Markdown

holdex Bot commented Oct 23, 2025

Time Submission Status

Member Status Time Action Last Update
williamrusdyputra ✅ Submitted 20min Update time Oct 23, 2025, 7:35 AM
MicBun ✅ Submitted 15min Update time Oct 23, 2025, 7:54 AM

MicBun

This comment was marked as outdated.

@MicBun MicBun dismissed their stale review October 23, 2025 04:16

The CI fails

@MicBun
Copy link
Copy Markdown
Contributor

MicBun commented Oct 23, 2025

The CI fails, can you check? @williamrusdyputra

Copy link
Copy Markdown

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/migrations/erc20-bridge/000-extension.sql (1)

1-1: Update the comment to reflect multi-chain support.

The comment indicates the contract address is for mainnet only, but the migration now supports both sepolia and ethereum chains. Clarify what the comment means or update it.

--- This is not meant to be run on tests as the contract address is valid for mainnet only.
+-- This is not meant to be run on tests as the contract addresses are valid for mainnet only.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c1abfd and a0bbb71.

📒 Files selected for processing (1)
  • internal/migrations/erc20-bridge/000-extension.sql (1 hunks)
🔇 Additional comments (1)
internal/migrations/erc20-bridge/000-extension.sql (1)

5-6: Verify the new escrow address with your testnet deployment and DevOps team.

Git history confirms the escrow address change was intentional (deca98f chore: change escrow address). Codebase analysis found no hardcoded references to the old address (0x2D4f435867066737bA1617ef024E073413909Ad2), and the new address 0xf6009345d772a04f4e52150b5ec91617db79be58 appears only in the migration file. Tests dynamically fetch the escrow address via getBridgeEscrowAddress(), so no breakage risk exists within the codebase.

However, confirm with your testnet/deployment team that the new address is correct for the sepolia testnet deployment and that no external systems depend on the previous address.

Comment thread internal/migrations/erc20-bridge/000-extension.sql Outdated
Copy link
Copy Markdown

@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: 1

♻️ Duplicate comments (1)
internal/migrations/erc20-bridge/002-public-transfer-actions.sql (1)

22-35: Action references unavailable binding.

The ethereum_transfer action correctly calls ethereum_bridge.transfer() (line 34), but the ethereum_bridge binding is not available—it remains commented out in 000-extension.sql. This action will fail at runtime when invoked until the binding is enabled.

Dependency: This issue is blocked by uncommenting the ethereum_bridge binding in 000-extension.sql (lines 9–14). Ensure that change is applied before this action becomes active on mainnet.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89689d6 and d86bcf6.

📒 Files selected for processing (3)
  • internal/migrations/erc20-bridge/000-extension.sql (1 hunks)
  • internal/migrations/erc20-bridge/002-public-transfer-actions.sql (2 hunks)
  • tests/extensions/erc20/erc20_bridge_transfer_actions_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/extensions/erc20/erc20_bridge_transfer_actions_test.go
🔇 Additional comments (2)
internal/migrations/erc20-bridge/000-extension.sql (1)

5-7: Verify escrow address and distribution_period changes for sepolia_bridge.

The sepolia_bridge escrow address has been updated from 0x2D4f435867066737bA1617ef024E073413909Ad2 to 0xf6009345d772a04f4e52150b5ec91617db79be58, and distribution_period: '30m' has been added. Confirm these changes are intentional, validated against the contract deployment, and consistent with the mainnet migration scope.

internal/migrations/erc20-bridge/002-public-transfer-actions.sql (1)

5-18: ✓ sepolia_transfer action is properly integrated.

The sepolia_transfer action correctly references the sepolia_bridge binding, which is uncommented and configured in 000-extension.sql. Validation logic and binding call are correct.

Comment thread internal/migrations/erc20-bridge/000-extension.sql Outdated
Copy link
Copy Markdown

@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: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d86bcf6 and 6aec304.

📒 Files selected for processing (1)
  • internal/migrations/erc20-bridge/000-extension.sql (1 hunks)
⏰ 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

Comment thread internal/migrations/erc20-bridge/000-extension.sql
Comment thread internal/migrations/erc20-bridge/000-extension.sql
Comment thread internal/migrations/erc20-bridge/000-extension.sql
@williamrusdyputra
Copy link
Copy Markdown
Contributor Author

@MicBun tests passed, migration changed to protect accidental execution, only enabling for tests.

@MicBun
Copy link
Copy Markdown
Contributor

MicBun commented Oct 23, 2025

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 23, 2025

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@MicBun MicBun left a comment

Choose a reason for hiding this comment

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

lgtm!

@MicBun MicBun merged commit d3b21de into main Oct 23, 2025
8 checks passed
@MicBun MicBun deleted the chore/update-erc20-actions branch October 23, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants