-
Notifications
You must be signed in to change notification settings - Fork 645
Run execution-spec-tests in nightly CI #3577
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3577 +/- ##
==========================================
- Coverage 22.66% 22.62% -0.05%
==========================================
Files 388 388
Lines 58907 58907
==========================================
- Hits 13354 13325 -29
- Misses 43506 43535 +29
Partials 2047 2047 🚀 New features to boost your workflow:
|
da1b059
to
acbd0e2
Compare
I wasn't too sure who to assign, I checked the history of who reviewed the last few nightly CI PR's and it has been Pepper so I requested him |
I tested both the failure and success case running in CI and both cases are behaving as I would expect |
echo "Devnode started with PID $NODE_PID" | ||
cd .. | ||
|
||
# Give the devnode to initialize if needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: # Give the devnode time to initialize ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied
32d931c
to
32d3438
Compare
* Close part files after the copy is done * fix: resolve race condition in storage test goroutines (OffchainLabs#3509) Co-authored-by: Pepper Lebeck-Jobe <[email protected]> * Fix relay backlog corruption at segment boundaries (OffchainLabs#3516) There was a bug where if a duplicate message was processed at a segment boundary, then it would cause an empty segment to be inserted, which broke the invariants of the backlog data structure. The invariants being violated wer: 1. Monotonic sequence number order 2. Segment Continuity: segment[n].End() + 1 == segment[n+1].Start() 3. Non-empty Segments: All segments should contain messages 4. Lookup Uniqueness: Each sequence number maps to exactly one segment 5. Cumulative Size Ordering: Later messages have higher cumulative sizes This bug would break most operations on the backlog (eg Get() which uses a binary search) and cause them to have unpredictable behavior. * Revert "Disable HTTP/2 for das aggregator by default" (OffchainLabs#3517) * Optimize signer * Return a fatal error instead of stopping the StopWaiter in case of ErrGlobalStateNotInChain * Return a fatal error instead of stopping the StopWaiter in case of ErrGlobalStateNotInChain * fix(backend): fix error handling and add comprehensive tests (OffchainLabs#3510) * fix(backend): fix error handling in GetCollectMachineHashes * test(backend): add comprehensive test coverage for GetCollectMachineHashes * fix test * test parallel batch posters with and without redis lock use * Add 10 minutes to bold tests (OffchainLabs#3514) For some reason, this is frequently timing out on CI builds. The builds and tests are passing locally, so, I assume the reason the CI jobs are failing is a timeout. The logs are far too verbose to understand if there is some actual failure in CI. Also, this commits some updates to the mod files generated by `go get -v -t ./...` and updates the workflow not to pass the now-deprecated `-d` option to `go get` * cleanup batch poster segments on lock acquiry failure * add batch poster handoff test * Don't log error if unable to return expired nonce cache failure (OffchainLabs#3486) * Don't log error if unable to return expired nonce cache failure When a nonce failure cache entry expires, don't error if unable to return original error to user because there is a good chance that user already disconnected * Remove code duplication * Add multi-dimensional gas collector (OffchainLabs#3453) * Add multi-deminsional gas collector * Enhance Multigas collector with StopWaiter * Multigas collector start and finalise block separation * Collect multigas in `ProduceBlockAdvanced` * Strat Multigas collector from execution engine * Re-wire multigas collector and apply review fixes * Multigas collector review fixes * Move Multigas collector from arbos to execution and some review fixes * Split Multigas collector interface from implementation * Add system test for multigas collector and add copyrights * Move multigas collector start/stop block message to ExecutionEngine * Fixup multigas collector comments * support both miniredis and redis version of batch poster handoff test * prevent nil deref when reading from queue in data paster * don't parallellise batch poster redis tests when using single external redis * Refactor move multigascollector to arbos (OffchainLabs#3524) This allows docker buildx build nitro-node-dev . to build. The reason is that that docker image builds the replay.wasm with strictly the directories that are supposed to contain code which affects the state transition function. When the multigascollector package was in execution, it pulled in all of the dependencies of that go package. So, things got a bit upside-down. * Add debug flag to enable deep copy of backlog msgs (OffchainLabs#3519) Co-authored-by: Aman Sanghi <[email protected]> * remove empty duplicate of TestBidValidatorAuctioneerRedisStream * add IsSharedTestRedisInstance helper * add comment to AttemptLock call * Update geth post merge of v1.16.0 (OffchainLabs#3523) * Update geth pin post geth merge v1.16.0 * update geth pin * fix CI * copy ancients in db conversion test * code cleanup * update geth pin * fix ci * TestGettingState is hashScheme specific so skip it in pathDB test runs * Consider arbos version for batch calldata gas calculation * Consider arbos version for batch calldata gas calculation * Update go-ethereum version * attempt locking redislock just before PostTransaction * shorten batch poster handoff test * cleanup newline * discard closed batch in defer as it cannot be reused * remove no longer needed line * Update geth post merge of v1.16.1 (OffchainLabs#3528) * Use the parent chain for BlobFeePerByte instead of an RPC call The value can be calculated from the configuration of the parent chain and shouldn't require an RPC call. Fixes: NIT-3335 * fix: correct error aggregation for multi-target compile results * Account multi-gas in StartTxHook (OffchainLabs#3533) * Instrument StartTxHook with multigas * And add new calldata multi gas types to collector --------- Co-authored-by: Pepper Lebeck-Jobe <[email protected]> * Add GetNativeTokenManagementFrom precompile to ArbOwnerPublic (OffchainLabs#3534) * Add the GetNativeTokenManagementFrom precompile This precompile is available in ArbosVersion_50 on the ArbOwnerPublic contract. * Add the implementation of GetNativeTokenManagementFrom This commit also allows arbos to initialize to arbos 50. * Update precompiles pin * Allow same prevTimestamp in eth_simulateV1 (OffchainLabs#3535) * Allow same prevTimestamp in eth_simulateV1 * fix test * Return False When Checking if Deposit Tx Type Has Poster Costs (OffchainLabs#3536) * return false for deposit tx type * test * Add arb-owner methods to get and set the calldata price * Applied new pricing only for STF * Removed unnecessary methods * Update geth post merge of v1.16.2 (OffchainLabs#3537) * Added arbos version in the l1pricing state * Updated go-ethereum version * Removed getL1CalldataPrice from ArbOwner * Reverted contracts hash * Removed arbos version check from getter * Fixed compilation error * Updated precompiles version * Fix test * Fix test * Improve CPU performance when processing blocks (OffchainLabs#3541) Pull the changes from OffchainLabs/go-ethereum#519 Close NIT-3769 * Added tests * Nit 3568 fix eip 2537 in nitro enable it as part of arbos 50 dia (OffchainLabs#3492) * NIT-3568: Fix EIP-2537 in Nitro (enable it as part of ArbOS 50 Dia) * Updated submodule reference * fixup! NIT-3568: Fix EIP-2537 in Nitro (enable it as part of ArbOS 50 Dia) * Update go-ethereum version * Extracted contract/address adding logic * Added Osaka precompiles * Update go-ethereum pin --------- Co-authored-by: Pepper Lebeck-Jobe <[email protected]> * Setting arbos version on opening the l1 pricing state * Fixed test for arbos version = 0 * Fixed tests * Addressed review comments * Fixed linting error * Fixed linting error * Fixed test * Support passing multi gas by value (OffchainLabs#3543) * Skip block size cap for arbitrum chains (OffchainLabs#3544) * Skip block size cap for arbitrum chains * RecordBlockCreation receives list of wasm targets * Only retrieve StylusArchs after spawner was started * Includes StylusArchs from redisValidator * Adds missing local target in ArbitratorSpawner * ValidationInputsAt provides the targets that needs to be recorded * Renames targets to wasmTargets * Fix comment * Don't compile TargetWasm * TestValidationInputsAtWithWasmTarget * Always adds local target before NewMessageCommitContext call * Remove rust nightly dependency * Implement EIP-7825 (OffchainLabs#3545) Implement EIP-7825 register new precompiles * CustomDA core Arbitrator & WASM infra CustomDA is a new Nitro feature that allows for 3rd parties to use their own logic data availability logic including storing, retrieving, validating, and proving without needing to modify Nitro code or the stock contracts. This is the first PR and it concerns adding the core validation and proving infrastructure. This commit adds the new WAVM opcode ValidateCertificate, and modifies the existing WAVM opcode ReadPreImage to add handling for the new CustomDA preimage type. The ValidateCertificate opcode will always be called before ReadPreImage by the replay binary. It will always skip calling ReadPreImage if ValidateCertificate returns false. When there is a dispute on a ValidateCertificate opcode there are two possibilities. The certificate posted to the parent chain might be invalid, but a malicious validator claims that it is valid, or it might be valid but a malicious validator claims it is invalid. Having ValidateCertificate as a separate opcode not part of ReadPreImage allows for us to narrow down disputes to either being about the validity of certificate, or about if the data matches the ceritficate. This makes the proofs individually smaller, especially in the case of the ceritifcate validity proof. When used by the block validator in reexecution mode, for CustomDA preimages ValidateCertificate simply checks that the preimage for the certificate was included by the block validator and returns valid if so, otherwise it returns invalid. The block validator does its own validation of the certificate by calling the 3rd party daprovider and only includes it if it is valid. For non CustomDA preimages it always returns valid. In reexecution mode ReadPreImage doesn't need any modification, since the certificate was already validated by the replay binary calling ValidateCertificate and therefore the preimage should be available. In proving mode for CustomDA preimage types, these opcodes both return an incomplete proof that needs enhancement by the Nitro process which is orchestrating the challenge. To indicate that the proof needs enhancement they OR 0x80 with the machine status byte of the proof, this is safe becasue the MSB of the first byte doesn't overlap with any of the existing machine statuses (0x00-0x03). The final byte of the proof is used as a marker to indicate the type enhancment needed: 0xDA is for CustomDA ReadPreImage proofs, and 0xDB is for CustomDA ValidateCertificate proofs. The final byte so the enhancer doesn't need to scan through the data to find the marker, it just looks at the final byte, and then can know how to interpret prior bytes depending on the type. Data needed by the enhancer from the opcode is returned via the proof. The enhancer in Nitro will call the 3rd party daprovider to get the rest of the proof. This commit contains the plumbing for the new and modified opcode to be used from the replay binary. ValidateCertificate: ``` wavmio.ValidateCertificate (wavmio/higher.go) ↓ wavmio.validateCertificate (wavmio/raw.go - stub/import declaration) ↓ [WASM boundary] wavmio__validateCertificate (wasm-libraries/host-io/src/lib.rs) ↓ wavm_validate_certificate (C FFI declaration) ↓ [Runtime resolution] WavmValidateCertificate (arbitrator/prover/src/host.rs) ↓ [Mapped to WAVM instruction] ValidateCertificate (arbitrator/prover/src/machine.rs - instruction handler) ``` ReadPreImage: ``` (existing path using wavmio.ResolveTypedPreimage) ↓ wavmio__resolveTypedPreimage (wasm-libraries/host-io/src/lib.rs) ↓ wavm_read_customda_preimage (C FFI declaration) ↓ [Runtime resolution] WavmReadCustomDAPreimage (arbitrator/prover/src/host.rs) ↓ [Mapped to WAVM instruction] ReadPreImage (arbitrator/prover/src/machine.rs - instruction handler) ``` * Enable arbos 50 (OffchainLabs#3547) * Fix linter issue * treat precompile code as empty during delegation after arbos 50 (OffchainLabs#3549) * Enable ArbOS 50 in system tests (OffchainLabs#3550) * Enable ArbOS 50 in system tests * fix remaining tests * code refactor * fix TestTimeboostTxsTimeoutByBlock system test * Add fd_tell wasi stub The usage of fd_tell was introduced in Rust 1.87 by rust-lang/rust#137165 * Update Rust to 1.89.0 (latest stable) Close NIT-3562 * Downgrade Rust to 1.88.0 Rust 1.89 breaks the wasmer build. So we need to upgrade wasmer before updating to 1.89. See: wasmerio/wasmer#5690 * Add consensus v41 to Dockerfile Also removed deprecated v42-rc.1 since no one should ever use it * prover: remove unnecessary wasm exports (OffchainLabs#3554) prover's unnecessary exports add lots of dependencies to user_host which imports it. They are only required for native and not when compiling to wasm. * Move the MaxTxGas limit enforcement (OffchainLabs#3557) * Revert "Enable ArbOS 50 in system tests (OffchainLabs#3550)" This reverts commit e167107. * Correctly implement EIP-7825 * Fix tests that need to set max block gas limit * Add support for consensus v50 to Docker (OffchainLabs#3551) * Revert "Add fd_tell wasi stub" This reverts commit 94c70b7. * Store single-gas in multi-gas collector (OffchainLabs#3552) * Fix cargo clippy warnings for Rust 1.88 * Fix cargo clippy warning Use variables directly in the `format!` string. * Prevent HTTP response body leaks in restful DAS client (OffchainLabs#3558) * Custom GH action for unified installing Rust (OffchainLabs#3559) * Create new action for installing Rust * Use the new action in arbitrator-ci.yml * Use the new action in the other workflows * Fix merge conflict * Instrument multi-gas in StylusPrograms (just for getBytes32) (OffchainLabs#3539) * Multigas collector support for WASM computation resource type * Instrument with multigas stylus contracts (getBytes32) * Add test for GetBytes32 multi gas usage * Add multigas system test for getBytes32 stylus program * Correct WasmStateLoadCost multigas cost * Attribute Wasm computation gas for OOG branch * chore: Update TODO comment for expectedWasmComputation * Remove hardcoded check for wasm multi gas * Correct attributeWasmComputation usedGas for both scenarios * Correct rightshift linter documentation (OffchainLabs#3562) * Update gammazero dependency Pulls in OffchainLabs/go-ethereum#530 Fixes: NIT-3760 * Update go-ethereum pin * Run go.mod tidy * Update go version to 1.24.x across github workflows Fixes: OffchainLabs#3566 Fixes: NIT-3799 * Merge v1.16.3 (OffchainLabs#3568) * Merge v1.16.3 * skip EIP-7825 for arbitrum chain * fix blob gas * Test Genesis assertion on nitro init * Change ValidatePreimage return type to u8 where possible This changes the exported declaration of wavmio__validateCertificate to have a u8 return type. In practice when it is built with the rust wasm32-unknown-unknown target, sizes smaller than WASM's minimum size type of i32 are extended. On the Go side, go:wasmimport only supports at least 32 bit types in order to match the wasm ABI requirements. So this change has no functional impact but it makes it slightly clearer what the return type represents. * Add error logging from jit on invalid preimage type * Multigas: support geth multigas api changes (OffchainLabs#3569) * chore(ci): update GitHub Actions to latest versions (OffchainLabs#3573) * Update nightly-ci.yml * Update arbitrator-ci.yml * Update EIP-7910 implementation (OffchainLabs#3571) * Update EIP-7910 implementation There are a few different things going on in this pull request. 1. Implementing the Name() function for precompiles. 2. Adding a system test for calling the new eth_config RPC method. Fixes: NIT-3679 * Remove system contracts which aren't needed for Arbitrum These contracts are related to Ethereum's consensus and beacon chain functionality. * Add suppoort for Consensus V50 (rc.3) to Docker * Rename CustomDA preimage type to DACertificate * Move new ValidateCertificate opcode to end, add note * add extra logs to figure out a round causing NO_ONCHAIN_CONTROLLER * chore: Add require-exhaustive-init linter to structs (1/n) (OffchainLabs#3575) * chore(lint): add require-exhaustive-init linter to structs * fix lint * Use `nil` as the default for slice --------- Co-authored-by: Tuan Tran <[email protected]> * fix: Correctly count struct fields in the structinit linter (OffchainLabs#3579) * Turn on linter for all structs (no opt-in) * Count fields in a correct way * Add verification Python script * Fix counting (include single-type-multi-name fields) * Remove script * Restore conditional linting * Fix PublishedMachine test to handle cases when latest release is behind latest ArbOS (OffchainLabs#3581) * Multigas: Instrument GasChargingHook with multi-gas (OffchainLabs#3576) * Instrument multi-gas in GasChargingHook * Fixup comments in tx processor * Run execution-spec-tests in nightly CI (OffchainLabs#3577) * Update restful_client.go * Update dasRpcServer.go * Move output dir setup to constructor; make Start launch-only (OffchainLabs#3578) * Move output dir setup to constructor; make Start launch-only * fix lint * rpc_test: remove typo and redundant checks * Improve error reporting in the batch poster * fix: Make structinit linter work cross packages (OffchainLabs#3586) * New linter approach: checkpoint (FieldCountAnalyzer without exporting facts) * Merge field counts across packages * Working cross packages * Fixing MessageSequencer 1/2 * Fixing MessageSequencer 2/2 * fieldcount tests * Merge analyzers * Finish tests * Add documentation * Lint linter * Fmt test fixes * Fix wording in the comment about linter tip * Add reference to the 'want' comments * Add copyright lines * Remove redundant Background context initialization in poster_test (OffchainLabs#3595) * Include msg.SetCodeAuthorizations field when constructing arbitrum.TransactionArgs (OffchainLabs#3598) * Fail instead of panic when building second node if first node was not built with L1 (OffchainLabs#3601) * Fail instead of panic when building second node if first node was not built with L1 * fix grammar * Unify DAS interfaces * Turn bold into a sub-package of nitro (OffchainLabs#3599) * Remove unnecessary files * fix readme.md * Replace github.com/offchainlabs/bold/ with github.com/offchainlabs/nitro/bold/ * fix dockerfile * remove bold specific workflow * Fix l1 pricing state field order * Data streamer * Stream parameters * Make DAS signing public * Add licence header * Move to das module. Revert making signing public * Start streaming protocol * Do stream * Finalize streaming * Plug in the new API * Simplify DASRPCClient * Docs * Do not rename const * Update license to trigger CI * Add to chain_info: feed-signed * Disable `feed-signed`` for arb1 and nova * // #nosec G115 * Named initializer linter * Use named init for all structs with more than 5 fields * Update the implementation of EIP-7825 (OffchainLabs#3589) * Update the implementation of EIP-7825 The previous implementation was incorrect and caused the gas to be limited in the wrong part of gas estimation and execution. * Simplify the implementation of EIP-7825 * Restore some files that don't need to change There were some spurious changes leftover from previous commits. * Clean up branching logic in GasChargingHook This produces one less arbos state access after the ArbOS 50 upgrade. * Fix l1 pricing state field order (OffchainLabs#3602) * Multigas: Add multi gas to transaction receipt and remove collector (OffchainLabs#3594) * Replace multigas collector with receipts in system tests * Delete multigas collector (use receipts instead) * Fixup TODO and bump go-ethereum version * Simplify TestMultigasDataFromReceipts * Remove the dryRun boolean If the tests are run in dryRun mode, they don't trigger the findings expected. * bugfix: genesis validation without l1 reader (OffchainLabs#3607) * Rename namedfields to namedfieldsinit * Added more test cases * Test cross-package init * Remove dryRun flag * Linters cleanup (OffchainLabs#3609) * Properly detect diagnostics in tests (structinit linter) * Properly detect diagnostics in tests (koanf linter) * Properly detect diagnostics in tests (pointcheck linter) * Properly detect diagnostics in tests (rightshift linter) * Add license headers * Fix license position in the structinit test files * Hide multi-gas behind feature flag (OffchainLabs#3613) * Hide multi-gas behind feature flag Ensure we don't expose a wrong multi-gas before finishing all instrumentation milestones. Close NIT-3838 * Test expose multi-gas set to false * Arbos50 block gaslimit (OffchainLabs#3606) * block_builder: use block gas limit as a soft boundary * arbosstate: increase gas limit on upgrade to arbos50 * gas limit: test new behavior * make lint * block_processor: check arbos-version before other conditions * gas-limit-test fixes * gas-limit-test: touch-ups * arbosstate: reduce arbos-50 block-gas-limit multiplier * Ignore the go.work and go.work.sum files These files can make certain IDEs and go tooling work better with the repository for local development, but they also complicate some of the CI workflows in unexpected ways. Therefore, let's ignore them from diffs to prevent their accidentally being checked-in by developers who use them. * bold/e2e: fix Anvil startup race causing RPC hangs in end-to-end tests (OffchainLabs#3612) * e2e: start backend before rollup deployment to avoid RPC on unready Anvil * anvil backend: create RPC client in Start and add robust readiness check * Recursively fetch submodules (OffchainLabs#3621) * Rename the old module to '..._sender' * Migrate and refactor partial message management * Implement receiver logic * Fix test * Extract consts * Extract endpoint names * Document * Add linter check * Multigas: remove hardcoded value in TestMultigasStylus_GetBytes32 (OffchainLabs#3614) * Fix ProduceBlockAdvanced handling of txs that don't need sequencing hooks (OffchainLabs#3627) Co-authored-by: Pepper Lebeck-Jobe <[email protected]> * [config] Handle node.staker.strategy and node.bold.strategy appropriately (OffchainLabs#3582) * Handle node.staker.strategy and node.bold.strategy appropriately * Update nitro-testnode * Gas floor (OffchainLabs#3618) * update contracts for internal batch posting report fields * batch-gas-cost: check for floor-gas-cost in arbos-50 * replace calldataprice with parentGasFloorPerToken * internal_tx: use floorGasPerToken * update acts and support legacy feed * contracts: revert to version on master * precompiles: updates for parent-gas-floor * internal_tx: add per-batch-gas even if using floor * eth-config: update forkid * Add a test for gas floor * Enable delayed sequencer, satisfy finality requirements * internal_tx: use a constant instead of PerBatchGasCost * update gas floor price test * incomingmessage: fix comments * Remove leftover comment --------- Co-authored-by: Joshua Colvin <[email protected]> Co-authored-by: Preston Van Loon <[email protected]> Co-authored-by: Forostovec <[email protected]> Co-authored-by: Pepper Lebeck-Jobe <[email protected]> Co-authored-by: Tristan-Wilson <[email protected]> Co-authored-by: Chihyun Song <[email protected]> Co-authored-by: Alexandros Filios <[email protected]> Co-authored-by: Tsahi Zidenberg <[email protected]> Co-authored-by: radik878 <[email protected]> Co-authored-by: Maciej Kulawik <[email protected]> Co-authored-by: Mikhail Rogachev <[email protected]> Co-authored-by: Maciej Kulawik <[email protected]> Co-authored-by: Aman Sanghi <[email protected]> Co-authored-by: Ganesh Vanahalli <[email protected]> Co-authored-by: Pepper Lebeck-Jobe <[email protected]> Co-authored-by: sashass1315 <[email protected]> Co-authored-by: Raul Jordan <[email protected]> Co-authored-by: Gabriel de Quadros Ligneul <[email protected]> Co-authored-by: Alexandros Filios <[email protected]> Co-authored-by: Diego Ximenes <[email protected]> Co-authored-by: Gabriel de Quadros Ligneul <[email protected]> Co-authored-by: Tristan Wilson <[email protected]> Co-authored-by: strmfos <[email protected]> Co-authored-by: Piotr Mikołajczyk <[email protected]> Co-authored-by: Ruslan Granger <[email protected]> Co-authored-by: Hopium <[email protected]> Co-authored-by: Tuan Tran <[email protected]> Co-authored-by: Kolby Moroz Liebl <[email protected]> Co-authored-by: sashaodessa <[email protected]> Co-authored-by: Alvarez <[email protected]> Co-authored-by: Galoretka <[email protected]> Co-authored-by: GarmashAlex <[email protected]>
Fixes NIT-3596
This PR makes it so execution-spec-tests https://github.com/OffchainLabs/execution-spec-tests runs on our Nightly CI, this will give us greater confidence that Arbitrum conforms to the EIP's we support.