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

backport #393 to nitro v3.3.x - fix flatCallTracer onexit out of bound #394

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

joshuacolvin0
Copy link
Member

@joshuacolvin0 joshuacolvin0 commented Jan 6, 2025

Pulled in with OffchainLabs/nitro#2866

@cla-bot cla-bot bot added the s CLA signed label Jan 6, 2025
joshuacolvin0 added a commit to OffchainLabs/nitro that referenced this pull request Jan 6, 2025
@joshuacolvin0 joshuacolvin0 merged commit a437823 into nitro_v3.3.1 Jan 6, 2025
4 checks passed
@joshuacolvin0 joshuacolvin0 deleted the flatcalltracer branch January 6, 2025 22:12
Sneh1999 added a commit to EspressoSystems/nitro-espresso-integration that referenced this pull request Jan 10, 2025
* Get's the bold_state_provider_test.go tests passing

The big problem is still the end-to-end TestChallengeProtocolBOLD
test.

* Get TestChallengeProtocolBOLD passing

* Fix the cache

Now that the virtual padding is handled in the BoLD protocol itself
and not in the creation of the hash leaves being fed into the history
committer, the number of hashes read from the cache doesn't need to
equal the number of leaves (including virtual leaves.)

* Use geth's in-tree BLS again after v1.14.0

* handle race condition and address minor comments

* update geth pin

* StateBuildingLogFunction shouldn't take targetHeader as an arg

* minor fix

* fix linter

* Revert "Do not use SetFinalizer to remove entries from preimageResolvers in a hostio machine"

This reverts commit 5548069.

* Uses preimageResolverRefCounter instead of relying on finalizers to remove preimage resolvers from global map

* Reuse same global map to keep preimage resolver reference counter

* TestEntriesAreDeletedFromPreimageResolversGlobalMap

* Adds machine's finalizer back

* Change machine destroy order in TestEntriesAreDeletedFromPreimageResolversGlobalMap

* fix MockTransfer in trace

* Add a test challenging the start step in BoLD

* Update the pin for the bold submodule

This is the current unify-req-meta branch which fixes a bug in
one-step proof calculation.

* Sets contextId to zero when destroying a machine

* contextId as pointer to avoid worrying about what are the valid context ids

* Simplifies if

* WIP: Add the boldmach wrapper

This *should* be able to wrap an arbitrator machine and do the special
handling for the BoLD protocol to make it look like there is one more
machine state at the front of processing a machine.

* Fix the two obviously errored hasStepped bits

This still doesn't get the test passing, but it's bound to be closer.

* Fix TestChallengeProtocolBOLDStartStepChallenge

* Handle SetPreimageResolver being called multiple times for the same machine

* panic in case when resolver is not found, or when ref counter is negative

* renames refCounterAfter to refCount

* fix variable name

* Fix the execution_run tests

* Attempt at fixing the virtual leaves issue

The CollectProof and CollectMachineHashes functions were both susceptible to
challenges where it was possible that the rival would have committed to more
messages than this validator. And, in that case, it would attempt to look up a
message number which was greater than the highest messge number it had verified
as part of the batch in which the challenge originated.

* Begin work on TestChallengeProtocolBOLDVirtualBlocks

* Update geth

* fix build

* Update geth

* Changes based on PR comments

* Changes based on PR comments

* Update geth

* disable blob posting in the test batch poster config

* Complete TestChallengeProtocolBOLDVirtualBlocks

* Handle challenges in the virtually padded part of the leaaves

Before this change, if there was a block challenge at a height in any block
above the batchLimit, then the validator was not correctly creating inclusion
proofs because it was attempting to fetch execution results for blocks which
didn't really exist.

Now, the code detects that situation and simply returns the hash of an
arbitrator machine in the FINISHED state (since the Virtual leaf hashes) are all
in that state by virtue of their being repeated copies of the end state of a
block.

* common_test: remove race condition

* Bump google.golang.org/grpc from 1.64.0 to 1.64.1

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.64.0 to 1.64.1.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.64.0...v1.64.1)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

* Get the test to compile

* Split virtual blocks test into first and near last virtual block tests

* Update test to better handle virtual blocks returned by honest impl

* switch geth-hook to PostingGasHook

* update geth pin for posting gas hook

* rename to RPCPostingGasHook

* Fix the CollectProof and CollectMachineHashes calls

Previously, they couldn't produce correct inclusion proofs for block-level
challenges on blocks in the virtual range. That is to say, if this validaotor
processed n L2Blocks when creating an assertion and a rival processed >= n+1
L2Blocks, this validator would attempt to lookup a block index for which no
real block existed.

Now, the code properly catches this case and the last machine state for the last
real block is used for all virtual L2Blocks.

* Add dangerous batch poster options configurable

* stylus test: infinite loop should create out-of-gas error

* fix user.wat

* update user.wat's module hash

* Use exec for docker entrypoint

* Get the branch building again

This is probably not where we ultimately want to be. Too much boilerplate is
escaping from the bold system. Maybe, before introducing a dependency injection
framework, I should just introduce something manual that would instatiate all
the instances that the challenge manager, watcher, assertion manager, etc. need
and then wires them together in the "default" way using the constructors from
each package.

* Add gci to the golangci-lint config

This change also runs the gci linter against the repo to give us a "hard reset"
on all of the package imports.

Editor integrations: https://golangci-lint.run/welcome/integrations/

* Make golangci-lint 1.62.0 happy

The linter learned how to find missing checks for safe typecasts and we had some
spots where they weren't being checked.

