From fa742c7e91e314519cbef4b54429c3bb0fac65c5 Mon Sep 17 00:00:00 2001 From: Rachel Bousfield Date: Thu, 18 Apr 2024 01:53:23 -0600 Subject: [PATCH 1/5] fixed memory edge case --- arbitrator/caller-env/src/guest_ptr.rs | 6 +++++ arbitrator/prover/src/programs/meter.rs | 2 +- arbitrator/stylus/tests/grow/fixed.wat | 25 +++++++++++++++++++ .../stylus/tests/{ => grow}/grow-120.wat | 0 .../stylus/tests/{ => grow}/grow-and-call.wat | 0 .../wasm-libraries/user-host/src/program.rs | 17 ++++++++----- system_tests/program_test.go | 9 +++++-- 7 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 arbitrator/stylus/tests/grow/fixed.wat rename arbitrator/stylus/tests/{ => grow}/grow-120.wat (100%) rename arbitrator/stylus/tests/{ => grow}/grow-and-call.wat (100%) diff --git a/arbitrator/caller-env/src/guest_ptr.rs b/arbitrator/caller-env/src/guest_ptr.rs index 566d2d61d..cbef490c6 100644 --- a/arbitrator/caller-env/src/guest_ptr.rs +++ b/arbitrator/caller-env/src/guest_ptr.rs @@ -41,3 +41,9 @@ impl Deref for GuestPtr { &self.0 } } + +impl GuestPtr { + pub fn to_u64(self) -> u64 { + self.into() + } +} diff --git a/arbitrator/prover/src/programs/meter.rs b/arbitrator/prover/src/programs/meter.rs index cb8f987a1..ab069fd91 100644 --- a/arbitrator/prover/src/programs/meter.rs +++ b/arbitrator/prover/src/programs/meter.rs @@ -401,7 +401,7 @@ pub fn pricing_v1(op: &Operator, tys: &HashMap) -> dot!(I32Store, I32Store8, I32Store16) => 825, dot!(I64Store, I64Store8, I64Store16, I64Store32) => 950, dot!(MemorySize) => 3000, - dot!(MemoryGrow) => 1, // cost handled by memory pricer + dot!(MemoryGrow) => 8050, // rest of cost handled by memory pricer op!(I32Eqz, I32Eq, I32Ne, I32LtS, I32LtU, I32GtS, I32GtU, I32LeS, I32LeU, I32GeS, I32GeU) => 170, op!(I64Eqz, I64Eq, I64Ne, I64LtS, I64LtU, I64GtS, I64GtU, I64LeS, I64LeU, I64GeS, I64GeU) => 225, diff --git a/arbitrator/stylus/tests/grow/fixed.wat b/arbitrator/stylus/tests/grow/fixed.wat new file mode 100644 index 000000000..7d6cc3aff --- /dev/null +++ b/arbitrator/stylus/tests/grow/fixed.wat @@ -0,0 +1,25 @@ +;; Copyright 2023-2024, Offchain Labs, Inc. +;; For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE + +(module + (import "console" "tee_i32" (func $tee_i32 (param i32) (result i32))) + (func (export "user_entrypoint") (param $args_len i32) (result i32) + ;; fail to grow the memory a non-zero number of pages + i32.const -65537 + call $tee_i32 + memory.grow + call $tee_i32 + i32.const -1 + i32.eq + i32.eqz + (if (then unreachable)) + + ;; succeed growing 0 pages + i32.const 0 + memory.grow + call $tee_i32 + i32.eqz + i32.eqz + ) + (memory (export "memory") 0 0) +) diff --git a/arbitrator/stylus/tests/grow-120.wat b/arbitrator/stylus/tests/grow/grow-120.wat similarity index 100% rename from arbitrator/stylus/tests/grow-120.wat rename to arbitrator/stylus/tests/grow/grow-120.wat diff --git a/arbitrator/stylus/tests/grow-and-call.wat b/arbitrator/stylus/tests/grow/grow-and-call.wat similarity index 100% rename from arbitrator/stylus/tests/grow-and-call.wat rename to arbitrator/stylus/tests/grow/grow-and-call.wat diff --git a/arbitrator/wasm-libraries/user-host/src/program.rs b/arbitrator/wasm-libraries/user-host/src/program.rs index b43e632b9..4199a691f 100644 --- a/arbitrator/wasm-libraries/user-host/src/program.rs +++ b/arbitrator/wasm-libraries/user-host/src/program.rs @@ -16,7 +16,7 @@ use eyre::{eyre, Result}; use prover::programs::prelude::*; use std::fmt::Display; use user_host_trait::UserHost; -use wasmer_types::WASM_PAGE_SIZE; +use wasmer_types::{Pages, WASM_PAGE_SIZE}; // allows introspection into user modules #[link(wasm_import_module = "hostio")] @@ -186,9 +186,14 @@ impl Program { unsafe { PROGRAMS.last_mut().expect("no program") } } - /// Reads the program's memory size in pages - fn memory_size(&self) -> u32 { - unsafe { program_memory_size(self.module) } + /// Reads the program's memory size in pages. + fn memory_size(&self) -> Pages { + unsafe { Pages(program_memory_size(self.module)) } + } + + /// Reads the program's memory size in bytes. + fn memory_size_bytes(&self) -> u64 { + self.memory_size().0 as u64 * WASM_PAGE_SIZE as u64 } /// Provides the length of the program's calldata in bytes. @@ -198,8 +203,8 @@ impl Program { /// Ensures an access is within bounds fn check_memory_access(&self, ptr: GuestPtr, bytes: u32) -> Result<(), MemoryBoundsError> { - let last_page = ptr.saturating_add(bytes) / (WASM_PAGE_SIZE as u32); - if last_page > self.memory_size() { + let end = ptr.to_u64() + bytes as u64; + if end > self.memory_size_bytes() { return Err(MemoryBoundsError); } Ok(()) diff --git a/system_tests/program_test.go b/system_tests/program_test.go index ab7185926..c9e32887f 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -838,7 +838,8 @@ func testMemory(t *testing.T, jit bool) { memoryAddr := deployWasm(t, ctx, auth, l2client, watFile("memory")) multiAddr := deployWasm(t, ctx, auth, l2client, rustFile("multicall")) - growCallAddr := deployWasm(t, ctx, auth, l2client, watFile("grow-and-call")) + growCallAddr := deployWasm(t, ctx, auth, l2client, watFile("grow/grow-and-call")) + growFixed := deployWasm(t, ctx, auth, l2client, watFile("grow/fixed")) expectFailure := func(to common.Address, data []byte, value *big.Int) { t.Helper() @@ -881,7 +882,7 @@ func testMemory(t *testing.T, jit bool) { expectFailure(multiAddr, args, oneEth) // check that activation fails when out of memory - wasm, _ := readWasmFile(t, watFile("grow-120")) + wasm, _ := readWasmFile(t, watFile("grow/grow-120")) growHugeAddr := deployContract(t, ctx, auth, l2client, wasm) colors.PrintGrey("memory.wat ", memoryAddr) colors.PrintGrey("multicall.rs ", multiAddr) @@ -924,6 +925,10 @@ func testMemory(t *testing.T, jit bool) { Fatal(t, "unexpected memory footprint", programMemoryFootprint) } + // check edge case where memory doesn't require `pay_for_memory_grow` + tx = l2info.PrepareTxTo("Owner", &growFixed, 1e9, nil, args) + ensure(tx, l2client.SendTransaction(ctx, tx)) + validateBlocks(t, 2, jit, builder) } From a26a6cb9ab8ffe6a9f9a6dd0dffdeb437ae3a834 Mon Sep 17 00:00:00 2001 From: Rachel Bousfield Date: Thu, 18 Apr 2024 10:23:26 -0600 Subject: [PATCH 2/5] exhaustive mem write test --- arbitrator/stylus/tests/grow/mem-write.wat | 45 ++++++++++++++++++++++ system_tests/program_test.go | 36 ++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 arbitrator/stylus/tests/grow/mem-write.wat diff --git a/arbitrator/stylus/tests/grow/mem-write.wat b/arbitrator/stylus/tests/grow/mem-write.wat new file mode 100644 index 000000000..ec6efd973 --- /dev/null +++ b/arbitrator/stylus/tests/grow/mem-write.wat @@ -0,0 +1,45 @@ +;; Copyright 2023, Offchain Labs, Inc. +;; For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE + +(module + (import "vm_hooks" "pay_for_memory_grow" (func $pay_for_memory_grow (param i32))) + (import "vm_hooks" "read_args" (func $read_args (param i32))) + (import "vm_hooks" "write_result" (func $write_result (param i32 i32))) + (import "console" "tee_i32" (func $tee_i32 (param i32) (result i32))) + (func (export "user_entrypoint") (param $args_len i32) (result i32) + local.get $args_len + i32.eqz + (if (then + ;; write an empty result to offset 0 + (call $write_result (i32.const 0) (i32.const 0)) + (return (i32.const 0)) + )) + + ;; grow 1 page so that we can read our args + i32.const 1 + memory.grow + drop + + ;; store the size argument at offset 0 + i32.const 0 + call $read_args + + ;; read the argument and grow the remainder + i32.const 0 + i32.load8_u + i32.const 1 + i32.sub + memory.grow + drop + + ;; write a result (should panic if out of bounds) + i32.const 1 + i32.load + i32.const 5 + i32.load + call $write_result + + i32.const 0 + ) + (memory (export "memory") 0) +) diff --git a/system_tests/program_test.go b/system_tests/program_test.go index c9e32887f..906c7de7b 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -840,6 +840,7 @@ func testMemory(t *testing.T, jit bool) { multiAddr := deployWasm(t, ctx, auth, l2client, rustFile("multicall")) growCallAddr := deployWasm(t, ctx, auth, l2client, watFile("grow/grow-and-call")) growFixed := deployWasm(t, ctx, auth, l2client, watFile("grow/fixed")) + memWrite := deployWasm(t, ctx, auth, l2client, watFile("grow/mem-write")) expectFailure := func(to common.Address, data []byte, value *big.Int) { t.Helper() @@ -929,7 +930,40 @@ func testMemory(t *testing.T, jit bool) { tx = l2info.PrepareTxTo("Owner", &growFixed, 1e9, nil, args) ensure(tx, l2client.SendTransaction(ctx, tx)) - validateBlocks(t, 2, jit, builder) + // check memory boundary conditions + type Case struct { + pass bool + size uint8 + spot uint32 + data uint32 + } + cases := []Case{ + Case{true, 0, 0, 0}, + Case{true, 1, 4, 0}, + Case{true, 1, 65536, 0}, + Case{false, 1, 65536, 1}, // 1st byte out of bounds + Case{false, 1, 65537, 0}, // 2nd byte out of bounds + Case{true, 1, 65535, 1}, // last byte in bounds + Case{false, 1, 65535, 2}, // 1st byte over-run + Case{true, 2, 131072, 0}, + Case{false, 2, 131073, 0}, + } + for _, test := range cases { + args := []byte{} + if test.size > 0 { + args = append(args, test.size) + args = binary.LittleEndian.AppendUint32(args, test.spot) + args = binary.LittleEndian.AppendUint32(args, test.data) + } + if test.pass { + tx = l2info.PrepareTxTo("Owner", &memWrite, 1e9, nil, args) + ensure(tx, l2client.SendTransaction(ctx, tx)) + } else { + expectFailure(memWrite, args, nil) + } + } + + validateBlocks(t, 3, jit, builder) } func TestProgramActivateFails(t *testing.T) { From 9831b4a705d9b8d79d51e0507d923dc7c6678cdb Mon Sep 17 00:00:00 2001 From: Rachel Bousfield Date: Fri, 3 May 2024 00:47:22 -0600 Subject: [PATCH 3/5] skip for now --- system_tests/program_test.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/system_tests/program_test.go b/system_tests/program_test.go index e7eea226a..319e0bda8 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -926,7 +926,11 @@ func testMemory(t *testing.T, jit bool) { Fatal(t, "unexpected memory footprint", programMemoryFootprint) } - // check edge case where memory doesn't require `pay_for_memory_grow` + if !t.Failed() { + validateBlocks(t, 3, jit, builder) + t.Skip("Succeeded up to here. Diagnose tests with larger numbers of blocks later.") + } + /*// check edge case where memory doesn't require `pay_for_memory_grow` tx = l2info.PrepareTxTo("Owner", &growFixed, 1e9, nil, args) ensure(tx, l2client.SendTransaction(ctx, tx)) @@ -938,15 +942,15 @@ func testMemory(t *testing.T, jit bool) { data uint32 } cases := []Case{ - Case{true, 0, 0, 0}, - Case{true, 1, 4, 0}, - Case{true, 1, 65536, 0}, - Case{false, 1, 65536, 1}, // 1st byte out of bounds - Case{false, 1, 65537, 0}, // 2nd byte out of bounds - Case{true, 1, 65535, 1}, // last byte in bounds - Case{false, 1, 65535, 2}, // 1st byte over-run - Case{true, 2, 131072, 0}, - Case{false, 2, 131073, 0}, + {true, 0, 0, 0}, + {true, 1, 4, 0}, + {true, 1, 65536, 0}, + {false, 1, 65536, 1}, // 1st byte out of bounds + {false, 1, 65537, 0}, // 2nd byte out of bounds + {true, 1, 65535, 1}, // last byte in bounds + {false, 1, 65535, 2}, // 1st byte over-run + {true, 2, 131072, 0}, + {false, 2, 131073, 0}, } for _, test := range cases { args := []byte{} @@ -961,7 +965,9 @@ func testMemory(t *testing.T, jit bool) { } else { expectFailure(memWrite, args, nil) } - } + }*/ + _ = memWrite + _ = growFixed validateBlocks(t, 3, jit, builder) } From 18d70dfa64e5f4b8858a83abbf522cb9c9464958 Mon Sep 17 00:00:00 2001 From: Rachel Bousfield Date: Fri, 3 May 2024 01:16:07 -0600 Subject: [PATCH 4/5] fix waitForSequencer usage --- system_tests/program_norace_test.go | 30 +++++++++++++++++++++++------ system_tests/program_test.go | 6 ++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/system_tests/program_norace_test.go b/system_tests/program_norace_test.go index 8e95596b2..56b204671 100644 --- a/system_tests/program_norace_test.go +++ b/system_tests/program_norace_test.go @@ -25,6 +25,26 @@ import ( "github.com/offchainlabs/nitro/util/testhelpers" ) +func blockIsEmpty(block *types.Block) bool { + for _, tx := range block.Transactions() { + if tx.Type() != types.ArbitrumInternalTxType { + return false + } + } + return true +} + +func nonEmptyBlockHeight(t *testing.T, builder *NodeBuilder) uint64 { + latestBlock, err := builder.L2.Client.BlockByNumber(builder.ctx, nil) + Require(t, err) + for blockIsEmpty(latestBlock) { + prior := arbmath.BigSubByUint(latestBlock.Number(), 1) + latestBlock, err = builder.L2.Client.BlockByNumber(builder.ctx, prior) + Require(t, err) + } + return latestBlock.NumberU64() +} + // used in program test func validateBlocks( t *testing.T, start uint64, jit bool, builder *NodeBuilder, @@ -34,9 +54,7 @@ func validateBlocks( start = 1 } - blockHeight, err := builder.L2.Client.BlockNumber(builder.ctx) - Require(t, err) - + blockHeight := nonEmptyBlockHeight(t, builder) blocks := []uint64{} for i := start; i <= blockHeight; i++ { blocks = append(blocks, i) @@ -50,18 +68,18 @@ func validateBlockRange( builder *NodeBuilder, ) { ctx := builder.ctx - waitForSequencer(t, builder, arbmath.MaxInt(blocks...)) - blockHeight, err := builder.L2.Client.BlockNumber(ctx) - Require(t, err) // validate everything if jit { + blockHeight := nonEmptyBlockHeight(t, builder) blocks = []uint64{} for i := uint64(1); i <= blockHeight; i++ { blocks = append(blocks, i) } } + waitForSequencer(t, builder, arbmath.MaxInt(blocks...)) + success := true wasmModuleRoot := currentRootModule(t) for _, block := range blocks { diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 319e0bda8..1e033cecf 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -930,7 +930,7 @@ func testMemory(t *testing.T, jit bool) { validateBlocks(t, 3, jit, builder) t.Skip("Succeeded up to here. Diagnose tests with larger numbers of blocks later.") } - /*// check edge case where memory doesn't require `pay_for_memory_grow` + // check edge case where memory doesn't require `pay_for_memory_grow` tx = l2info.PrepareTxTo("Owner", &growFixed, 1e9, nil, args) ensure(tx, l2client.SendTransaction(ctx, tx)) @@ -965,9 +965,7 @@ func testMemory(t *testing.T, jit bool) { } else { expectFailure(memWrite, args, nil) } - }*/ - _ = memWrite - _ = growFixed + } validateBlocks(t, 3, jit, builder) } From 19739210dc74bd69594e32dff492046d37b853e8 Mon Sep 17 00:00:00 2001 From: Rachel Bousfield Date: Fri, 3 May 2024 01:25:43 -0600 Subject: [PATCH 5/5] remove skip --- arbnode/batch_poster.go | 2 +- system_tests/program_test.go | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index bca82cbd5..0a9a45cc1 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -1119,7 +1119,7 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error) } config := b.config() - forcePostBatch := time.Since(firstMsgTime) >= config.MaxDelay + forcePostBatch := config.MaxDelay <= 0 || time.Since(firstMsgTime) >= config.MaxDelay var l1BoundMaxBlockNumber uint64 = math.MaxUint64 var l1BoundMaxTimestamp uint64 = math.MaxUint64 diff --git a/system_tests/program_test.go b/system_tests/program_test.go index 1e033cecf..b20efe074 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -926,10 +926,6 @@ func testMemory(t *testing.T, jit bool) { Fatal(t, "unexpected memory footprint", programMemoryFootprint) } - if !t.Failed() { - validateBlocks(t, 3, jit, builder) - t.Skip("Succeeded up to here. Diagnose tests with larger numbers of blocks later.") - } // check edge case where memory doesn't require `pay_for_memory_grow` tx = l2info.PrepareTxTo("Owner", &growFixed, 1e9, nil, args) ensure(tx, l2client.SendTransaction(ctx, tx))