fix(sdk): Fix multicall3 support for sdk #6053

roninjin10 · 2023-06-17T01:33:57Z

fixes multicall3 withdrawals for sdk
I have tests for this in feat: Implement multiwithdrawals #6024 passing locally but they fail ci for unrelated reasons
- they are failing because the packages I yarn installed broke other packages
- I plan on fixing this in it's own seperate pr via upgrading some packages and implementing no-hoisting for affected packages to make them less fragile

changeset-bot · 2023-06-17T01:34:01Z

🦋 Changeset detected

Latest commit: 19e7059

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/sdk	Minor
@eth-optimism/chain-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

netlify · 2023-06-17T01:34:02Z

✅ Deploy Preview for opstack-docs canceled.

Name	Link
🔨 Latest commit	`19e7059`
🔍 Latest deploy log	https://app.netlify.com/sites/opstack-docs/deploys/649e13716cdbae0007069e0c

roninjin10 · 2023-06-17T01:34:09Z

Current dependencies on/for this PR:

develop
- PR fix(sdk): check for both v0 and v1 messages when looking for RelayedMessages #6042
  - PR fix(sdk): Fix multicall3 support for sdk #6053 👈

This comment was auto-generated by Graphite.

packages/sdk/src/cross-chain-messenger.ts

codecov · 2023-06-17T02:15:06Z

Codecov Report

Merging #6053 (186b1d1) into develop (13c710c) will decrease coverage by 1.35%.
The diff coverage is 36.76%.

❗ Current head 186b1d1 differs from pull request most recent head 11c4569. Consider uploading reports for the commit 11c4569 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6053      +/-   ##
===========================================
- Coverage    44.76%   43.41%   -1.35%     
===========================================
  Files          450      344     -106     
  Lines        29506    26300    -3206     
  Branches       691      369     -322     
===========================================
- Hits         13208    11418    -1790     
+ Misses       15219    13891    -1328     
+ Partials      1079      991      -88

