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

CanonicalTransactionChain and L1ToL2TransactionQueue #139

Merged
merged 25 commits into from
May 23, 2020

Conversation

K-Ho
Copy link
Contributor

@K-Ho K-Ho commented May 20, 2020

Description

Adds

  1. RollupQueue contract which handles enqueueing and dequeuing of timestamped transactions (for both L1ToL2TransactionQueue and later the SlowTransactionQueue)
  2. L1ToL2TransactionQueue contract which limits enqueueing to the l1ToL2TransactionPasser contract and limits dequeuing to the CanonicalTransactionChain contract.
  3. CanonicalTransactionChain contract that allows batches of transactions to be added either directly from the sequencer or by moving transactions over from one of the transaction queues. It also allows for the verification of the position of a single transaction in the chain.

Questions

  • Let me know your thoughts on naming! Definitely room for improvement on that front

Fixes

  • Fixes #YAS-373, YAS-371, YAS-374, YAS-375

Contributing Agreement

@K-Ho K-Ho changed the title [WIP] CanonicalTransactionChain and L1ToL2TransactionQueue CanonicalTransactionChain and L1ToL2TransactionQueue May 22, 2020
Copy link
Collaborator

@ben-chain ben-chain left a comment

Choose a reason for hiding this comment

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

Woohoo!! Left a few small comments, but this LGTM!


contract CanonicalTransactionChain {
address public sequencer;
uint public sequencerLivenessAssumption;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know we've used this term in passing before, but a name more related to queueing might be good here as "liveness" is overloaded in crypto generally. forceInclusionPeriod? Idk hahaha

Comment on lines +60 to +68
bytes32 batchHeaderHash = keccak256(abi.encodePacked(
timestamp,
true, // isL1ToL2Tx
elementsMerkleRoot,
numElementsInBatch,
cumulativeNumElements // cumulativePrevElements
));
batches.push(batchHeaderHash);
cumulativeNumElements += numElementsInBatch;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noting that this seems like a reasonable place for an appendBatch(time, isL1, numElements) function, seems like this functionality would be shared in a few places

await l1ToL2TxQueue
.enqueueBlock(block)
.enqueueTx(defaultTx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW there is a TestUtils.assertRevertsAsync which does this and auto-apends VM Exception while processing transaction: revert :)

)
})

it('should rever if submitting a 10 minute old batch', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
it('should rever if submitting a 10 minute old batch', async () => {
it('should revert if submitting a batch older than the liveness assumption', async () => {

it('should update cumulativeNumElements correctly', async () => {
await appendBatch(DEFAULT_BATCH)
const cumulativeNumElements = await canonicalTxChain.cumulativeNumElements.call()
cumulativeNumElements.toNumber().should.equal(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
cumulativeNumElements.toNumber().should.equal(2)
cumulativeNumElements.toNumber().should.equal(DEFAULT_BATCH.length)

@K-Ho K-Ho merged commit 6ab9d83 into master May 23, 2020
@gakonst gakonst deleted the YAS-374/RollupChain/BatchHeader branch March 18, 2021 15:01
teddyknox pushed a commit to teddyknox/optimism that referenced this pull request Oct 3, 2023
bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024
ClaytonNorthey92 added a commit to hemilabs/optimism that referenced this pull request Dec 9, 2024
c9dbc5f58 Test fix & fmt
fcf735497 ExternalHeaderMode rebase
f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159)
5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186)
1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181)
383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178)
6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177)
9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175)
a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173)
939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174)
9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139)
159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163)
60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169)
fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168)
65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167)
8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162)
67166046b scripts: add release script (ethereum-optimism#164)
eea96059b ci: add labeler action (ethereum-optimism#165)

git-subtree-dir: heminetwork
git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1
OptimismBot pushed a commit that referenced this pull request Jan 16, 2025
* feat: integrate portal to lockbox

* fix: pr fixes

* test: refactor assert
blockchaindevsh pushed a commit to blockchaindevsh/optimism that referenced this pull request Jan 25, 2025
mslipper added a commit that referenced this pull request Feb 6, 2025
* feat: add shared lockbox (#126)

* feat: create shared lockbox contract with its interface and unit tests

* chore: polish tests and interfaces

* chore: run pre-pr

* chore: improve natspec

* chore: run pre-pr

* chore: update compiler version

* feat: integrate portal to lockbox (#139)

* feat: integrate portal to lockbox

* fix: pr fixes

* test: refactor assert

* feat: add liquidity migrator contract with its unit test and interface (#128)

* feat: create shared lockbox contract with its interface and unit tests

* chore: polish tests and interfaces

* chore: run pre-pr

* chore: improve natspec

* chore: run pre-pr

* feat: add liqudity migrator contract with its unit test and interface

* chore: remove underscore on stack var

* chore: add todo

* chore: run pre-pr

* chore: add contract title natspec and proxied

* refactor: integrate testing suite with common test

* chore: pre-pr

* chore: add spec test

* feat: integrate system config with superchain config (#140)

* feat: integrate portal to lockbox

* fix: pr fixes

* test: refactor assert

* feat: integrate system config with superchain config

* fix: remove OPCM interop

* test: add dependency counter test

* feat: manage dependency set on superchain config (#138)

* chore: add zero dependencies check (#142)

* fix: pre pr

* feat: Add pause check (#145)

* feat: Add pause check

Co-authored-by: 0xParticle <[email protected]>
Co-authored-by: gotzenx <[email protected]>
Co-authored-by: Joxess <[email protected]>

* test: add tests natspecs

---------

Co-authored-by: 0xParticle <[email protected]>
Co-authored-by: gotzenx <[email protected]>
Co-authored-by: Joxess <[email protected]>

* fix: pre pr and interfaces imports

* feat: add upgrader role to superchain config (#163)

* feat: use superchain config lockbox in portal (#164)

* feat: use superchain config lockbox in portal

* test: add new sharedlockbox test

* fix: pre pr

* feat: liquidity migrator deployment (#166)

* feat: liquidity migrator deployment

* test: fix comment

* test: fix internal variables names

* feat: dependency set refactor (#170)

* feat: dependency set refactor

* fix: deploy script variable name

* fix: pr

* fix: pr

* fix: pre pr

* fix: semgrep

* fix: merge conflict

* [WIP] feat: new lockbox (#192)

* chore: partial implementation comments

* feat: new lockbox

* feat: introduce dependency manager predeploy

* feat: remove timestamp check from CrossL2Inbox

* feat: introduce cluster manager role and remove immutables

* fix: remove unnecessary code, fix tests and setup

* feat: use unstructured storage and OZ v5

* fix: L2ToL2CDM dependency set check

* fix: dependency manager gas limit

* feat: refactor interop feature contracts (#200)

* feat: refactor interop feature contracts

* fix: add noops comment

* feat: adds OptimismPortal migrated flag

* test: add missing tests

* fix: portal interop storage naming

---------

Co-authored-by: Skeletor Spaceman <[email protected]>

* fix: pre pr, setup and tests

* fix: remove system config interop and add interop portal target check (#205)

* fix: remove system config interop and add interop portal check

* fix: interop portal target check order

* fix: remove wrong comment

* fix: refactor portal noops function (#206)

* test: add dependency manager and portal interop tests (#209)

* feat: initialize shared lockbox in interop portal (#211)

* feat: initialize shared lockbox in interop portal

* fix: refactor shared lockbox storage getter

* fix: lockbox pr fixes (#214)

* fix: pre pr

* fix: semver lock

* fix: semver lock

* feat: descope dependency manager (#242)

* feat: descope dependency manager

* test: fix tests

* test: fix tests

* chore: improve eth liquidity test (#248)

* fix: internal review fixes (#243)

* fix: I-0

* fix: I-1

* fix: I-2

* fix: I-3

* fix: I-6

* fix: I-7

* fix: I-9

* fix: pre pr

* fix: pre pr

* fix: portal withdrawal checks (#255)

* fix: portal withdrawal checks

* fix: include current withdrawal check

* fix: remove unused interop contracts (#256)

* test: fix flake tests (#257)

* fix: adjust op-deployer interop scripts (#262)

* fix: pre pr

* fix op-deployer tests

* remove dependency code

* fix lint

* use Bob account

---------

Co-authored-by: Disco <[email protected]>
Co-authored-by: AgusDuha <[email protected]>
Co-authored-by: agusduha <[email protected]>
Co-authored-by: 0xParticle <[email protected]>
Co-authored-by: gotzenx <[email protected]>
Co-authored-by: Joxess <[email protected]>
Co-authored-by: Skeletor Spaceman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants