-
Notifications
You must be signed in to change notification settings - Fork 8
feat: auto-calculate transfer fees via Bridge Indexer API #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
8efab08 to
ebf0939
Compare
frolvanya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Thanks for the contribution!
I know that PR is still in a draft, just decided to mention issues right now, since some of them is easier to fix now instead of the time when all logic for all chains will be ready
Also, we need to calculate fee for init_transfer on every chain (right now it's on near only), therefore instead of duplicating bridge_indexer_api_url for all bridge clients it's better to move whole fee calculation logic to cli's side (I've described reasoning behind this in one of the comments)
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
e2d1fa1 to
fc3a329
Compare
-use U128-typed fees -early-return on missing indexer URL -error instead of defaulting on missing transfer fee -include gas_fee in schema and update tests. Tests: cargo test -p near-bridge-client --tests; cargo clippy --workspace --all-targets --all-features. Use shared indexer URL constants for mainnet/testnet/devnet defaults.
6f1118e to
8ed0faa
Compare
- Add bridge-cli/src/fee.rs helper (v3 endpoint, U128-typed fees) and module wiring. - In NearInitTransfer, fetch fees via indexer when not provided and pass them explicitly; handle missing config/parse/API errors without panicking. - Keep client-side indexer logic untouched for now.
8ed0faa to
049dc05
Compare
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
- Drop bridge_indexer_api_url, TransferFee DTO, reqwest/dev deps, and get_transfer_fee helper from near-bridge-client. - Require callers to supply fee/native_fee to init_transfer (error if missing); no HTTP calls in the client. - Stop passing indexer URL into NearBridgeClient builder in the CLI.
Extends the CLI fee-fetch pattern to EvmInitTransfer, SolanaInitTransfer, and SolanaInitTransferSol commands. When fees are omitted, the CLI fetches them from the Bridge Indexer v3 API. Changes: - Add derive_evm_sender() to get chain:0x... from configured private key - Add derive_solana_sender() to get sol:<pubkey> from configured keypair - Add chain_prefix() returning Result to avoid silent failures - Add native_fee_to_u64() with overflow check for Solana - Make fee/native_fee optional for EVM and Solana init commands - Wire fetch_transfer_fee() with proper error handling (no panics) - Preserve escape hatch: skip indexer when both fees are provided - Use sol:So111... for native SOL token address UTXO gas_fee/protocol_fee wiring deferred to follow-up commit.
Addresses feedback to reduce code duplication in match branches. Changes: - Add resolve_near_fees() for Near init transfer fee resolution - Add resolve_evm_fees() for EVM chains (Eth/Base/Arb/Bnb/Pol) - Add resolve_solana_fees() for Solana SPL and native SOL transfers - Inline fetch_fee_quote usage from fee::fetch_transfer_fee - Each helper returns Result, propagates errors cleanly to call site - Preserves escape hatch: returns early when both fees are provided - Match arms now concise single-call + error handling
- Use ChainKind::as_ref() for prefixes - Rely on OmniAddress::new_from_evm_address for EVM sender/token strings. - Rename/document solana_native_fee_to_u64 to make the Solana-only downcast explicit and error on overflow.
…Request struct and NativeFeeConversion enum were added to satisfy clippy "too many args". - Add message plumbing for Near init transfers, so UTXO gas/protocol fee payloads can be passed through
bridge-sdk/bridge-clients/near-bridge-client/src/near_bridge_client.rs
Outdated
Show resolved
Hide resolved
- Remove `pub` from `utxo_bridges` field in NearBridgeClient for encapsulation - Replace generic `resolve_fees` with simpler `fetch_indexer_fees` helper - Remove `FeeRequest` struct, `NativeFeeConversion` enum, and closures - Move fee branching logic (manual vs auto) to call sites for clarity - Chain-specific wrappers (`resolve_evm_fees`, `resolve_solana_fees`) now call `fetch_indexer_fees` directly and handle type conversions - Require both `fee` and `native_fee` for manual mode; error on partial input
frolvanya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix CI as well
| let (fee, native_fee, gas_fee) = match (fee, native_fee) { | ||
| (Some(f), Some(nf)) => (f, nf, None), | ||
| (Some(_), None) | (None, Some(_)) => { | ||
| eprintln!("Both fee and native_fee must be provided to skip indexer calculation"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's incorrect. Transfers need only one type of fee: either transferred fee or native fee, so we don't need to provide both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see. Relaxed requirements to allow either --fee or --native-fee independently. And set the other one to 0.
… flexible fee input
- Unified TransferFeeDto and FeeQuote into a single TransferFee struct using near-sdk compatible U128 types. - Relaxed fee requirements for init-transfer commands to allow providing either --fee or --native-fee independently. Empty values set to 0. - Refactored derive_evm_sender to utilize the evm_address_to_omni helper for address derivation. - Cleaned up unused usd_fee code and resolved clippy warnings for inlined format arguments.
|
Implements automatic fee calculation for NEAR transfers by integrating with the Bridge Indexer API.
Fixes #181
Key Features:
transferred_token_feeandnative_token_feefrom/api/v1/transfer-feewhen not explicitly provided by the user.bridge_indexer_api_urltoCliConfig(configurable via CLI arg or env var).Safety & Hardening:
reqwestwith a strict 10s timeout to prevent hangs.usd_feeoptional in deserialization to handle API variance.native_feeis correctly added to the required storage balance.Implementation Details:
init_transferinnear-bridge-clientto accept optional fees.reqwestandserdedependencies for API interaction.Verification & Testing
test_bridge_indexer_fee_response_parsingunit test for JSON structure verification.test_get_transfer_fee_from_indexerintegration test usinghttpmockto simulate realistic API responses and verify client behavior.I have verified the implementation using:
httpmock).