Flag	Coverage Δ
bedrock-go-tests	`43.57% <ø> (-0.11%)`	⬇️
cannon-go-tests	`?`
common-ts-tests	`?`
contracts-bedrock-tests	`?`
core-utils-tests	`?`
fault-detector-tests	`26.95% <ø> (ø)`
sdk-next-tests	`42.36% <36.76%> (-0.12%)`	⬇️
sdk-tests	`42.36% <36.76%> (-0.12%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/sdk/src/cross-chain-messenger.ts	`51.52% <36.76%> (-0.53%)`	⬇️

... and 112 files with indirect coverage changes

packages/sdk/src/cross-chain-messenger.ts

packages/sdk/test/cross-chain-messenger.spec.ts

hamdiallam

LGTM!

1 comment about the added function & getMessagesByTransaction

packages/sdk/src/cross-chain-messenger.ts

hamdiallam · 2023-06-27T19:15:57Z

also we may want to lift the added comments to the functions to the @param annotation

tynes

Generally looks good to me

mergify · 2023-06-29T02:02:38Z

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

mul gas by 1.5 lint delete dead code changeset fix: gas estimate without decimals lint again fix: Fix the allowance bug that happens if you don't override the signer lint again fix: Use isSigner instead fix: missing a ?

mergify · 2023-06-29T23:41:19Z

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

mergify · 2023-06-29T23:41:21Z

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

roninjin10 requested a review from a team as a code owner June 17, 2023 01:33

mergify bot added the sdk label Jun 17, 2023

roninjin10 changed the base branch from develop to willc/prebedrockbug June 17, 2023 01:36

roninjin10 force-pushed the willc/multiwithdraw-no-test branch from 11db633 to 3bc03dc Compare June 17, 2023 01:36

roninjin10 mentioned this pull request Jun 17, 2023

fix(sdk): check for both v0 and v1 messages when looking for RelayedMessages #6042

Merged

roninjin10 force-pushed the willc/multiwithdraw-no-test branch from 3bc03dc to ca4a5a7 Compare June 17, 2023 01:38

roninjin10 commented Jun 17, 2023

View reviewed changes

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

roninjin10 commented Jun 17, 2023

View reviewed changes

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

roninjin10 force-pushed the willc/multiwithdraw-no-test branch 2 times, most recently from 89b7ee7 to 30710c3 Compare June 17, 2023 01:53

annieke mentioned this pull request Jun 21, 2023

CrossChainMessenger: Support multiple withdrawals #5983

Closed

tynes reviewed Jun 21, 2023

View reviewed changes

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

tynes reviewed Jun 21, 2023

View reviewed changes

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

tynes reviewed Jun 21, 2023

View reviewed changes

packages/sdk/test/cross-chain-messenger.spec.ts Outdated Show resolved Hide resolved

zainbacchus mentioned this pull request Jun 27, 2023

Relayer Can't Find Multi-Withdrawal Transactions ethereum-optimism/superchain-relayer#12

Closed

hamdiallam reviewed Jun 27, 2023

View reviewed changes

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

packages/sdk/src/cross-chain-messenger.ts Outdated Show resolved Hide resolved

tynes approved these changes Jun 28, 2023

View reviewed changes

roninjin10 force-pushed the willc/prebedrockbug branch from 79c1ecd to 89ca741 Compare June 29, 2023 02:02

roninjin10 requested review from a team as code owners June 29, 2023 02:02

roninjin10 requested review from felipe-op and smartcontracts and removed request for a team June 29, 2023 02:02

mergify bot added the conflict label Jun 29, 2023

roninjin10 force-pushed the willc/multiwithdraw-no-test branch from f050230 to 6e71d6d Compare June 29, 2023 02:03

mergify bot removed the conflict label Jun 29, 2023

Base automatically changed from willc/prebedrockbug to develop June 29, 2023 02:28

roninjin10 force-pushed the willc/multiwithdraw-no-test branch from 9e8faf1 to 40994ef Compare June 29, 2023 12:20

Will Cory added 10 commits June 29, 2023 16:23

fix(sdk): Fix multicall3 support for sdk

f5758b1

mul gas by 1.5 lint delete dead code changeset fix: gas estimate without decimals lint again fix: Fix the allowance bug that happens if you don't override the signer lint again fix: Use isSigner instead fix: missing a ?

test: Update error tests

ab9a16d

changeset

ff57745

test: Better tests

352a105

fix: catch out of bounds errors

28ad99f

messageIndex

b80fc6e

annotated return type

ad1c906

make unnecessary function private instead of public

26e0d12

linter

f0e1f90

chore: Remove unnecesary abstraction

11c4569

roninjin10 force-pushed the willc/multiwithdraw-no-test branch from 6350c27 to 11c4569 Compare June 29, 2023 23:23

hamdiallam approved these changes Jun 29, 2023

View reviewed changes

add a comment about the cleanup

19e7059

OptimismBot merged commit e4ca19e into develop Jun 29, 2023

OptimismBot deleted the willc/multiwithdraw-no-test branch June 29, 2023 23:41

mergify bot added the on-merge-train label Jun 29, 2023

mergify bot removed the on-merge-train label Jun 29, 2023

This was referenced Jul 18, 2023

Version Packages #6337

Closed

Version Packages #6340

Merged

Version Packages #6357

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sdk): Fix multicall3 support for sdk #6053

fix(sdk): Fix multicall3 support for sdk #6053

roninjin10 commented Jun 17, 2023 •

edited

Loading

changeset-bot bot commented Jun 17, 2023 •

edited

Loading

netlify bot commented Jun 17, 2023 •

edited

Loading

roninjin10 commented Jun 17, 2023 •

edited

Loading

codecov bot commented Jun 17, 2023 •

edited

Loading

hamdiallam left a comment

hamdiallam commented Jun 27, 2023

tynes left a comment

mergify bot commented Jun 29, 2023

mergify bot commented Jun 29, 2023

mergify bot commented Jun 29, 2023

fix(sdk): Fix multicall3 support for sdk #6053

fix(sdk): Fix multicall3 support for sdk #6053

Conversation

roninjin10 commented Jun 17, 2023 • edited Loading

changeset-bot bot commented Jun 17, 2023 • edited Loading

🦋 Changeset detected

netlify bot commented Jun 17, 2023 • edited Loading

✅ Deploy Preview for opstack-docs canceled.

roninjin10 commented Jun 17, 2023 • edited Loading

codecov bot commented Jun 17, 2023 • edited Loading

Codecov Report

hamdiallam left a comment

Choose a reason for hiding this comment

hamdiallam commented Jun 27, 2023

tynes left a comment

Choose a reason for hiding this comment

mergify bot commented Jun 29, 2023

mergify bot commented Jun 29, 2023

mergify bot commented Jun 29, 2023

roninjin10 commented Jun 17, 2023 •

edited

Loading

changeset-bot bot commented Jun 17, 2023 •

edited

Loading

netlify bot commented Jun 17, 2023 •

edited

Loading

roninjin10 commented Jun 17, 2023 •

edited

Loading

codecov bot commented Jun 17, 2023 •

edited

Loading