Skip to content

feat: add dry-run transaction contract#9

Merged
yongjun925 merged 9 commits into
mainfrom
feat/edge-privy-dry-run-transaction
Jul 10, 2026
Merged

feat: add dry-run transaction contract#9
yongjun925 merged 9 commits into
mainfrom
feat/edge-privy-dry-run-transaction

Conversation

@yongjun925

Copy link
Copy Markdown
Contributor

Summary

This PR replaces the previous swap prepare approach with a dry-run transaction contract on existing swap commands.

It keeps the CLI surface unchanged:

  • no new swap prepare subcommand
  • swap execute --dry-run --json returns a standard unsigned transaction payload
  • swap approve --dry-run --json returns ERC-20 approve calldata
  • swap revoke --dry-run --json returns ERC-20 revoke calldata

Supersedes #8.

What Changed

  • Adds an external-signer payload to dry-run JSON responses:

    • source
    • operation
    • chain_id
    • caip2
    • from
    • transaction { to, value, data, chain_id, gas }
    • quote
    • risk
  • Supports:

    • operation="swap_execute" for swap execute --dry-run
    • operation="approve" for swap approve --dry-run
    • operation="revoke" for swap revoke --dry-run
  • Keeps existing legacy JSON fields for compatibility.

  • Fixes symbol-token approve dry-run by using resolved TokenRef.decimals, so swap approve --token USDC --dry-run does not require an extra RPC decimals lookup after token resolution.

  • Rejects native token approvals with a clear error.

  • Bumps version to 1.2.0-rc.3.

Example

chainpilot --json --chain-id 8453 \
  --wallet-address 0x49Ca8a959a78E926a928Eb0c1c05679a94985a2A \
  swap approve \
  --token USDC \
  --spender 0x3A7Bc5F9E41356728f037f17D88c642EE46d1Aaa \
  --amount 1 \
  --dry-run

@github-actions

Copy link
Copy Markdown

Findings

  • P1 src/commands/swap.rs:1389: swap execute --dry-run succeeds without a wallet, but silently omits the entire external-signer contract (source, operation, transaction, quote, and risk). This contradicts clap’s promise that dry-run emits an unsigned payload and can break consumers expecting data.transaction. Require a resolved sender for dry-run and return a structured error when neither signer, --wallet, nor global wallet is available.

  • P2 src/commands/swap.rs:979: execute dry-run ignores --gas-limit and --max-fee-gwei; it always uses quote gas and emits no max fee. Thus the unsigned transaction differs from the requested transaction. Pass these overrides into build_dry_run_execute_payload and encode the fee in wei hex. Clarify whether --gas-buffer-pct and --skip-estimate apply to dry-run.

  • P2 README.md:248: both README tables still say dry-run “Build and simulate” (README_CN.md says the equivalent), while clap and the new skill define it as payload construction without execution. Update both descriptions to avoid implying an RPC simulation.

Project Checks

  • Output convention: complete. No new direct println!/print! was introduced; command results use print_output.
  • Clap help: changed commands and arguments have help text. The gas-option dry-run applicability remains unclear as noted above.
  • Skills: skills/chainpilot/SKILL.md was updated for the new workflow.
  • Multi-language README: English and Chinese updates are synchronized, aside from the shared stale dry-run wording above.
  • Verification: cargo test --quiet and git diff --check pass.

@github-actions

Copy link
Copy Markdown
  1. High: approve/revoke dry-run can emit an unusable payload without from. swap.rs and swap.rs allow dry-run with neither signer nor WALLET_ADDRESS; build_dry_run_approval_result then serializes transaction while omitting from. This contradicts the documented stable external-signer contract and differs from execute dry-run, which returns NoDryRunWallet. Require a resolved sender for approve/revoke too, or explicitly document and model sender-less payloads.

  2. Medium: approve help describes the wrong quote-derived spender. swap.rs says --quote-id derives the “router address,” while swap.rs actually selects the chain’s dodo_approve contract. Update the clap text to say DODOApprove/approval contract so users know what they authorize.

