optimism icon indicating copy to clipboard operation
optimism copied to clipboard

Directly test bedrock v0 encoding and hashing against legacy implementations

Open maurelian opened this issue 2 years ago • 3 comments

Description

Although it may seem obvious, we were not directly testing that Bedrock's V0 hashing and encoding schemes match those of the legacy system.

Tests

Two tests:

  1. testFuzz_encodeCrossDomainMessage_matchesLegacy_succeeds
  2. testFuzz_hashCrossDomainMessageV0_matchesLegacy_succeeds

maurelian avatar Feb 16 '23 15:02 maurelian

⚠️ No Changeset found

Latest commit: fd31fc823875e34fa89a358567861dbe6b027633

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

changeset-bot[bot] avatar Feb 16 '23 15:02 changeset-bot[bot]

Perhaps there is non determinism with weth9 because its pulling in the weth9 from the legacy contracts repo?

tynes avatar Feb 16 '23 17:02 tynes

If this causes build issues by pulling in the dep, I recommend we just copy/paste the function in with a comment including a hyperlink to the git commit that it was copy pasted from, since we are going to delete the contracts package eventually

tynes avatar Feb 17 '23 02:02 tynes

Codecov Report

Merging #4901 (fd31fc8) into develop (c10214b) will increase coverage by 0.00%. The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4901   +/-   ##
========================================
  Coverage    41.03%   41.03%           
========================================
  Files          336      337    +1     
  Lines        20481    20482    +1     
  Branches       771      771           
========================================
+ Hits          8404     8405    +1     
- Misses       11436    11438    +2     
+ Partials       641      639    -2     
Flag Coverage Δ
bedrock-go-tests 36.62% <ø> (+<0.01%) :arrow_up:
contracts-bedrock-tests 49.87% <0.00%> (-0.07%) :arrow_down:
contracts-tests 98.86% <ø> (ø)
core-utils-tests 60.41% <ø> (ø)
dtl-tests 47.15% <ø> (ø)
fault-detector-tests 33.88% <ø> (ø)
sdk-tests 38.74% <ø> (ø)

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

Impacted Files Coverage Δ
...ock/contracts/libraries/LegacyCrossDomainUtils.sol 0.00% <0.00%> (ø)
op-node/heartbeat/service.go 57.89% <0.00%> (+2.63%) :arrow_up:

codecov[bot] avatar Mar 08 '23 18:03 codecov[bot]

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

mergify[bot] avatar Mar 13 '23 15:03 mergify[bot]