Skip to content

feat: withdrawal migration#3528

Merged
mergify[bot] merged 3 commits intodevelopfrom
feat/xdm-migrate
Oct 18, 2022
Merged

feat: withdrawal migration#3528
mergify[bot] merged 3 commits intodevelopfrom
feat/xdm-migrate

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented Sep 22, 2022 •
edited
Loading

Description

Implement withdrawal hash migration functions with tests. The tests reuse the receipts in op-chain-ops/crossdomain/testdata.
The branch coverage does hit both the finalizeETHWithdrawal case and the other case.

To really know if this migration code works, we need to do end to end tests.

Closes ENG-2743

@changeset-bot
Copy link

changeset-bot bot commented Sep 22, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: 305cbb6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@mergify
Copy link
Contributor

mergify bot commented Sep 22, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@tynes tynes mentioned this pull request Sep 22, 2022
Merged
@tynes tynes force-pushed the feat/xdm-withdrawals branch from a14b2fd to 57dd3f9 Compare September 25, 2022 20:16
@mergify
Copy link
Contributor

mergify bot commented Sep 25, 2022

Hey @tynes! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Sep 25, 2022
Base automatically changed from feat/xdm-withdrawals to develop September 25, 2022 20:34
@mergify mergify bot removed the conflict label Sep 25, 2022
@tynes tynes force-pushed the feat/xdm-migrate branch 2 times, most recently from a05bada to cf861f6 Compare October 3, 2022 20:18
@tynes tynes mentioned this pull request Oct 4, 2022
Merged
@tynes tynes force-pushed the feat/xdm-migrate branch from 8950fe7 to c05e489 Compare October 6, 2022 23:00
@tynes tynes force-pushed the feat/xdm-migrate branch from c05e489 to 499925b Compare October 9, 2022 02:23
@tynes tynes marked this pull request as ready for review October 9, 2022 02:26
trianglesphere
trianglesphere reviewed Oct 11, 2022
View reviewed changes
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

Code is pretty good. A lot of my questions are about making sure that I have enough context about the system.

trianglesphere
trianglesphere approved these changes Oct 13, 2022
View reviewed changes
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

lgtm

smartcontracts
smartcontracts reviewed Oct 17, 2022
View reviewed changes
@mergify
Copy link
Contributor

mergify bot commented Oct 18, 2022

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

@mergify mergify bot merged commit 9803573 into develop Oct 18, 2022
@mergify mergify bot deleted the feat/xdm-migrate branch October 18, 2022 22:37
@mergify
Copy link
Contributor

mergify bot commented Oct 18, 2022

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

@mergify mergify bot removed the on-merge-train label Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smartcontracts smartcontracts smartcontracts approved these changes

+1 more reviewer

@trianglesphere trianglesphere trianglesphere approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tynes @trianglesphere @smartcontracts