* Also log an error if the backlogSegment type cannot be asserted

* Panic in syncmap if the key cannot be converted to the type

* change default scheme used in tests to HashScheme

* Also panic in the other syncmap function on failed type assertions

* TestHostioWithoutEVMEquivalentCosts

* Renames test to TestGasUsageOfHostiosThatDontHaveGoodEVMEquivalents

* expectedInc to expectedInk

* Split TestHostioWithoutEVMEquivalentCosts in multiple tests, test
hostios with multiple arguments

* Adds missing t.Parallel() to gas usage hostio tests

* TestPayForMemoryGrowGasUsage with zero pages

* HOSTIO_INK const

* Fix lint issues

* Fix lint issues

* Fix the wiring after the giant refactoring in bold repo

* update batch-poster.max-size and batch-poster.max-4844-batch-size descriptions

* Move ArbOS upgrade handling to a bit later in block production

* block reexecution should requite init.then-quit

* broadcastclient: check if compression was negotiated when connecting to feed server

* use t.Run in broadcastclient tests

* Merge branch 'master' into bold-review

* Update to the same go-ethereum pin as master

This version of the go-ethereum project is needed for some of the tests to
behave correctly.

* Fix the path since it was moved down a dir

* update geth pin

* update geth submodule

* Update the bold pin and remove some logging

* Fix 2 lint issues

The first is to sort the imports in mock_machine_test.go.

The second is to rearrange the bold machine to avoid the downcast.

* update geth pin to master

* Check batchProcessed > 0 to avoid underflow

* Remove maxNumberOfBlocks from bold state provider

The signature changed in the bold repo, and needed to be updated in Nitro as
well.

* Update the bold pin to use the tip of main

* Update bold pin and adjust nitro code accordingly

Specifically, the type of the 2 AssertionHash fields in the
AssertionCreationInfo struct is now a protocol.AssertionHash instead of
`[32]byte`.

This change also updates nitro's `go.mod` and `go.sum` with `go mod tidy` to
pick up some of the new indrect dependencies the bold machinary pulls in.

* Get the BoLD challenge tests compiling again

Note, not "passing", but "compiling."

* Bump nitro-testnode to bold-upgrade branch for bold testing

* Fix bold staker without block validation enabled

* Fix multi protocol staker legacy staker wallet initialization

* Sync in the latests changes from the bold repo

This includes wiring the HeaderProvider through to the challenge stack and a
slightly clunky backend wrapper around the ethereum client instances.

* Validator wallet refactor for bold

* Address PR comments

* Add documentation for the EOA struct

* Make bold state validated check more sturdy

* Fix bold state provider test exiting loop early

* bug fix

* del

* add back tag

* edit bold submodule

* bold submod

* bold submod

* Make previousGlobalState a value instead of pointer

There's no longer a need to be able to pass in nil.

* Update bold repo pin to main

* remove nil opts from validation node creation

* remove nil opts from validation node creation

* rem print

* Test an overflow assertion

This test confirms that when there are more messages in the sequencer inbox than
the block challenge level height, the next assertion is an "overflow" assertion
and is not required to wait to minimumAssertionPeriod blocks before posting.

Part of NIT-2794

* Make the finished machine actually pass the global state

No reason to keep this in a separate API.

* Add BoLD ascii art for the log

* Rename maxInboxCount -> maxSeqInboxCount

This makes it clear that the number is the maximum sequencer inbox count, and
not the maximum delayed inbox count.

* Move the Info logging into the staker implmentations

* chore: update AEP link

* commentary from lee addressed, part 1

* address more review comments

* check stake token addr

* move to test only file

* lee feedback on testonly machine wrapper

* nolint

* feedback

* more feedback from lee, contexts, no caching module root

* address remaining bold commentary

* add back

* add back block validator set module root

* rm log

* assertion hash topic and path defaults

* builds

* gci

* gci

* build

* test

* Less useBoldMachine pointers

* builds

* builds

* Don't double wrap bold machines

* Bump nitro-testnode to latest master to fix CI

* Update bold submodule to fix shutdown

* Pull in geth tipReceipient fix

* Include tx compression level in calldata units cache

* Bump geth pin to fix lint

* update geth: cherry-pick tracing fix

* Pull in flatcalltracer fix

Pulls in OffchainLabs/go-ethereum#394

* merge

* update submodule

* fix ci

* update go-ethereum

* fix transaction streamer

* update go deps

* Lint

* update flake.nix

* Safe cast

* update ci.yml

* Fix challenge test

* Skip tests

* Skip BOLD test

* skip bold test TestChallengeProtocolBOLDFirstVirtualBlock

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Pepper Lebeck-Jobe <[email protected]>
Co-authored-by: Lee Bousfield <[email protected]>
Co-authored-by: amsanghi <[email protected]>
Co-authored-by: Tsahi Zidenberg <[email protected]>
Co-authored-by: Ganesh Vanahalli <[email protected]>
Co-authored-by: Aman Sanghi <[email protected]>
Co-authored-by: Diego Ximenes <[email protected]>
Co-authored-by: Tsahi Zidenberg <[email protected]>
Co-authored-by: Joshua Colvin <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Maciej Kulawik <[email protected]>
Co-authored-by: Maciej Kulawik <[email protected]>
Co-authored-by: Harry Kalodner <[email protected]>
Co-authored-by: Raul Jordan <[email protected]>
Co-authored-by: Harry Kalodner <[email protected]>
Co-authored-by: Jeremy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants