Skip to content

Commit b2a1ba2

Browse files
pepe-anchorclaude
andcommitted
fix(ethereum): show first-execution-hop fee for V3 ExactOut swaps
Same bug class as the ExactOut token-role fix: ExactOut paths are encoded in reverse on-chain (`tokenOut || fee_{N-1} || ... || fee_0 || tokenIn`), and execution flows right-to-left from the signer's view. The fee the signer's input pays first is the one immediately before `tokenIn` in the byte layout, not the leading 3 bytes after `tokenOut`. The old code surfaced the leading fee as "first fee" in the disclosure subtitle, which corresponds to the FINAL execution hop. On the multi-hop fixture that's a 100x discrepancy (10000 = 1% disclosed in place of 100 = 0.01%). `decode_v3_path` now returns both the leading and trailing fees as raw positions. ExactIn keeps the leading fee (execution flows left-to-right, so first-hop = leading). ExactOut switches to the trailing fee. Tests assert the correct fee is disclosed and the wrong one is not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e3b3540 commit b2a1ba2

1 file changed

Lines changed: 80 additions & 26 deletions

File tree

src/chain_parsers/visualsign-ethereum/src/protocols/uniswap/contracts/universal_router.rs

Lines changed: 80 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -399,28 +399,42 @@ impl UniversalRouterVisualizer {
399399
/// Paths are encoded as: token(20) | fee(3) | token(20) | fee(3) | ... | token(20).
400400
/// A valid N-hop path therefore has length 20 + N * 23.
401401
///
402-
/// Returns `(first_token, last_token, first_hop_fee, hop_count)` or `None` for malformed
403-
/// paths, where `first_token` is the leading 20 bytes and `last_token` is the trailing
404-
/// 20 bytes. The helper is intentionally semantics-free: role assignment depends on the
405-
/// caller's swap direction.
402+
/// Returns `(first_token, last_token, leading_fee, trailing_fee, hop_count)` or `None`
403+
/// for malformed paths, where `first_token` is the leading 20 bytes, `last_token` is
404+
/// the trailing 20 bytes, `leading_fee` is the fee immediately after `first_token`
405+
/// (`path[20..23]`), and `trailing_fee` is the fee immediately before `last_token`
406+
/// (`path[len-23..len-20]`). On a single-hop path the two fees coincide.
406407
///
407-
/// - ExactIn paths are encoded `tokenIn || fee || ... || tokenOut`, so callers alias
408-
/// `first_token = tokenIn`, `last_token = tokenOut`.
409-
/// - ExactOut paths are encoded in reverse (`tokenOut || fee || ... || tokenIn`) per
410-
/// Uniswap V3 convention (see `SwapRouter.exactOutputInternal` /
411-
/// `Path.decodeFirstPool`), so callers must alias
412-
/// `first_token = tokenOut`, `last_token = tokenIn`.
413-
fn decode_v3_path(path: &[u8]) -> Option<(Address, Address, u32, usize)> {
408+
/// The helper is intentionally semantics-free: token and fee role assignment depends
409+
/// on the caller's swap direction.
410+
///
411+
/// - ExactIn paths are encoded `tokenIn || fee_0 || ... || tokenOut`, where execution
412+
/// flows left-to-right, so callers alias `first_token = tokenIn`,
413+
/// `last_token = tokenOut`, first-execution-hop fee = `leading_fee`.
414+
/// - ExactOut paths are encoded in reverse (`tokenOut || fee_{N-1} || ... || tokenIn`)
415+
/// per Uniswap V3 convention (see `SwapRouter.exactOutputInternal` /
416+
/// `Path.decodeFirstPool`). Execution flows right-to-left from the signer's view,
417+
/// so callers alias `first_token = tokenOut`, `last_token = tokenIn`, and the
418+
/// first-execution-hop fee (the fee the signer's input pays first) is
419+
/// `trailing_fee`, not `leading_fee`.
420+
fn decode_v3_path(path: &[u8]) -> Option<(Address, Address, u32, u32, usize)> {
414421
// Single hop minimum: 20 + 23 = 43 bytes. Each additional hop adds 23 bytes.
415422
if path.len() < 43 || (path.len() - 20) % 23 != 0 {
416423
return None;
417424
}
418-
let token_in = Address::from_slice(&path[0..20]);
419-
let first_fee = u32::from_be_bytes([0, path[20], path[21], path[22]]);
420-
let token_out_start = path.len() - 20;
421-
let token_out = Address::from_slice(&path[token_out_start..]);
425+
let first_token = Address::from_slice(&path[0..20]);
426+
let leading_fee = u32::from_be_bytes([0, path[20], path[21], path[22]]);
427+
let last_token_start = path.len() - 20;
428+
let last_token = Address::from_slice(&path[last_token_start..]);
429+
let trailing_fee_start = last_token_start - 3;
430+
let trailing_fee = u32::from_be_bytes([
431+
0,
432+
path[trailing_fee_start],
433+
path[trailing_fee_start + 1],
434+
path[trailing_fee_start + 2],
435+
]);
422436
let hops = (path.len() - 20) / 23;
423-
Some((token_in, token_out, first_fee, hops))
437+
Some((first_token, last_token, leading_fee, trailing_fee, hops))
424438
}
425439

426440
/// Decodes V3_SWAP_EXACT_IN command parameters
@@ -454,7 +468,10 @@ impl UniversalRouterVisualizer {
454468

455469
// Decode V3 path. For multi-hop paths, the output token MUST be read from the
456470
// FINAL 20 bytes, not from offset 23..43 (which is the first intermediate token).
457-
let (token_in, token_out, fee, hops) = match Self::decode_v3_path(&path) {
471+
// ExactIn paths flow in execution order, so the first-execution-hop fee is the
472+
// leading fee. `trailing_fee` (the last execution hop's fee) is intentionally
473+
// discarded here, the caller surfaces only the first-hop fee for disclosure.
474+
let (token_in, token_out, fee, _trailing_fee, hops) = match Self::decode_v3_path(&path) {
458475
Some(decoded) => decoded,
459476
None => {
460477
return SignablePayloadField::TextV2 {
@@ -801,9 +818,13 @@ impl UniversalRouterVisualizer {
801818

802819
// Decode V3 path. ExactOut paths are encoded in reverse vs ExactIn per Uniswap V3
803820
// convention (see `SwapRouter.exactOutputInternal` / `Path.decodeFirstPool`): the
804-
// on-chain layout is `tokenOut || fee || ... || tokenIn`. The helper returns raw
805-
// byte positions, so we alias `first_token = tokenOut`, `last_token = tokenIn`.
806-
let (token_out, token_in, fee, hops) = match Self::decode_v3_path(&path) {
821+
// on-chain layout is `tokenOut || fee_{N-1} || ... || fee_0 || tokenIn`. The helper
822+
// returns raw byte positions, so we alias `first_token = tokenOut`,
823+
// `last_token = tokenIn`. Execution flows right-to-left from the signer's view, so
824+
// the first-execution-hop fee (the fee the signer's input pays first) is the
825+
// helper's `trailing_fee`, NOT `leading_fee`. The leading fee corresponds to the
826+
// final hop (tokenMid -> tokenOut) and is intentionally discarded for disclosure.
827+
let (token_out, token_in, _leading_fee, fee, hops) = match Self::decode_v3_path(&path) {
807828
Some(decoded) => decoded,
808829
None => {
809830
return SignablePayloadField::TextV2 {
@@ -2318,11 +2339,13 @@ mod tests {
23182339
let path = build_v3_path(&[token_in, token_out], &[3000]);
23192340
assert_eq!(path.len(), 43);
23202341

2321-
let (in_, out_, fee, hops) =
2342+
let (in_, out_, leading_fee, trailing_fee, hops) =
23222343
UniversalRouterVisualizer::decode_v3_path(&path).expect("valid single-hop path");
23232344
assert_eq!(in_, token_in);
23242345
assert_eq!(out_, token_out);
2325-
assert_eq!(fee, 3000);
2346+
assert_eq!(leading_fee, 3000);
2347+
// Single-hop path: leading and trailing fees coincide.
2348+
assert_eq!(trailing_fee, 3000);
23262349
assert_eq!(hops, 1);
23272350
}
23282351

@@ -2343,13 +2366,15 @@ mod tests {
23432366
// 20 + 2*23 = 66 bytes
23442367
assert_eq!(path.len(), 66);
23452368

2346-
let (in_, out_, fee, hops) =
2369+
let (in_, out_, leading_fee, trailing_fee, hops) =
23472370
UniversalRouterVisualizer::decode_v3_path(&path).expect("valid two-hop path");
23482371
assert_eq!(in_, token_in);
23492372
// Critical: this must be the FINAL token, not the intermediate.
23502373
assert_eq!(out_, token_out);
23512374
assert_ne!(out_, token_mid, "regression: must not return intermediate");
2352-
assert_eq!(fee, 500);
2375+
assert_eq!(leading_fee, 500);
2376+
// Distinct fees: trailing fee is the second-to-last 3 bytes of the path.
2377+
assert_eq!(trailing_fee, 3000);
23532378
assert_eq!(hops, 2);
23542379
}
23552380

@@ -2371,11 +2396,14 @@ mod tests {
23712396
// 20 + 3*23 = 89
23722397
assert_eq!(path.len(), 89);
23732398

2374-
let (in_, out_, fee, hops) =
2399+
let (in_, out_, leading_fee, trailing_fee, hops) =
23752400
UniversalRouterVisualizer::decode_v3_path(&path).expect("valid three-hop path");
23762401
assert_eq!(in_, t0);
23772402
assert_eq!(out_, t3);
2378-
assert_eq!(fee, 100);
2403+
assert_eq!(leading_fee, 100);
2404+
// Three-hop fees are [100, 500, 10000]; the trailing fee sits in the bytes
2405+
// immediately before the last token, i.e. the final entry of the fee list.
2406+
assert_eq!(trailing_fee, 10000);
23792407
assert_eq!(hops, 3);
23802408
}
23812409

@@ -2443,6 +2471,18 @@ mod tests {
24432471
subtitle.contains("2 hops"),
24442472
"subtitle should disclose hop count, got: {subtitle}"
24452473
);
2474+
// ExactIn execution flows tokenIn -> tokenMid -> tokenOut; the first
2475+
// execution hop uses the FIRST entry in the fee list (500 = 0.05%).
2476+
// Anything else would mean the signer is shown a hop fee that doesn't
2477+
// apply to the first leg.
2478+
assert!(
2479+
subtitle.contains("first fee 0.05%"),
2480+
"subtitle should disclose first-hop fee (0.05%), got: {subtitle}"
2481+
);
2482+
assert!(
2483+
!subtitle.contains("first fee 0.3%"),
2484+
"subtitle must NOT disclose the second-hop fee as the first, got: {subtitle}"
2485+
);
24462486

24472487
let expanded = preview_layout.expanded.expect("expanded present");
24482488
// Find the "Output Token" field.
@@ -2523,6 +2563,20 @@ mod tests {
25232563
subtitle.contains("2 hops"),
25242564
"subtitle should disclose hop count, got: {subtitle}"
25252565
);
2566+
// ExactOut path bytes for this fixture: [real_token_out | 10000 | token_mid
2567+
// | 100 | real_token_in]. Execution flows real_token_in -> token_mid first
2568+
// (the signer's first leg, paid out of the user's input), using fee=100 =
2569+
// 0.01%. The leading-byte fee (10000 = 1.0%) corresponds to the FINAL
2570+
// execution hop. Disclosing the leading-byte fee as "first fee" misleads
2571+
// the signer about the hop their input touches first.
2572+
assert!(
2573+
subtitle.contains("first fee 0.01%"),
2574+
"subtitle should disclose first-execution-hop fee (0.01%), got: {subtitle}"
2575+
);
2576+
assert!(
2577+
!subtitle.contains("first fee 1%"),
2578+
"subtitle must NOT disclose the final-hop fee as the first, got: {subtitle}"
2579+
);
25262580

25272581
let expanded = preview_layout.expanded.expect("expanded present");
25282582
let find_field = |label: &str| -> String {

0 commit comments

Comments
 (0)