Validation Inputs wiring #2604
Validation Inputs wiring #2604
Conversation
There are several changes in this commit.
1. Move the `parse_inputs.rs` and `prepare.rs` files from the benchbin
package to the prover. This enables the prover to ingest most of the
information it needs from the `block_inputs_<id>.json` file.
2. Add a `--json-inputs` flag to the `prover` bin which obviates
several competing flags preferring to read the data from the json.
3. Write the json file to disk in the working directory before the
arbitrator spawner writes the `run_prover.sh` file which is attempting
to make a similar validaiton easy to reproduce.
4. Introduce a new `recordBlock` method to the tests suite which can be
called once a block has been recorded to L1.
5. Wire the call to `recordBlock` into the `TestProgramStorage` to be
sure to capture a block that actually uses a Stylus contract so that
the prover can be shown to work with that block as well.
6. Enhance the `parse_inputs.rs` to know how to handle the `UserWasms`
deserialization.
To reproduce the failure state at this commit:
1. make clean && make all
2. go test -timeout 10m -tags challengetest -run ^TestProgramStorage$ github.com/offchainlabs/nitro/system_tests
3. Observe, there are now two new files in `ls system_tests/block_inputs_{2,5}.json`
4. target/bin/prover target/machines/latest/machine.wavm.br -b --json-inputs system_tests/block_inputs_5.json
5. The error is:
```
thread 'main' panicked at prover/src/machine.rs:690:58:
unknown dictionary: TryFromPrimitiveError { number: 10 }
```
If you make changes to the rust code, you can quickly rebuild and install with:
```
cargo build --manifest-path arbitrator/Cargo.toml --release --bin prover && install arbitrator/target/release/prover target/bin/prover
```
This change also: 1. Documents the `parse_inputs.rs` file in an attempt to give context to why the deserialization is special. 2. Hardcodes the architecture to `wasm` when recording ValidationInputs for use with the prover. Previously, the `jit` validator would use the go system architecture, and the `arbitrator` validator would use `wavm`. 3. Switches the `TestStorageProgram` test back to the `jit` validator. Note: As of this commit, there is still a small problem that when running the validator using the json inputs, it does not come up with the same end has as `entry.End.Hash`.
This makes it easier to visually confirm the expected outcome if a human is looking at the output of the prover and knows what the expected end global state should be.
The thrid one is left around as an example of how to use the new method.
1. Remove the part of the writeToFile method on ArbitratorSpawner which was writing the bash scripts. (TODO: Write the block_inputs_<id>.json files somewhere better.) 2. Drop the expOut from the WriteToFile signature since none of the implementations need it any more. 3. Rename some unused function arguments to "_".
At this point the location of the output file is hardcoded to be in the `$HOME/.arbitrum` directory. Hopefully, this will be okay.
cla-bot
bot
added
the
s
Reviewers, please tell me the "right" way to ensure that the transaction is visible in the L1. I don't like the idea of adding sleeps in tests to ensure that some event has occurred. Is there some RPC I can call or event I can subscribe to that I can then wait for before proceeding?
If the inbox is lagging behind the message, the node won't be able to create a validation entry for it.
Meant to do this in the previous commit.
This change attempts to make the location for writing the json file more flexibly configurable by the clients without making it difficult to put the data in a resonable spot by default.
validator/inputs/writer.go
Outdated
| } | ||
|
|
||
| // SetSlug configures the Writer to use the given slug as a directory name. | ||
| func (w *Writer) SetSlug(slug string) *Writer { |
There was a problem hiding this comment.
Is there a use case for changing slug during activation? I didn't see one. Seems like it'd be better to pass slug as input to NewWriter
There was a problem hiding this comment.
I'm not sure what you mean by "activation." I'm guessing you mean initialization.
Sure. The use-case is like this. We might want to be able to distinguish among the following block_inputs_3.json files:
~/.arbitrum/validation_inputs/TestProgramStorage/20241021_1905/block_inputs_3.json
~/.arbitrum/validation_inputs/TestProgramMemory/20241021_1905/block_inputs_3.json
~/.arbitrum/validation_inputs/BlockValidator/20241021_1905/block_inputs_3.json
In this case, it's obvious that the first two came from different test case executions, and the third one came from within the BlockValidator.
With the new functinal options pattern, the slug will be passed to the call to NewWriter. So, it should satisfy your desires, I think.
validator/inputs/writer.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| baseDir := fmt.Sprintf("%s/.arbitrum/validation-inputs", homeDir) |
There was a problem hiding this comment.
not sure how easy it is to get here.. but a good base dir would be stack.InstanceDir()
There was a problem hiding this comment.
It looks slightly odd to wire together. But, I suggest that good separation of concerns would keep the inputs.Writer from knowing about the stack at all. If anything, I could see the various clients of this library (in the common case the BlockValidator) getting the stack.InstanceDir() and passing in using the WithBaseDir option.
... but, even the BlockValidator doesn't currently have convienent access to the stack directly.
NewStatelessBlockValidator is currently being passed a stack argument. But, it doesn't store the reference to it anywhere.
If you feel strongly that we should pass in stack.InstanceDir() from the BlockValidator, we could add a reference to the stack as a member of the StatelessBlockValidator and then use it to come up with the InstanceDir()
In fact, I'll just go ahead and wire that together now, and I can back it out if you think it's getting too messy.
There was a problem hiding this comment.
I agree it's not very pretty - but the good thing about stack.InstanceDir is that we know this process has write permission there, and that this directory is saved in reboot.
I think we've never had real failure. If we by default write to a dir that doesn't have these two properties - we probably wouldn't know until it's too late.
This change also saves the stack argument passed to the NewStatelessBlockValidator function to a member of StatelessBlockValidator so that the BlockValidator can use the `stack.InstanceDir()` method to set a reasonable base directory for storing validation inputs as json files.
Replaces the
validation_writeToFilerpc call with anarbdebug_validationInputsAtrpc call.Rather than actually writing the file directly, the rpc call returns a JSON representation of the validation inputs, and leaves it to the caller to write the JSON to disk.
This change also calls the new ValidationInputsAt method from a test using a new
recordBlockfunction in common_test.go and writes the JSON file directly when the BlockValidator fails to validate a block.The arbitrator prover is also updated to be able to parse the JSON inputs instead of getting all of the options from the command-line.
Here's an example of how to use the feature:
Fixes: https://linear.app/offchain-labs/issue/NIT-2519/create-a-simple-way-to-record-system-tests-and-specific-blocks-into