Skip to content

feat(evm)!: add prevDepositBlock to the Deposit event - #9

Open
hubagaspar91 wants to merge 1 commit into
mainfrom
feat/evm/deposit-prev-block
Open

hubagaspar91 wants to merge 1 commit into
mainfrom
feat/evm/deposit-prev-block

Conversation

@hubagaspar91

Copy link
Copy Markdown
Collaborator

What

This PR adds the field uint256 prevDepositBlock as the last field of the Deposit event binding. The bridge upgrade from DLC-link/evm-asset-bridge#68 adds this field. The upstream contract change is DLC-link/evm-asset-bridge#69.

  • The old signature is Deposit(bytes32,bytes,address,uint256), topic0 0xacaea730…7ef8.
  • The new signature is Deposit(bytes32,bytes,address,uint256,uint256), topic0 0xf3ca8897…30e2.

DEPOSIT_EVENT_TOPIC still comes from the event, so its value changes with the event. The names Deposit and DEPOSIT_EVENT_TOPIC stay the same.

Why

The upgraded bridge emits only the new event. The old binding does not match those logs. The crate replaces the old event and does not keep both, because two deposit topics in the API would stay forever. The crate is new and has few users, so we accept the breaking change now.

Breaking change

  • The exported Deposit struct has a new field. Code that builds or decodes a Deposit must handle it.
  • DEPOSIT_EVENT_TOPIC matches only the new event.
  • The CHANGELOG.md entry gives the old topic0 as a literal. A caller that reads pre-upgrade logs uses it.

A probe test showed two facts, which the changelog states. Deposit::decode_log rejects an old log, because its topic0 differs. Old log data decodes as a Deposit without an error, but prevDepositBlock then holds the byte length of the deposit id.

No caller of DEPOSIT_EVENT_TOPIC exists in the org today. This PR does not cut a release. The next release is 0.4.0.

How tested

  • The existing topic test now pins the new topic0. It failed before the event change and passes after it.
  • cargo test, cargo clippy --all-targets --all-features -D warnings, cargo doc with -D warnings and cargo fmt --check pass.

Out of scope

  • A binding for the new lastDepositBlock view.
  • Any other change to the public API.

🤖 Generated with Claude Code

The bridge upgrade from DLC-link/evm-asset-bridge#68 adds a
prevDepositBlock field to its Deposit event. The new field changes
the event signature and its topic0, so the old binding no longer
matches the logs that the upgraded bridge emits.

The crate replaces the old event and does not keep both. Two
deposit topics in the API would stay forever, and the crate has few
users. The changelog gives the old topic0 as a literal for a caller
that reads logs from before the upgrade.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 3, 2026 19:21
@hubagaspar91

Copy link
Copy Markdown
Collaborator Author

Factory report: requirements

Done

  • I read the handoff and checked the upstream change. Number 68 in evm-asset-bridge is an issue. PR #69 implements it.
  • PR #69 declares prevDepositBlock as a non-indexed uint256 after amount. The indexed flags decide how a log decodes, so I checked them first.
  • I searched src/, tests/, examples/ and README.md. Only two places use the event: the sol! block and its topic test. src/tokens/beth/mod.rs and tests/public_surface.rs only re-export the names. No decoder and no struct literal of Deposit exist.
  • The worktree HEAD was behind origin/main, so I made the branch from origin/main.

Sizing

  • The task is small and mechanical. I skipped the product spec and the implementation spec.

Considered and rejected

  • I did not keep the old event next to the new one. The handoff rejects two deposit topics in the API.

Agents and models

  • No subagent ran. Claude Opus 5.5 at medium effort did this phase.

@hubagaspar91

Copy link
Copy Markdown
Collaborator Author

Factory report: implementation

Done

  • I changed the pinned topic0 in the_selectors_and_the_event_topic_match_the_deployed_bridge first. The test failed.
  • I added uint256 prevDepositBlock as the last field. The test passed. DEPOSIT_EVENT_TOPIC still comes from <Deposit as SolEvent>::SIGNATURE_HASH.
  • I added a doc comment on the event. It says what the field holds, with the upstream NatSpec meaning.
  • I added the "Changed — breaking" entry under [Unreleased] in CHANGELOG.md.
  • A temporary probe test checked the changelog claims. I removed the probe before the commit.
    • Deposit::decode_log_data rejects an old log with InvalidEventSignatureHash.
    • Old log data decodes as a Deposit without an error. prevDepositBlock then holds the byte length of the id.
  • cargo test, cargo clippy --all-targets --all-features -D warnings, cargo doc with -D warnings and cargo fmt --check pass.

Considered and rejected

  • I did not keep the probe as a permanent test. The handoff says to change only what the new field requires.
  • I did not add a type for the old event. The handoff rules out a second event in the API.

Agents and models

  • No subagent ran. Claude Opus 5.5 at medium effort did this phase.

@hubagaspar91

Copy link
Copy Markdown
Collaborator Author

Factory report: review and fix

Done

  • Two reviewers from two vendors read the diff. Each reviewer had read-only access. This light pass suits a two-file change.
  • Claude Sonnet 5.5 at high effort used two lenses: spec match, and security with correctness. It reported no blocking finding.
    • It checked the probe claim from the ABI layout. Word 2 of the old data is the id length, and the new layout reads that word as prevDepositBlock.
    • It reported one nit. The changelog told a caller to decode old logs "with the four-field layout", but the crate has no type for that layout.
  • Codex GPT-6.1-Sol at medium effort was the second-vendor lens. It reported no findings. It ran cargo test --locked, Clippy and cargo doc.

Fix

  • I accepted the nit. The changelog now tells a caller to declare the four-field layout in their own sol! block.

Considered and rejected

  • I did not run a Copilot review. Copilot is opt-in and costs per token.
  • I did not run more review rounds. Both reviewers agreed, and the fix changed one changelog sentence.

Human gate

  • The pilot approved the commit message. The commit b89ad15 has a good signature.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues block the Deposit binding update.

No open issues 🎉

What changed in this PR

Updates the EVM bridge’s Deposit binding to match the upgraded contract. This is a breaking API change intended for the next release, 0.4.0; this PR does not cut a release.

Changes:

  • Adds prevDepositBlock as the final event field and documents its meaning.
  • Updates the topic test to pin the new event signature.
  • Documents migration requirements, including the old topic and safe decoding of pre-upgrade logs.
File Description
src/​kits/​evm/​evm_asset_bridge.rs Extends the event binding and updates the expected deposit topic.
CHANGELOG.md Explains the breaking change and legacy-log decoding requirements.

Review effort: Balanced

@hubagaspar91

Copy link
Copy Markdown
Collaborator Author

Factory report: CI and handoff

Done

  • The first CI run failed in the Localnet suite before any test ran. The image registry returned toomanyrequests: Rate exceeded when the job pulled the canton image.
  • I ran the failed job again. All checks now pass, and the Localnet suite passes too.

Considered and rejected

  • I did not change the workflow for the rate limit. That change is outside the scope of this PR.

Agents and models

  • No subagent ran. Claude Opus 5.5 at medium effort did this phase.

Next step

  • The pilot reviews the PR and decides when it merges. The 0.4.0 release is a separate step.

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.

3 participants