Project checks:

  • Output convention: complete. No new direct println!/print!; changed command results use print_output.
  • Skills: skills/chainpilot/SKILL.md covers the new workflow, subject to the missing-sender mismatch above.
  • README: English and Chinese READMEs were both updated consistently, with the same mismatch above.
  • Tests: git diff --check passed. cargo test --locked could not complete because the restricted environment could not resolve index.crates.io.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1] Explicit dry-run wallet is ignored when a signer is configuredsrc/commands/swap.rs:844
    dry_run_from_address prioritizes the address derived from PRIVATE_KEY/keystore over --wallet and --wallet-address. Thus swap execute --dry-run --wallet 0xExternal... can emit a payload whose from belongs to an unrelated configured signer. This contradicts the flag help and normal CLI-over-environment precedence, and an external signer may reject or incorrectly authorize the transaction. Prefer subcommand_wallet.or(global_wallet).or(derived_from_signer).

  • [P2] Global wallet help is stale for the new workflowsrc/cli/mod.rs:43
    --wallet-address is described only as “quote/simulate context,” but the PR makes it required sender input for approve/revoke dry-runs. Update the help text to include those commands and explain that it populates the unsigned transaction sender.

Project-specific checks:

  • Output convention: complete. No new direct println!/print! calls; changed handler results use print_output.
  • Help docs: incomplete due to the stale global --wallet-address description above.
  • Skills: skills/chainpilot/SKILL.md was updated for execute/approve/revoke dry-runs.
  • Multi-language README: README.md and README_CN.md contain matching updates.
  • Tests: could not run because dependencies were unavailable locally and network access to crates.io failed.

@github-actions

Copy link
Copy Markdown
  • High: Approval dry-run reports incorrect risk.token_in.amount_usd. swap.rs always derives USD value from the saved quote’s original input amount. This is wrong when --amount differs from the quote or --token overrides the quote token, potentially misleading external signer risk checks. Compute USD value from the actual approval token and approval amount, or omit it when unavailable.

  • Low: Execute dry-run sender documentation does not match resolution behavior. mod.rs says global --wallet-address applies only to approve/revoke, and SKILL.md explicitly says execute uses --wallet, but swap.rs also accepts global --wallet-address/WALLET_ADDRESS. Either document this fallback consistently or remove it.

Project checks:

  • Output convention: complete; no new direct println!/print!.
  • Help docs: updated, except for the sender-resolution mismatch above.
  • Skills: updated, but contains the same mismatch.
  • READMEs: both English and Chinese were updated and remain synchronized.

@github-actions

Copy link
Copy Markdown

Findings

  • High: src/commands/swap.rs:1022 treats quote.to_amount_min as base units, but the quote model stores it as a human-readable amount. It then exposes the original value as min_amount_raw at line 1060. For a 2970 USDC minimum, the payload reports raw "2970" and display "0.00297" instead of raw "2970000000" and display "2970". External signer risk checks could therefore substantially underestimate the minimum output. Keep to_amount_min as min_amount_display and derive min_amount_raw with to_raw_amount(..., quote.to_token.decimals).

Project-specific checks:

  • Output convention: complete. No new direct println!/print! calls were introduced.
  • Clap help: updated consistently for the changed dry-run behavior and arguments.
  • Skills: skills/chainpilot/SKILL.md documents the new external-signer workflow.
  • READMEs: both English and Chinese READMEs were updated with equivalent content.
  • Tests: could not run because dependencies were unavailable locally and network access to crates.io failed.

@github-actions

Copy link
Copy Markdown

No blocking issues found in the PR changes.

Project-specific checks:

  • Output convention: Complete. No new direct println!/print!; command results use print_output.
  • Help docs: Complete and consistent with dry-run behavior.
  • Skills: skills/chainpilot/SKILL.md documents the new external-signer workflow.
  • READMEs: README.md and README_CN.md contain corresponding updates and remain synchronized.
  • git diff --check: Passed.
  • Tests: Could not run because dependencies were unavailable locally and network access to crates.io is blocked.

@github-actions

Copy link
Copy Markdown
  • P2: Unlimited approval dry-runs expose "unlimited" in risk.token_in.amount_raw (src/commands/swap.rs). amount_raw should contain the actual decimal U256::MAX value so external signer/risk systems can parse it consistently. Keep "unlimited" only in the legacy top-level field, or add an explicit unlimited indicator.

Project-specific checks:

  • Output convention: no new direct stdout printing; changed handlers use print_output.
  • Help text: updated and consistent with the changed dry-run behavior.
  • Skills: skills/chainpilot/SKILL.md documents the new workflow.
  • READMEs: English and Chinese documentation were both updated consistently.

Tests could not run because dependencies were unavailable locally and network access to crates.io failed.

@yongjun925
yongjun925 merged commit 3af2d8e into main Jul 10, 2026
5 checks passed
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.

1 participant