Merge pull request #230 from OffchainLabs/fixed-memory-edge-case

Memory Edge Cases

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()
+    }
+}

arbitrator/prover/src/programs/meter.rs

@@ -401,7 +401,7 @@ pub fn pricing_v1(op: &Operator, tys: &HashMap<SignatureIndex, FunctionType>) ->
         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,

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)
+)

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)
+)

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(())

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

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 {

system_tests/program_test.go

@@ -838,7 +838,9 @@ 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"))
+	memWrite := deployWasm(t, ctx, auth, l2client, watFile("grow/mem-write"))
 
 	expectFailure := func(to common.Address, data []byte, value *big.Int) {
 		t.Helper()
@@ -881,7 +883,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,7 +926,44 @@ func testMemory(t *testing.T, jit bool) {
 		Fatal(t, "unexpected memory footprint", programMemoryFootprint)
 	}
 
-	validateBlocks(t, 2, jit, builder)
+	// 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))
+
+	// check memory boundary conditions
+	type Case struct {
+		pass bool
+		size uint8
+		spot uint32
+		data uint32
+	}
+	cases := []Case{
+		{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{}
+		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) {