op-node: Use unmetered L1 Attributes Transaction by trianglesphere · Pull Request #3157 · ethereum-optimism/optimism

trianglesphere · 2022-08-02T15:20:27Z

Description
This enables the IsSystemTransaction flag in the L1 Attributes
deposit & updates to the latest version of geth.

changeset-bot · 2022-08-02T15:20:30Z

🦋 Changeset detected

Latest commit: 1a543cf

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

This PR includes changesets to release 9 packages

Name	Type
@eth-optimism/contracts-bedrock	Patch
@eth-optimism/core-utils	Patch
@eth-optimism/sdk	Patch
@eth-optimism/common-ts	Patch
@eth-optimism/contracts	Patch
@eth-optimism/data-transport-layer	Patch
@eth-optimism/drippie-mon	Patch
@eth-optimism/message-relayer	Patch
@eth-optimism/replica-healthcheck	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

mergify · 2022-08-02T15:21:02Z

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

This enables the IsSystemTransaction flag in the L1 Attributes deposit & updates to the latest version of geth.

protolambda

Can you update the specs?

deposits doc needs the system tx format bool: https://github.com/ethereum-optimism/optimism/blob/develop/specs/deposits.md

and derivation doc needs to set it to true for block info, false for user deposits: https://github.com/ethereum-optimism/optimism/blob/develop/specs/derivation.md

specs/deposits.md

protolambda

need to rerun CI, op-e2e seems to have failed to start / output anything.

mslipper

CI is flagging a real bug, so requesting change until we fix it.

protolambda · 2022-08-03T04:42:42Z

Ah, I think this breaks the deposit hash calculation in TS (used in devnet test), because of the new deposit systemTx field.

The op-e2e timing out happens sometimes in CI though, I think that's unrelated.

protolambda · 2022-08-03T14:49:38Z

The devnet ethereumoptimism/reference-optimistic-geth:latest image may still need to be updated. We switched from optimism-prototype to optimism branch as default in the reference-optimistic-geth repo. Both to keep the audit branch nice and stable, as well as to get rid of the -prototype suffix: it's not a prototype anymore, we're about to enter alpha public testnet.

protolambda · 2022-08-03T16:57:20Z

Cancelled CI, devnet was running for 2h, we should add a timeout on that job. I think we just need to update the L2 reference geth docker image to point to the new branch that this PR introduces.

protolambda · 2022-08-03T18:27:55Z

Geth repo nows builds docker images for the new default optimism branch, including the updates required for this PR. I don't think the devnet is using a specific docker tag, but just uses the latest, so other CI might temporarily break until we merge this in.

tynes · 2022-08-03T18:38:25Z

We need to merge this because its breaking the devnet for other PRs

protolambda · 2022-08-03T18:41:04Z

I think either the CI is running an older docker image, or my JS change is not good, I'm seeing this in CI logs:

Sending from 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Depositing 1 ETH to 0xB79f76EF2c5F0286176833E7B2eEe103b1CC3244
0xB79f76EF2c5F0286176833E7B2eEe103b1CC3244 has 0.0 ETH on L2
Got TX hash 0x4b6f7e960d69276ef9c9a431e8acf27b8ced8a01a6b83bf8b61ec3f518a9e025. Waiting...
Included in block 0xc00acaa964fcdfa65490c20b2c92c6baeadcf28efdb710fd041e27403a26e102
Deposit has log index 1
Waiting for L2 TX hash 0x67251c1f210dd63a82eb58aaf2f0317f66bc72aec1a9fddd8b35834ab6b61743
Unexpected balance increase without detecting deposit transaction
latest block 211:0x7c59d94d829089088090d3acb3d64d4c4b839798eb2e441266ad84715766d50a
Unexpected balance increase without detecting deposit transaction
latest block 236:0x965047b4588d06a40b7c52f7d1682bc36c727ee02fdb3dd8a1b20de0fafe93d4

Which I think is because of the depsit-tx hash computation not matching between Go and JS. cc @tynes

packages/core-utils/src/optimism/deposit-transaction.ts

tynes · 2022-08-03T18:47:48Z

I think either the CI is running an older docker image, or my JS change is not good, I'm seeing this in CI logs:

Sending from 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Depositing 1 ETH to 0xB79f76EF2c5F0286176833E7B2eEe103b1CC3244
0xB79f76EF2c5F0286176833E7B2eEe103b1CC3244 has 0.0 ETH on L2
Got TX hash 0x4b6f7e960d69276ef9c9a431e8acf27b8ced8a01a6b83bf8b61ec3f518a9e025. Waiting...
Included in block 0xc00acaa964fcdfa65490c20b2c92c6baeadcf28efdb710fd041e27403a26e102
Deposit has log index 1
Waiting for L2 TX hash 0x67251c1f210dd63a82eb58aaf2f0317f66bc72aec1a9fddd8b35834ab6b61743
Unexpected balance increase without detecting deposit transaction
latest block 211:0x7c59d94d829089088090d3acb3d64d4c4b839798eb2e441266ad84715766d50a
Unexpected balance increase without detecting deposit transaction
latest block 236:0x965047b4588d06a40b7c52f7d1682bc36c727ee02fdb3dd8a1b20de0fafe93d4

Which I think is because of the depsit-tx hash computation not matching between Go and JS. cc @tynes

This means the JS isn't computing the hash of the deposit tx correctly

protolambda · 2022-08-03T18:48:25Z

Just checked the CI logs. Pulled down the exact docker image that is being used, and grepped the symbols in the geth binary to make sure it includes the latest updates, and it does. So it's some code in our testing that can't find the deposit correctly. Likely JS/solidity updates I missed.

protolambda · 2022-08-03T19:40:20Z

Devnet stuck due to *remaining fixes, cancelled CI job because it didn't time out automatically (1h...)

protolambda · 2022-08-04T12:41:05Z

Fixed a bug in deposit tx decoding (it was still skipping an extra byte, from when deposits had a version byte) + fixed encoding/decoding of the new boolean.

…and encoding to handle isSystemTransaction bool

unmetered fixes + more tests

contracts-bedrock: fix test

mergify · 2022-08-04T16:37:05Z

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

mergify · 2022-08-04T16:37:15Z

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

* op-node: Use unmetered L1 Attributes Transaction This enables the IsSystemTransaction flag in the L1 Attributes deposit & updates to the latest version of geth. * specs updates * Update specs/deposits.md * feat: bedrock deposit transaction type update * bedrock: update geth dependency * fix(core-utils): bedrock deposit tx encode/decode typescript fixes * feat(packages/contracts-bedrock): update UserDepositTransaction type and encoding to handle isSystemTransaction bool * contracts-bedrock: update differential deposit tx solidity <> js fuzzing * core-utils * contracts-bedrock * contracts-bedrock: fix test * contracts-bedrock: fix differential tests * contracts-bedrock: fix broken test * contracts-bedrock: gas snapshot Co-authored-by: protolambda <proto@protolambda.com> Co-authored-by: Mark Tyneway <mark.tyneway@gmail.com>

trianglesphere added the mergify-ignore label Aug 2, 2022

trianglesphere requested review from protolambda and tynes and removed request for protolambda August 2, 2022 15:20

trianglesphere force-pushed the jg/unmetered_l1_attributes branch from 007ab5f to 9b7aecb Compare August 2, 2022 15:23

op-node: Use unmetered L1 Attributes Transaction

ab50558

This enables the IsSystemTransaction flag in the L1 Attributes deposit & updates to the latest version of geth.

trianglesphere force-pushed the jg/unmetered_l1_attributes branch from 9b7aecb to ab50558 Compare August 2, 2022 20:08

trianglesphere removed the mergify-ignore label Aug 2, 2022

trianglesphere requested a review from mslipper August 2, 2022 20:22

protolambda suggested changes Aug 2, 2022

View reviewed changes

specs updates

6e165a6

protolambda reviewed Aug 2, 2022

View reviewed changes

specs/deposits.md Show resolved Hide resolved

trianglesphere commented Aug 2, 2022

View reviewed changes

specs/deposits.md Show resolved Hide resolved

Update specs/deposits.md

c3df078

protolambda approved these changes Aug 2, 2022

View reviewed changes

mslipper approved these changes Aug 3, 2022

View reviewed changes

mslipper requested changes Aug 3, 2022

View reviewed changes

feat: bedrock deposit transaction type update

e8a9bf7

github-actions bot added the A-pkg-core-utils Area: packages/core-utils label Aug 3, 2022

bedrock: update geth dependency

b2ffcd3

tynes reviewed Aug 3, 2022

View reviewed changes

packages/core-utils/src/optimism/deposit-transaction.ts Show resolved Hide resolved

fix(core-utils): bedrock deposit tx encode/decode typescript fixes

78f3338

feat(packages/contracts-bedrock): update UserDepositTransaction type …

5750bf5

…and encoding to handle isSystemTransaction bool

github-actions bot added 2-reviewers A-pkg-contracts-bedrock Area: packages/contracts-bedrock labels Aug 4, 2022

contracts-bedrock: update differential deposit tx solidity <> js fuzzing

52757be

protolambda requested a review from mslipper August 4, 2022 13:29

tynes and others added 9 commits August 4, 2022 07:48

core-utils

e5aadac

contracts-bedrock

79dc795

Merge pull request #3168 from ethereum-optimism/tynes/unmetered

b9be438

unmetered fixes + more tests

contracts-bedrock: fix test

0078dd7

Merge pull request #3169 from ethereum-optimism/fix/unmetered

4c925fc

contracts-bedrock: fix test

contracts-bedrock: fix differential tests

7317b47

contracts-bedrock: fix broken test

3786830

contracts-bedrock: gas snapshot

a72214b

Merge branch 'develop' into jg/unmetered_l1_attributes

1a543cf

mslipper approved these changes Aug 4, 2022

View reviewed changes

mergify bot added the on-merge-train label Aug 4, 2022

mergify bot merged commit 8ae3915 into develop Aug 4, 2022

mergify bot deleted the jg/unmetered_l1_attributes branch August 4, 2022 16:37

mergify bot removed the on-merge-train label Aug 4, 2022

mslipper mentioned this pull request Aug 4, 2022

Develop -> Master #3170

Merged

Conversation

trianglesphere commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Aug 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

mergify bot commented Aug 2, 2022

Uh oh!

protolambda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

protolambda left a comment

Choose a reason for hiding this comment

Uh oh!

mslipper left a comment

Choose a reason for hiding this comment

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

tynes commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

Uh oh!

tynes commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022

Uh oh!

protolambda commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

protolambda commented Aug 4, 2022

Uh oh!

mergify bot commented Aug 4, 2022

Uh oh!

mergify bot commented Aug 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

trianglesphere commented Aug 2, 2022 •

edited

Loading

changeset-bot bot commented Aug 2, 2022 •

edited

Loading

protolambda commented Aug 3, 2022 •

edited

Loading