Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contracts-bedrock: L2ToL1MessagePasser event extension #3135

Merged
merged 4 commits into from Aug 1, 2022

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Jul 29, 2022

Description

Emit an additional event during initiateWithdrawal that includes
the withdrawal hash so that it is easy to observe changes to the
sentMessages mapping.

@changeset-bot
Copy link

changeset-bot bot commented Jul 29, 2022

🦋 Changeset detected

Latest commit: 6e272fc

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

This PR includes changesets to release 1 package
Name Type
@eth-optimism/contracts-bedrock 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-pkg-contracts-bedrock Area: packages/contracts-bedrock labels Jul 29, 2022
@maurelian
Copy link
Contributor

I guess you need this here rather than the fixes repo because of the SDK?

Copy link
Contributor

@maurelian maurelian left a comment

Choose a reason for hiding this comment

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

Should add a test for this new event.

@tynes tynes force-pushed the fix/contracts-event-extension branch from 7f23f19 to 55e195f Compare August 1, 2022 21:24
@mergify
Copy link
Contributor

mergify bot commented Aug 1, 2022

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

@mergify mergify bot added the conflict label Aug 1, 2022
Emit an additional event during `initiateWithdrawal` that includes
the withdrawal hash so that it is easy to observe changes to the
`sentMessages` mapping.
@tynes tynes force-pushed the fix/contracts-event-extension branch from 55e195f to 97e7345 Compare August 1, 2022 21:26
@mergify mergify bot removed the conflict label Aug 1, 2022
@tynes tynes force-pushed the fix/contracts-event-extension branch from 97e7345 to 5eb0f6f Compare August 1, 2022 21:36
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

please keep the specs in sync

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.

go code's good. Adding to the specs would be nice as well.

op-node/withdrawals/utils.go Outdated Show resolved Hide resolved
@tynes tynes force-pushed the fix/contracts-event-extension branch from e805294 to 6e272fc Compare August 1, 2022 21:57
@tynes
Copy link
Contributor Author

tynes commented Aug 1, 2022

Not sure why the e2e tests hang when running in the cloud, they work fine locally

edit: nvm seems like a flake or something, they worked now

@mergify
Copy link
Contributor

mergify bot commented Aug 1, 2022

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

@tynes tynes merged commit 20a9fe4 into develop Aug 1, 2022
@tynes tynes deleted the fix/contracts-event-extension branch August 1, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pkg-contracts-bedrock Area: packages/contracts-bedrock
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants