Skip to content

test(integration): replace watcher with sdk CrossChainMessenger#2159

Merged
smartcontracts merged 1 commit intoethereum-optimism:developfrom
jgresham:replace_watcher_with_sdk_#2055
Feb 9, 2022
Merged

test(integration): replace watcher with sdk CrossChainMessenger#2159
smartcontracts merged 1 commit intoethereum-optimism:developfrom
jgresham:replace_watcher_with_sdk_#2055

Conversation

@jgresham
Copy link
Copy Markdown
Contributor

@jgresham jgresham commented Feb 7, 2022 •
edited
Loading

Description
Removes all remaining usage of the Watcher from the integration tests.

Additional context
Makes it possible to entirely remove the Watcher in a following commit.

Metadata

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 7, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: a8a74a9

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

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 the A-integration Area: integration tests label Feb 7, 2022
@smartcontracts smartcontracts mentioned this pull request Feb 7, 2022
Closed
smartcontracts
smartcontracts reviewed Feb 7, 2022
View reviewed changes
integration-tests/test/native-eth-ovm-calls.spec.ts Outdated

const value = ethers.utils.parseEther('0.01')
await fundUser(env.watcher, env.l1Bridge, value, wallet.address)
await fundUser(env.messenger, env.l1Bridge, value, wallet.address)
Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts Feb 7, 2022

Choose a reason for hiding this comment

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

Note that fundUser has already been patched to use the SDK in a recent commit, you may need to make a second PR or rebase.

@jgresham jgresham force-pushed the replace_watcher_with_sdk_#2055 branch from 4a94610 to f0117cd Compare February 8, 2022 21:08
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 9, 2022 •
edited
Loading

Codecov Report

Merging #2159 (a5239f9) into develop (57c66bb) will decrease coverage by 0.79%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2159      +/-   ##
===========================================
- Coverage    71.37%   70.57%   -0.80%     
===========================================
  Files           89       89              
  Lines         3032     3028       -4     
  Branches       516      509       -7     
===========================================
- Hits          2164     2137      -27     
- Misses         868      891      +23
Flag Coverage Δ
batch-submitter 62.63% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 62.08% <ø> (ø)
data-transport-layer 37.74% <ø> (ø)
message-relayer 70.86% <ø> (ø)
sdk 59.97% <ø> (-3.44%) ⬇️

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

Impacted Files Coverage Δ
packages/sdk/src/adapters/standard-bridge.ts 28.39% <0.00%> (-22.75%) ⬇️
packages/sdk/src/adapters/eth-bridge.ts 53.84% <0.00%> (-16.37%) ⬇️
packages/sdk/src/cross-chain-messenger.ts 72.00% <0.00%> (+0.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57c66bb...a5239f9. Read the comment docs.

@jgresham jgresham force-pushed the replace_watcher_with_sdk_#2055 branch from 1f92d38 to 988db20 Compare February 9, 2022 03:20
@jgresham
Copy link
Copy Markdown
Contributor Author

jgresham commented Feb 9, 2022 •
edited
Loading

@smartcontracts I took most the changes from 3535fb2

There was one conflict with l1Messenger and messenger initialization co-dependency. I tried to fix here by taking a piece from Watcher, please take a look here https://github.com/ethereum-optimism/optimism/pull/2159/files#diff-ddcad9599e1f38130f84505d2605b25aa8b5ef273c02d7244fa2162481c06ba8R86
cc @mslipper because I see you made changes here recently

const l1MessengerAddress = await addressManager.getAddress(
    'Proxy__OVM_L1CrossDomainMessenger'
)
const l1Messenger = getContractFactory('L1CrossDomainMessenger')
      .connect(l1Wallet)
      // .attach(watcher.l1.messengerAddress)
      .attach(l1MessengerAddress)

@jgresham jgresham changed the title WIP: test(integration): replace watcher with sdk CrossChainMessenger test(integration): replace watcher with sdk CrossChainMessenger Feb 9, 2022
@jgresham jgresham force-pushed the replace_watcher_with_sdk_#2055 branch from a5239f9 to a8a74a9 Compare February 9, 2022 12:40
@jgresham
Copy link
Copy Markdown
Contributor Author

jgresham commented Feb 9, 2022

I think I came across a race condition in an itest, which failed on a previous push, here https://github.com/ethereum-optimism/optimism/blob/develop/integration-tests/test/ovmcontext.spec.ts#L149

@smartcontracts
Copy link
Copy Markdown
Contributor

smartcontracts commented Feb 9, 2022

I think I came across a race condition in an itest, which failed on a previous push, here https://github.com/ethereum-optimism/optimism/blob/develop/integration-tests/test/ovmcontext.spec.ts#L149

Hmmm yeah this is a known problem. We should fix that test. I'll make an issue for it today.

@smartcontracts
Copy link
Copy Markdown
Contributor

smartcontracts commented Feb 9, 2022

Great work here! One step closer to deleting the Watcher entirely!

@smartcontracts smartcontracts merged commit 5a3753d into ethereum-optimism:develop Feb 9, 2022
theochap added a commit that referenced this pull request Dec 10, 2025
@theochap
test(node/e2e-devstack): deprecate p2p tests in devnet-sdk (#2159)
b752af8 
## Description

Deprecate p2p tests in `devnet-sdk` now we have full-feature equivalence
inside the `devstack`.

Adds a finalized sync test.

Close #2100
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

Assignees

No one assigned

Labels

A-integration Area: integration tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace Watcher with new SDK in integration tests

3 participants

@jgresham @codecov-commenter @smartcontracts