Skip to content

fix: preserve bytes32 address width in auction observer - #246

Open
Kewe63 wants to merge 1 commit into
t1protocol:canaryfrom
Kewe63:fix/245-decode-bytes32-addresses
Open

Kewe63 wants to merge 1 commit into
t1protocol:canaryfrom
Kewe63:fix/245-decode-bytes32-addresses

Conversation

@Kewe63

@Kewe63 Kewe63 commented Sep 19, 2026

Copy link
Copy Markdown

Description

  • Decode bytes32-encoded Ethereum addresses by removing exactly the 12-byte ABI padding.
  • Apply the fixed-width conversion to sender, recipient, inputToken, outputToken, and destinationSettler.
  • Add a real-observer regression test covering leading-zero and all-zero 20-byte addresses.

Motivation and Context

ViemIntentObserver used Viem's trim() for address-valued bytes32 fields. trim() removes every leading zero byte, including bytes that belong to the 20-byte address. A valid token such as 0x00000000000000000000000000000000000000a1 therefore became 0xa1 before reaching the auction service, preventing exact token-address matching.

The new helper uses sliceHex(value, 12) to discard only the ABI padding and preserve the full address width.

Fixes #245.

Open PR #223 also modifies the observer for recovery behavior but does not address fixed-width address decoding. It currently reports mergeable: false against canary, and a pre-push merge-tree check conflicts in ViemIntentObserver.ts; this PR remains independently based on current canary.

How Has This Been Tested?

  • Full serialization audit on current canary reproduced 0x0000…00a1 becoming 0xa1; ordinary-address, ABI tuple/event, and EIP-712 controls passed.
  • Pre-fix regression test — 0 passed, 1 failed with expected full sender address vs received 0xa1.
  • bun test test/ViemIntentObserver.address-decoding.test.ts — 1 passed, 0 failed, 5 assertions after the fix.
  • Full package suite after the fix — 2 passed, 3 failed. The same three unrelated failures occur on the clean audited tree: two stale WebSocket price fixtures raise Invalid argument type in ToBigInt operation, and processIntentLogs.test.ts checks an asynchronous notification before it occurs.
  • bun x tsc -p tsconfig.json --noEmit — blocked by the same pre-existing ABI event typing, WebSocket response typing, missing vitest, and stale test-signature errors on clean canary; no new diagnostic references the added regression test or fixed-width conversion.
  • git diff --cached --check — passed.
  • Independent code review — passed with no blocking correctness, security, typing, or hidden-regression findings.

Types of changes (remove all unchecked types)

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I assessed the documentation impact; no documentation update is required.
  • I assessed the deployment-script impact; no deployment script is changed.
  • I added regression coverage for every address-valued field decoded by this observer path.

Remove exactly the ABI padding from address-valued bytes32 fields so valid leading-zero addresses reach auction matching unchanged.\n\nFixes t1protocol#245.
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.

ViemIntentObserver truncates valid leading-zero token addresses

1 participant