feat: cherry-pick combo adaptors on opbnb v0.5.2-alpha.1 #39
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* op-chain-ops: decouple from bindings Another commit that removes bindings from `op-chain-ops`. The entire repo is very close to being completely decoupled from having a global set of bindings. This does have a little bit of code duplication, but it generally results in a more simple layout. Both the `crossdomain` and `upgrades` package end up with their own `bindings` package. The `crossdomain` package is mostly legacy but the `indexer` still depends on it. The `indexer` is in maintenance mode, after reth exex it makes a lot of sense to build indexers directly into reth. The `crossdomain` package uses the bindings to migrate and serialize L1 to L2 cross domain messages. These bindings never need to be updated. The `upgrades` package has a justfile added that can be used to regenerate the bindings. This makes it easier to generate bindings based on a specific release. Previously the `upgrades` bindings were coupled in a strange way and backwards compatibility hacks needed to exist. Now the team working on upgrades can update the bindings however they want so the proper upgrade can easily be generated. * upgrades: error case Since the AddressList doesn't have the `DisputeGameFactory` yet, it cannot be pulled into the upgrade scripts. Instead return an error so that we MUST update it. https://github.com/ethereum-optimism/superchain-registry/blob/110e744c97b4873384ad2da365c281639fc0668e/superchain/superchain.go#L194
…t labels (ethereum-optimism#10481) If the collateral for a DelayedWETH contract is sufficient and then becomes insufficient, we need to zero out the metric with the sufficient label and set the values on the insufficient labelled version.
…0485) Completed games weren't updating their latest acted block because they don't get scheduled which meant the metric always reported 0.
Updates a single import to use the `op-proposer` bindings instead of the global monorepo bindings. The big consumer of the bindings after this are `op-e2e` and some tests in `op-service`.
This moves the ERC20 bindings that the `op-service` tests depend on into the `test` package that already exists. This ensures that these bindings can always exist safely and be used as part of the tests.
Updates another import to reference local bindings rather than global bindings. This test isn't going to need to worry about having its bindings be updated.
Don't apply the conf depth if l1Head is empty (as it is during the startup case before the l1State is initialized)
* ctb: Test ownership setup scripts * ctb: Add nested config of modules * Add guard and module checks * Update packages/contracts-bedrock/scripts/DeployOwnership.s.sol Co-authored-by: Matt Solomon <[email protected]> --------- Co-authored-by: Matt Solomon <[email protected]>
* contracts-bedrock: add deposit event coverage Adds deposit event differential coverage for the two different places where the deposit tx event exists. This ensures that a valid event ends up getting emitted. Also modularizes the code for serializing the event so that there are not footguns when computing the "opaque data". * contracts-bedrock: revert diff to portal Removes the diff to the portal, perhaps we want it but test still passes without it * contracts-bedrock: lint * test: improve * tests: update * tests: fixup * semver: fix * lint: fix
…ptimism#10465) * op-node/rollup: Add MaxSequencerDrift to ChainSpec * op-node/rollup,op-e2e: Use spec max seq drift instead of config * op-node/rollup: Showcase feature/fork separation pattern with seq drift change * op-node/driver: add origin selector Fjord test * op-node/rollup: refactor batch validation test prepare to allow for general modification of rollup config * op-node/rollup: add batch validation test * Update op-node/rollup/types.go Co-authored-by: Joshua Gutow <[email protected]> * op-node/rollup/derive: add Fjord span batch validation test --------- Co-authored-by: Joshua Gutow <[email protected]>
Updates the weth predeploy to be versioned. Even though the weth predeploy is not proxied, adding a version safely lets us make small modifications to it in the future such as updating the version of `solc` that is used to compile it and be able to be aware offchain of the version that the chain has without needing to keep a registry of codehashes.
* op-node: Generic Commitment This is a generic commitment to be used by the op-node & op-batcher. * abstract commitments to CommitmentData interface * correct byte-stripping ; add tests ; finish wiring * make GenericCommitment always verify * correct action tests * PR comments * fix unit test * remove fmt.Println --------- Co-authored-by: Joshua Gutow <[email protected]>
…ism#10451) * feat(op-dispute-mon): L2BlockNumberChallenged dispute monitor support feat(op-dispute-mon): query for the l2 block number through the game metadata call fix(op-dispute-mon): query for the l2 block number through the game metadata call fix(op-dispute-mon): query for the l2 block number through the game metadata call * fix(op-dispute-mon): block challenge check * fix(op-dispute-mon): use agreement and metrice * op-dispute-mon: Separate l2 challenge metric (ethereum-optimism#10483) * op-dispute-mon: Consider l2 block number challenged when forecasting but don't make it a new game status. Add a separate metric to report the number of successful L2 block number challenges. * op-challenger: Remove unused blockNumChallenged field from list-games info struct * op-dispute-mon: Consider l2 block challenger in expected credits. (ethereum-optimism#10484) * feat(op-dispute-mon): Update L2 Challenges Metrics (ethereum-optimism#10492) * fix: change the l2 challenges to tha gauge vec * fix: consistent metric labels --------- Co-authored-by: refcell <[email protected]> --------- Co-authored-by: Adrian Sutton <[email protected]>
Migrates the `op-bindings/predeploys` package to `op-service/predeploys`. There is various other "system related" code there. This unblocks fully deleting `op-bindings` as a top level package.
…m#10514) * cannon: Add units tests around hint writes * cannon: Update hint write fuzz tests to randomize target memory * cannon: Fix inverted conditional check * cannon: Clean up unit test - remove panic helper * cannon: Add error handling to fuzz test randomBytes helper
* feat(ctb): Enforce maximum L2 gas limit Enforces a maximum L2 gas limit within the `SystemConfig`. This change helps ensure that OP Stack chain governors keep the L2 block gas limit within a reasonable range in order to guarantee that the L2 blocks may be proven. * In the `_setResourceConfig` function, the new minimum gas limit is checked to be less than the current `gasLimit`. This value may never be larger than `MAX_GAS_LIMIT`, per the checks in `_setGasLimit`. This ensures that the `minimumGasLimit <= maximumGasLimit` * Update SystemConfig.t.sol Co-authored-by: Matt Solomon <[email protected]> --------- Co-authored-by: Matt Solomon <[email protected]>
* op-chain-ops: bindings decouple More small bindings decoupling. After this PR is merged, it should be possible to move `op-bindings/bindings` into `op-e2e/bindings` * dead-code: cleanup * build: fix
* cannon: Handle div by zero in MIPS.sol * fix revert stmt style
…mism#10515) * ctb: Change Safe contract version to GnosisSafe v1.3.0 This aligns with what the Security Council is running * ctb: Update snapshots and semver-lock
…ism#10429) * feat(ctb): Support `OptimismPortal2` in kontrol tests Adds a second `DeploymentSummary` to `kontrol` for fault proofs, and ports the existing `OptimismPortal` proofs with the `OptimismPortal2`. * summary tests * Update packages/contracts-bedrock/test/kontrol/README.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * tests * Update `check-snapshots` * Update packages/contracts-bedrock/test/kontrol/README.md Co-authored-by: Matt Solomon <[email protected]> * Separate snapshot gen --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Matt Solomon <[email protected]>
* wip * wip * fix * fix * fix * fix * address some of the bots comments * use version bit of 1 * fix lint * adding compression type * update batch reader * abstract span channel compressor * test and singular batch compressor * fix * lint * move channel compressor as interface * add base class * fix go mod * test fixes * address comments * fix * fix * revert channel builder test * revert ratio compressor test * add checks to accept brotli only post fjord * revemo unnecessary in test * fix forge-std * gofmt * address comments * remove methods in compressor * fix error msg * add compression algo flag to optional flags * add Clone() function --------- Co-authored-by: Roberto Bayardo <[email protected]>
Prepare for v0.5.0 release
feat: handle sequencer recover related logic
feat: EIP-7702 adaption
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
312
to
430
| _isFinalizationPeriodElapsed(provenWithdrawal.timestamp), | ||
| "OptimismPortal: proven withdrawal finalization period has not elapsed" | ||
| ); | ||
|
|
||
| // Grab the OutputProposal from the L2OutputOracle, will revert if the output that | ||
| // corresponds to the given index has not been proposed yet. | ||
| Types.OutputProposal memory proposal = l2Oracle.getL2Output(provenWithdrawal.l2OutputIndex); | ||
|
|
||
| // Check that the output root that was used to prove the withdrawal is the same as the | ||
| // current output root for the given output index. An output root may change if it is | ||
| // deleted by the challenger address and then re-proposed. | ||
| require( | ||
| proposal.outputRoot == provenWithdrawal.outputRoot, | ||
| "OptimismPortal: output root proven is not the same as current output root" | ||
| ); | ||
|
|
||
| // Check that the output proposal has also been finalized. | ||
| require( | ||
| _isFinalizationPeriodElapsed(proposal.timestamp), | ||
| "OptimismPortal: output proposal finalization period has not elapsed" | ||
| ); | ||
|
|
||
| // Check that this withdrawal has not already been finalized, this is replay protection. | ||
| require(finalizedWithdrawals[withdrawalHash] == false, "OptimismPortal: withdrawal has already been finalized"); | ||
|
|
||
| // Mark the withdrawal as finalized so it can't be replayed. | ||
| finalizedWithdrawals[withdrawalHash] = true; | ||
|
|
||
| // Set the l2Sender so contracts know who triggered this withdrawal on L2. | ||
| // This acts as a reentrancy guard. | ||
| l2Sender = _tx.sender; | ||
|
|
||
| // Trigger the call to the target contract. We use a custom low level method | ||
| // SafeCall.callWithMinGas to ensure two key properties | ||
| // 1. Target contracts cannot force this call to run out of gas by returning a very large | ||
| // amount of data (and this is OK because we don't care about the returndata here). | ||
| // 2. The amount of gas provided to the execution context of the target is at least the | ||
| // gas limit specified by the user. If there is not enough gas in the current context | ||
| // to accomplish this, `callWithMinGas` will revert. | ||
| bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data); | ||
| bool success; | ||
| (address token,) = gasPayingToken(); | ||
| if (token == Constants.ETHER) { | ||
| // Trigger the call to the target contract. We use a custom low level method | ||
| // SafeCall.callWithMinGas to ensure two key properties | ||
| // 1. Target contracts cannot force this call to run out of gas by returning a very large | ||
| // amount of data (and this is OK because we don't care about the returndata here). | ||
| // 2. The amount of gas provided to the execution context of the target is at least the | ||
| // gas limit specified by the user. If there is not enough gas in the current context | ||
| // to accomplish this, `callWithMinGas` will revert. | ||
| success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.value, _tx.data); | ||
| } else { | ||
| // Cannot call the token contract directly from the portal. This would allow an attacker | ||
| // to call approve from a withdrawal and drain the balance of the portal. | ||
| if (_tx.target == token) revert BadTarget(); | ||
|
|
||
| // Only transfer value when a non zero value is specified. This saves gas in the case of | ||
| // using the standard bridge or arbitrary message passing. | ||
| if (_tx.value != 0) { | ||
| // Update the contracts internal accounting of the amount of native asset in L2. | ||
| _balance -= _tx.value; | ||
|
|
||
| // Read the balance of the target contract before the transfer so the consistency | ||
| // of the transfer can be checked afterwards. | ||
| uint256 startBalance = IERC20(token).balanceOf(address(this)); | ||
|
|
||
| // Transfer the ERC20 balance to the target, accounting for non standard ERC20 | ||
| // implementations that may not return a boolean. This reverts if the low level | ||
| // call is not successful. | ||
| IERC20(token).safeTransfer({ to: _tx.target, value: _tx.value }); | ||
|
|
||
| // The balance must be transferred exactly. | ||
| if (IERC20(token).balanceOf(address(this)) != startBalance - _tx.value) { | ||
| revert TransferFailed(); | ||
| } | ||
| } | ||
|
|
||
| // Make a call to the target contract only if there is calldata. | ||
| if (_tx.data.length != 0) { | ||
| success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, 0, _tx.data); | ||
| } else { | ||
| success = true; | ||
| } | ||
| } | ||
|
|
||
| // Reset the l2Sender back to the default value. | ||
| l2Sender = Constants.DEFAULT_L2_SENDER; | ||
|
|
||
| // All withdrawals are immediately finalized. Replayability can | ||
| // be achieved through contracts built on top of this contract | ||
| emit WithdrawalFinalized(withdrawalHash, success); | ||
|
|
||
| // Reverting here is useful for determining the exact gas cost to successfully execute the | ||
| // sub call to the target contract if the minimum gas limit specified by the user would not | ||
| // be sufficient to execute the sub call. | ||
| if (success == false && tx.origin == Constants.ESTIMATION_ADDRESS) { | ||
| revert("OptimismPortal: withdrawal failed"); | ||
| revert GasEstimation(); | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Reentrancy vulnerabilities Medium
Comment on lines
+142
to
+220
| if (_clockExtension.raw() > _maxClockDuration.raw()) revert InvalidClockExtension(); | ||
|
|
||
| GAME_TYPE = _gameType; | ||
| ABSOLUTE_PRESTATE = _absolutePrestate; | ||
| MAX_GAME_DEPTH = _maxGameDepth; | ||
| SPLIT_DEPTH = _splitDepth; | ||
| GAME_DURATION = _gameDuration; | ||
| CLOCK_EXTENSION = _clockExtension; | ||
| MAX_CLOCK_DURATION = _maxClockDuration; | ||
| VM = _vm; | ||
| WETH = _weth; | ||
| ANCHOR_STATE_REGISTRY = _anchorStateRegistry; | ||
| L2_CHAIN_ID = _l2ChainId; | ||
| } | ||
|
|
||
| /// @notice Receive function to allow the contract to receive ETH. | ||
| receive() external payable { } | ||
| /// @inheritdoc IInitializable | ||
| function initialize() public payable virtual { | ||
| // SAFETY: Any revert in this function will bubble up to the DisputeGameFactory and | ||
| // prevent the game from being created. | ||
| // | ||
| // Implicit assumptions: | ||
| // - The `gameStatus` state variable defaults to 0, which is `GameStatus.IN_PROGRESS` | ||
| // - The dispute game factory will enforce the required bond to initialize the game. | ||
| // | ||
| // Explicit checks: | ||
| // - The game must not have already been initialized. | ||
| // - An output root cannot be proposed at or before the starting block number. | ||
|
|
||
| // INVARIANT: The game must not have already been initialized. | ||
| if (initialized) revert AlreadyInitialized(); | ||
|
|
||
| // Grab the latest anchor root. | ||
| (Hash root, uint256 rootBlockNumber) = ANCHOR_STATE_REGISTRY.anchors(GAME_TYPE); | ||
|
|
||
| // Should only happen if this is a new game type that hasn't been set up yet. | ||
| if (root.raw() == bytes32(0)) revert AnchorRootNotFound(); | ||
|
|
||
| // Set the starting output root. | ||
| startingOutputRoot = OutputRoot({ l2BlockNumber: rootBlockNumber, root: root }); | ||
|
|
||
| // Revert if the calldata size is not the expected length. | ||
| // | ||
| // This is to prevent adding extra or omitting bytes from to `extraData` that result in a different game UUID | ||
| // in the factory, but are not used by the game, which would allow for multiple dispute games for the same | ||
| // output proposal to be created. | ||
| // | ||
| // Expected length: 0x7A | ||
| // - 0x04 selector | ||
| // - 0x14 creator address | ||
| // - 0x20 root claim | ||
| // - 0x20 l1 head | ||
| // - 0x20 extraData | ||
| // - 0x02 CWIA bytes | ||
| assembly { | ||
| if iszero(eq(calldatasize(), 0x7A)) { | ||
| // Store the selector for `BadExtraData()` & revert | ||
| mstore(0x00, 0x9824bdab) | ||
| revert(0x1C, 0x04) | ||
| } | ||
| } | ||
|
|
||
| // Do not allow the game to be initialized if the root claim corresponds to a block at or before the | ||
| // configured starting block number. | ||
| if (l2BlockNumber() <= rootBlockNumber) revert UnexpectedRootClaim(rootClaim()); | ||
|
|
||
| // Set the root claim | ||
| claimData.push( | ||
| ClaimData({ | ||
| parentIndex: type(uint32).max, | ||
| counteredBy: address(0), | ||
| claimant: gameCreator(), | ||
| bond: uint128(msg.value), | ||
| claim: rootClaim(), | ||
| position: ROOT_POSITION, | ||
| clock: LibClock.wrap(Duration.wrap(0), Timestamp.wrap(uint64(block.timestamp))) | ||
| }) | ||
| ); | ||
|
|
||
| // Set the game as initialized. | ||
| initialized = true; |
Check warning
Code scanning / Slither
Reentrancy vulnerabilities Medium
| ClaimData storage subgameRootClaim = claimData[_claimIndex]; | ||
|
|
||
| // Fetch the parent of the subgame root's clock, if it exists. | ||
| Clock parentClock; |
Check warning
Code scanning / Slither
Uninitialized local variables Medium
Comment on lines
+96
to
+102
| Claim _disputed, | ||
| uint256 _challengeIndex, | ||
| Claim _claim, | ||
| bool _isAttack | ||
| ) | ||
| public | ||
| payable |
Check warning
Code scanning / Slither
Dangerous usage of `tx.origin` Medium
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
add a description of your changes here...
Rationale
tell us why we need these changes...
Example
add an example CLI or API response...
Changes
Notable changes: