Skip to content

ctp: Remove ERC721Refunded events#3522

Merged
mergify[bot] merged 2 commits intodevelopfrom
ctp/rm-refund-event
Sep 22, 2022
Merged

ctp: Remove ERC721Refunded events#3522
mergify[bot] merged 2 commits intodevelopfrom
ctp/rm-refund-event

Conversation

@maurelian
Copy link
Contributor

Description

Simplifies branching logic in the ERC721 bridge by removing the ERC20Refunded event.

This has the downside of making it slightly more difficult for indexers to differentiate between a transfer from Bob to Alice vs. a refund resulting from a failed transfer from Alice to Bob. However:

  1. This is most likely to happen in case where we don't care about one of the two tokens.
  2. A proper indexer should be watching both bridges, and can detect the ERC721BridgeFailed event.

Tests

I removed a couple tests that became redundant after removing this functionality.

Additional context

Yay for code deletion.

@changeset-bot
Copy link

changeset-bot bot commented Sep 21, 2022

🦋 Changeset detected

Latest commit: 60ba0e5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@eth-optimism/contracts-periphery Patch
@eth-optimism/drippie-mon Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added 2-reviewers A-op-bindings Area: op-bindings labels Sep 21, 2022
@maurelian maurelian requested a review from tynes September 21, 2022 20:47
@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 3883f34 into develop Sep 22, 2022
@mergify mergify bot deleted the ctp/rm-refund-event branch September 22, 2022 16:28
@mergify mergify bot removed the on-merge-train label Sep 22, 2022
mslipper pushed a commit that referenced this pull request Sep 22, 2022
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-op-bindings Area: op-bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants