node: Ignore No-Op Unsafe Reset Requests #14256
Merged
+9
−3
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #14256 +/- ##
===========================================
- Coverage 45.75% 44.52% -1.23%
===========================================
Files 1007 950 -57
Lines 86213 81544 -4669
===========================================
- Hits 39446 36310 -3136
+ Misses 43788 42425 -1363
+ Partials 2979 2809 -170
Flags with carried forward coverage won't be shown. Click here to find out more.
|
axelKingsley
force-pushed
the
interop-ignore-bad-reset
branch
2 times, most recently
from
c941638 to
c6d383b
Compare
protolambda
force-pushed
the
fix-local-safe
branch
from
c8dd1b6 to
026eacd
Compare
axelKingsley
force-pushed
the
interop-ignore-bad-reset
branch
from
c6d383b to
98cdae4
Compare
axelKingsley
changed the title
axelKingsley
force-pushed
the
interop-ignore-bad-reset
branch
from
98cdae4 to
541b2a7
Compare
protolambda
approved these changes
Feb 7, 2025
There was a problem hiding this comment.
Fine to merge as-is, the engine code is covered in the e2e. The interop edge case here we hit on devnet, and will need an e2e test too.
Opened an issue for myself to get unit-testing in place once we get rid of the engine-controller legacy in there.
#14260
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
protolambda
added
A-op-node
maurelian
pushed a commit
that referenced
this pull request
Feb 11, 2025
maurelian
pushed a commit
that referenced
this pull request
Feb 11, 2025
maurelian
pushed a commit
that referenced
this pull request
Feb 11, 2025
alcueca
pushed a commit
that referenced
this pull request
Feb 12, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 12, 2025
* fix: generate salt through uint, retrieve cahinId through bytes20+uint160 * fix: bytes20 is not necessary * fix: generated semver-lock.json * chore: lint * fix: is this what you want? * fix: regenerated semver-lock * chore: bumping version and semver-lock so that CI sees them changing at the same time. * ctb: move vm.broadcast into createDeterministic (#14243) * ctb: Use startbroadcast/stopbroadcast with createDeterministic This backports #14241 onto develop. While deploying the interop contracts we discovered a bug where nonces would incorrectly increase following calls to `createDeterministic`. This is because `vm.broadcast` will broadcast the _next_ call, but `createDeterministic` sometimes doesn't emit a call at all. This means that (in the case of the Sueprchain deployment) the calls to the input contract's `set` method would be called instead. This in turn caused nonces to increase incorrectly. To fix this issue, this PR updates the Solidity implementation to use `vm.startBroadcast()` and `vm.stopBroadcast()`. This avoids the issue altogether. To prevent further incorrect usages of `vm.broadcast` I also opened #14242 which adds a Semgrep rule to check for this case. * move vm.broadcast into createDeterministic * op-node/rollup/attributes: Add missing EIP1559Params consolidation checks (#14179) * op-node/rollup/attributes: Add missing EIP1559Params consolidation checks * all: use OptimismConfig in consolidation to translate zero attribs * TEST using default config if it cannot be fetched * op-node/node: add ChainOpConfig override and use it instead of a default config * Add ChainConfig's OptimismConfig into rollup.Config * Fix TestGetRollupConfig * revert passing OptimismConfig around it's now part of the rollup.Config * dependabot(gomod): bump github.com/kurtosis-tech/kurtosis/api/golang (#14250) Bumps [github.com/kurtosis-tech/kurtosis/api/golang](https://github.com/kurtosis-tech/kurtosis) from 1.4.3 to 1.4.4. - [Release notes](https://github.com/kurtosis-tech/kurtosis/releases) - [Changelog](https://github.com/kurtosis-tech/kurtosis/blob/main/CHANGELOG.md) - [Commits](kurtosis-tech/kurtosis@1.4.3...1.4.4) --- updated-dependencies: - dependency-name: github.com/kurtosis-tech/kurtosis/api/golang dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Added README to op-dripper (#14165) * fix(op-deployer): don't deploy dachallenge contract when using GenericCommitment (#14248) * op-supervisor: handle out-of-sync on known local-safe tip with already-known source blocks (#14252) * op-supervisor: handle out-of-sync on known local-safe tip with already-known source blocks * op-supervisor: improve logging * op-supervisor: fix fromda test, add test cases * op-supervisor: fix lint * interop: op-supervisor event metrics (#14258) * op-node,op-supervisor: share event metrics code Signed-off-by: protolambda <proto@protolambda.com> * op-node: cleanup noop metrics --------- Signed-off-by: protolambda <proto@protolambda.com> * node: Ignore No-Op Reset Requests (#14256) * op-supervisor: improve logging (#14262) * fix(devnet-sdk): use correct Address type (#14259) * chore: update kurtosis to 1.4.4 (#14255) also add kurtosis client to mise tools. * fix(devnet-sdk): use correct chain ID type (#14261) * fix(devnet-sdk): use correct Address type * fix(devnet-sdk): use correct chain ID type * ci: Log op-e2e output to files (#14257) * ci: Log op-e2e output to files * update log text * fix(devnet-sdk): ensure balances are comparable (#14267) In error cases, make sure we can still compare resulting balances without crashing. In particular, this makes interop_smoke_test skip properly if it can't find a funded wallet to use. * ci: Reduce publish job to daily (#14276) * op-deployer: add architecture diagram (#14281) * op-deployer: add architecture diagram * fix formatting * less verbose diagram description * fix: Orbifying checkout-with-mise. (#14282) * Add bootnodes ran by UL (#14279) * op-conductor: Add HeartbeatTimeout & LeaderLeaseTimeout flags (#14271) * Add MT Cannon audit reports (Coinbase and Spearbit) (#14218) * Add MT Cannon audit report * Fix MT Cannon audit report name Co-authored-by: mbaxter <meredith@oplabs.co> * Add coinbase MT Cannon audit report * Leave "subsequent release" blank for now on audit readme --------- Co-authored-by: mbaxter <meredith@oplabs.co> * feat(devnet-sdk): enable preconditions enforcement (#14268) In some use-cases, failure to meet preconditions is in itself an error. For example, when running acceptance tests against a devnet, we presumably epxect that the acceptance tests should apply. This change adds support for an environment variable DEVNET_EXPECT_PRECONDITIONS_MET=1 that will then mark as failed the tests for which the preconditions are not met by the provided system. * fix: bump patch version for mise orb. (#14290) * feat: pass args thru contracts build commands (#14287) Updates contracts justfile to pass arguments through the build commands. * op-node: fix p2p NAT option (#14280) * op-program: Update test to ensure interop bootstrapping does not load the L2 chain ID local key (#14300) * op-conductor: Add new Raft flags into optional flags list (#14299) * Update op-geth dependency to add uni-mainnet config (#14301) * op-node: Define message expiry time constant (#14296) * op-e2e: Fix fpp bls precompile tests (#14291) * op-challenger: Handle requesting superchain roots beyond the current unsafe head (#14206) * op-e2e: Introduce helpers for building transition states. * op-e2e: Add test for disputing a block prior to the proposal block * op-challenger: Handle requesting superchain roots beyond the current chain head. * Detect not found responses in prestate provider too. * Add test to confirm correct enforcement of trace extension activation. * Add more variants of tests where agreed = disputed. Cover all combinations of trace extension activating. * Remove printf * op-supervisor: Fix AddLink error type and clean up reset logic (#14302) * Fix AddLink error type ; clean up reset logic * fix tests * feat(devnet-sdk): add helper to build execution environment (#14298) * fix: add mirrored restrictions to lite profile (#14295) Adds mirrored compiler restrictions to the lite profile so that all generated artifacts get overwritten when switching between build and build-dev. * ci: make frozen files check depend on contracts-bedrock-build (#14316) This parallelizes the job with contracts-bedrock-build so that results are returned more quickly. * fix(kurtosis-devnet): make logs more readable (#14317) By wrapping the observed errors one layer too deep we were introducing escaped strings into the logs. Now multi-line and quoted strings appear as-is. * kt interop-devnet: enable super-cannon with interop-prestate (#14172) * kt-devnet: switch to geth-teku for l1 (#14305) * op-program: Ensure exec msg inclusion during consolidation (#14101) * op-program: Ensure exec msg inclusion during consolidation * implement timestamp invariants in the program * use msg expiry in rollup config * remove leftover non-working test * op-supervisor: Don't reset when node is far behind (#14324) * op-supervisor: don't reet on all errors (#14326) * feat: Ignore frozen files check by using a PR label (#14247) * feat: Ignore frozen files check by using a PR label * fix: following https://github.com/orgs/community/discussions/26712 * fix: maybe * fix: follow existing pattern * fix: temporarily remove the contracts changed check to make it run * spike: Using jq instead of grep * fix: don't extract labels twice * fix: a bit cleaner output * fix: minor change to trigger CI again * fix: Bring back the change check * feat: Docs * ci: use circleci-agent step halt to end job --------- Co-authored-by: alcueca <alberto@yield.is> Co-authored-by: Maurelian <maurelian@protonmail.ch> Co-authored-by: Maurelian <john@oplabs.co> * op-e2e: Add action test for cascading reorgs in interop fault proofs (#14266) * op-e2e: Add tests for cascading invalidations * Make InboxContract an entity in the DSL. * op-e2e: Focus test cases on consolidation. * op-e2e: Generate correct payload when executing messages. * op-e2e: Skip known failing test with reference to tracking issues. * OPCM: OPPrestateUpdater for L1 Pectra Defense (#13998) * Add a new updatePrestatehash method to the OPContractsManager * deployed * second deployment * Go back to refactor * delete file * format * make diff smaller * even smaller * missing bracket * Go back on refactoring * fix build * fix build * fix build * fix build * fix versions * add snapshots * Add a new updatePrestatehash method to the OPContractsManager * deployed * second deployment * Go back to refactor * delete file * format * make diff smaller * even smaller * missing bracket * Go back on refactoring * fix build * fix build * fix build * fix build * fix versions * fix versions * add snapshots * update snapshots & semver * correct semver * bump OPCM version * move interface to inherit IOPCM * fix interface * update versions * add specs * fix deploy opcm setters * fix deploy opcm setters * changes asked on review & semver-lock * Add non implemented tests for coverage * linting * New tests & semverlock * rewrite hasFDG boolean * semver-lock * fix review comments * pre-pr * semver * semver-lock * semverlock again * final semverlock * fix comment about versions * fix comment about versions * semverlock opcm * feat: test upgrading just the pdg vs. both games * feat: remove unused assertValidGameType() function * review changes * remove unnecessary comment * update interface to fix CI * change file format * rebase * semver lock * valid semlock * rerun ci --------- Co-authored-by: JosepBove <josep@oplabs.co> Co-authored-by: JosepBove <josep.bove.dalmases@gmail.com> Co-authored-by: JosepBove <josep@aave.com> * fix: generated semver-lock.json * chore: bumping version and semver-lock so that CI sees them changing at the same time. * chore: bump semver-lock.json * chore: bump versions and semver-lock --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: protolambda <proto@protolambda.com> Co-authored-by: alcueca <alberto@yield.is> Co-authored-by: Matthew Slipper <me@matthewslipper.com> Co-authored-by: Sebastian Stammler <seb@oplabs.co> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: ControlCplusControlV <44706811+ControlCplusControlV@users.noreply.github.com> Co-authored-by: Samuel Laferriere <samlaf92@gmail.com> Co-authored-by: protolambda <proto@protolambda.com> Co-authored-by: Axel Kingsley <axel.kingsley@gmail.com> Co-authored-by: Yann Hodique <yann@oplabs.co> Co-authored-by: Sam Stokes <35908605+bitwiseguy@users.noreply.github.com> Co-authored-by: blaine <blainemalone01@gmail.com> Co-authored-by: Joshua Gutow <jgutow@oplabs.co> Co-authored-by: Tei Im <40449056+ImTei@users.noreply.github.com> Co-authored-by: Paul Dowman <paul@pauldowman.com> Co-authored-by: mbaxter <meredith@oplabs.co> Co-authored-by: smartcontracts <kelvin@optimism.io> Co-authored-by: Adrian Sutton <adrian@oplabs.co> Co-authored-by: Inphi <mlaw2501@gmail.com> Co-authored-by: Maurelian <john@oplabs.co> Co-authored-by: Zach Howard <zach@oplabs.co> Co-authored-by: Park Changwan <pcw109550@gmail.com> Co-authored-by: Maurelian <maurelian@protonmail.ch> Co-authored-by: JosepBove <josep@oplabs.co> Co-authored-by: JosepBove <josep.bove.dalmases@gmail.com> Co-authored-by: JosepBove <josep@aave.com>
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.
When a reset signal is given, we shouldn't reset the unsafe in any case: