diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index acd6295b7c..b439fe4aec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,12 +87,12 @@ jobs: uses: actions/cache@v3 with: path: | - ~/.cargo/registry/ - ~/.cargo/git/ + ~/.cargo/ arbitrator/target/ arbitrator/wasm-libraries/target/ - arbitrator/wasm-libraries/soft-float/SoftFloat/build + arbitrator/wasm-libraries/soft-float/ target/etc/initial-machine-cache/ + /home/runner/.rustup/toolchains/ key: ${{ runner.os }}-cargo-${{ steps.install-rust.outputs.rustc_hash }}-min-${{ hashFiles('arbitrator/Cargo.lock') }}-${{ matrix.test-mode }} restore-keys: ${{ runner.os }}-cargo-${{ steps.install-rust.outputs.rustc_hash }}- @@ -166,19 +166,6 @@ jobs: fi done - - name: run tests with race detection and path state scheme - if: matrix.test-mode == 'race' - env: - TEST_STATE_SCHEME: path - run: | - packages=`go list ./...` - for package in $packages; do - echo running tests for $package - if ! stdbuf -oL gotestsum --format short-verbose --packages="$package" --rerun-fails=2 --no-color=false -- -race -timeout 30m > >(stdbuf -oL tee -a full.log | grep -vE "INFO|seal"); then - exit 1 - fi - done - - name: run tests with race detection and hash state scheme if: matrix.test-mode == 'race' env: diff --git a/LICENSE.md b/LICENSE.md index ea9a53da75..25768b3010 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -22,7 +22,7 @@ Additional Use Grant: You may use the Licensed Work in a production environment Expansion Program Term of Use](https://docs.arbitrum.foundation/assets/files/Arbitrum%20Expansion%20Program%20Jan182024-4f08b0c2cb476a55dc153380fa3e64b0.pdf). For purposes of this Additional Use Grant, the "Covered Arbitrum Chains" are (a) Arbitrum One (chainid:42161), Arbitrum Nova (chainid:42170), - rbitrum Rinkeby testnet/Rinkarby (chainid:421611),Arbitrum Nitro + Arbitrum Rinkeby testnet/Rinkarby (chainid:421611),Arbitrum Nitro Goerli testnet (chainid:421613), and Arbitrum Sepolia Testnet (chainid:421614); (b) any future blockchains authorized to be designated as Covered Arbitrum Chains by the decentralized autonomous diff --git a/Makefile b/Makefile index 0a71d64f12..c3cf1a5144 100644 --- a/Makefile +++ b/Makefile @@ -283,6 +283,7 @@ clean: rm -f arbitrator/wasm-libraries/soft-float/SoftFloat/build/Wasm-Clang/*.a rm -f arbitrator/wasm-libraries/forward/*.wat rm -rf arbitrator/stylus/tests/*/target/ arbitrator/stylus/tests/*/*.wasm + rm -rf brotli/buildfiles @rm -rf contracts/build contracts/cache solgen/go/ @rm -f .make/* diff --git a/arbcompress/native.go b/arbcompress/native.go index 8244010979..f7b8f0b8e0 100644 --- a/arbcompress/native.go +++ b/arbcompress/native.go @@ -7,7 +7,7 @@ package arbcompress /* -#cgo CFLAGS: -g -Wall -I${SRCDIR}/../target/include/ +#cgo CFLAGS: -g -I${SRCDIR}/../target/include/ #cgo LDFLAGS: ${SRCDIR}/../target/lib/libstylus.a -lm #include "arbitrator.h" */ diff --git a/arbitrator/Cargo.lock b/arbitrator/Cargo.lock index 2aa9864b08..2b437968fa 100644 --- a/arbitrator/Cargo.lock +++ b/arbitrator/Cargo.lock @@ -495,6 +495,12 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" +[[package]] +name = "clru" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cbd0f76e066e64fdc5631e3bb46381254deab9ef1158292f27c8c57e3bf3fe59" + [[package]] name = "colorchoice" version = "1.0.2" @@ -2206,13 +2212,13 @@ dependencies = [ "bincode", "brotli", "caller-env", + "clru", "derivative", "eyre", "fnv", "hex", "lazy_static", "libc", - "lru", "num-bigint", "parking_lot", "prover", diff --git a/arbitrator/Cargo.toml b/arbitrator/Cargo.toml index 94ca08b0b5..eaafb6e439 100644 --- a/arbitrator/Cargo.toml +++ b/arbitrator/Cargo.toml @@ -24,9 +24,7 @@ repository = "https://github.com/OffchainLabs/nitro.git" rust-version = "1.67" [workspace.dependencies] -cfg-if = "1.0.0" lazy_static = "1.4.0" -lru = "0.12.3" num_enum = { version = "0.7.2", default-features = false } ruint2 = "1.9.0" wasmparser = "0.121" diff --git a/arbitrator/arbutil/src/evm/req.rs b/arbitrator/arbutil/src/evm/req.rs index 1cfceda6b7..0304f2d378 100644 --- a/arbitrator/arbutil/src/evm/req.rs +++ b/arbitrator/arbutil/src/evm/req.rs @@ -298,9 +298,10 @@ impl> EvmApi for EvmApiRequestor { let mut request = Vec::with_capacity(2 * 8 + 3 * 2 + name.len() + args.len() + outs.len()); request.extend(start_ink.to_be_bytes()); request.extend(end_ink.to_be_bytes()); - request.extend((name.len() as u16).to_be_bytes()); - request.extend((args.len() as u16).to_be_bytes()); - request.extend((outs.len() as u16).to_be_bytes()); + // u32 is enough to represent the slices lengths because the WASM environment runs in 32 bits. + request.extend((name.len() as u32).to_be_bytes()); + request.extend((args.len() as u32).to_be_bytes()); + request.extend((outs.len() as u32).to_be_bytes()); request.extend(name.as_bytes()); request.extend(args); request.extend(outs); diff --git a/arbitrator/stylus/Cargo.toml b/arbitrator/stylus/Cargo.toml index 4717bd631a..ea1d878ea0 100644 --- a/arbitrator/stylus/Cargo.toml +++ b/arbitrator/stylus/Cargo.toml @@ -21,11 +21,11 @@ thiserror = "1.0.33" bincode = "1.3.3" lazy_static.workspace = true libc = "0.2.108" -lru.workspace = true eyre = "0.6.5" rand = "0.8.5" fnv = "1.0.7" hex = "0.4.3" +clru = "0.6.2" [dev-dependencies] num-bigint = "0.4.4" diff --git a/arbitrator/stylus/src/cache.rs b/arbitrator/stylus/src/cache.rs index fa38d45419..c1fdaaccee 100644 --- a/arbitrator/stylus/src/cache.rs +++ b/arbitrator/stylus/src/cache.rs @@ -2,18 +2,19 @@ // For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE use arbutil::Bytes32; +use clru::{CLruCache, CLruCacheConfig, WeightScale}; use eyre::Result; use lazy_static::lazy_static; -use lru::LruCache; use parking_lot::Mutex; use prover::programs::config::CompileConfig; +use std::hash::RandomState; use std::{collections::HashMap, num::NonZeroUsize}; use wasmer::{Engine, Module, Store}; use crate::target_cache::target_native; lazy_static! { - static ref INIT_CACHE: Mutex = Mutex::new(InitCache::new(256)); + static ref INIT_CACHE: Mutex = Mutex::new(InitCache::new(256 * 1024 * 1024)); } macro_rules! cache { @@ -22,9 +23,16 @@ macro_rules! cache { }; } +pub struct LruCounters { + pub hits: u32, + pub misses: u32, + pub does_not_fit: u32, +} + pub struct InitCache { long_term: HashMap, - lru: LruCache, + lru: CLruCache, + lru_counters: LruCounters, } #[derive(Clone, Copy, Hash, PartialEq, Eq)] @@ -48,11 +56,16 @@ impl CacheKey { struct CacheItem { module: Module, engine: Engine, + entry_size_estimate_bytes: usize, } impl CacheItem { - fn new(module: Module, engine: Engine) -> Self { - Self { module, engine } + fn new(module: Module, engine: Engine, entry_size_estimate_bytes: usize) -> Self { + Self { + module, + engine, + entry_size_estimate_bytes, + } } fn data(&self) -> (Module, Store) { @@ -60,23 +73,66 @@ impl CacheItem { } } +struct CustomWeightScale; +impl WeightScale for CustomWeightScale { + fn weight(&self, _key: &CacheKey, val: &CacheItem) -> usize { + // clru defines that each entry consumes (weight + 1) of the cache capacity. + // We subtract 1 since we only want to use the weight as the size of the entry. + val.entry_size_estimate_bytes.saturating_sub(1) + } +} + +#[repr(C)] +pub struct LruCacheMetrics { + pub size_bytes: u64, + pub count: u32, + pub hits: u32, + pub misses: u32, + pub does_not_fit: u32, +} + +pub fn deserialize_module( + module: &[u8], + version: u16, + debug: bool, +) -> Result<(Module, Engine, usize)> { + let engine = CompileConfig::version(version, debug).engine(target_native()); + let module = unsafe { Module::deserialize_unchecked(&engine, module)? }; + + let asm_size_estimate_bytes = module.serialize()?.len(); + // add 128 bytes for the cache item overhead + let entry_size_estimate_bytes = asm_size_estimate_bytes + 128; + + Ok((module, engine, entry_size_estimate_bytes)) +} + impl InitCache { // current implementation only has one tag that stores to the long_term // future implementations might have more, but 0 is a reserved tag // that will never modify long_term state const ARBOS_TAG: u32 = 1; - fn new(size: usize) -> Self { + const DOES_NOT_FIT_MSG: &'static str = "Failed to insert into LRU cache, item too large"; + + fn new(size_bytes: usize) -> Self { Self { long_term: HashMap::new(), - lru: LruCache::new(NonZeroUsize::new(size).unwrap()), + lru: CLruCache::with_config( + CLruCacheConfig::new(NonZeroUsize::new(size_bytes).unwrap()) + .with_scale(CustomWeightScale), + ), + lru_counters: LruCounters { + hits: 0, + misses: 0, + does_not_fit: 0, + }, } } - pub fn set_lru_size(size: u32) { + pub fn set_lru_capacity(capacity_bytes: u64) { cache!() .lru - .resize(NonZeroUsize::new(size.try_into().unwrap()).unwrap()) + .resize(NonZeroUsize::new(capacity_bytes.try_into().unwrap()).unwrap()) } /// Retrieves a cached value, updating items as necessary. @@ -91,8 +147,11 @@ impl InitCache { // See if the item is in the LRU cache, promoting if so if let Some(item) = cache.lru.get(&key) { - return Some(item.data()); + let data = item.data(); + cache.lru_counters.hits += 1; + return Some(data); } + cache.lru_counters.misses += 1; None } @@ -116,20 +175,24 @@ impl InitCache { if long_term_tag == Self::ARBOS_TAG { cache.long_term.insert(key, item.clone()); } else { - cache.lru.promote(&key) + // only calls get to move the key to the head of the LRU list + cache.lru.get(&key); } return Ok(item.data()); } drop(cache); - let engine = CompileConfig::version(version, debug).engine(target_native()); - let module = unsafe { Module::deserialize_unchecked(&engine, module)? }; + let (module, engine, entry_size_estimate_bytes) = + deserialize_module(module, version, debug)?; - let item = CacheItem::new(module, engine); + let item = CacheItem::new(module, engine, entry_size_estimate_bytes); let data = item.data(); let mut cache = cache!(); if long_term_tag != Self::ARBOS_TAG { - cache.lru.put(key, item); + if cache.lru.put_with_weight(key, item).is_err() { + cache.lru_counters.does_not_fit += 1; + eprintln!("{}", Self::DOES_NOT_FIT_MSG); + }; } else { cache.long_term.insert(key, item); } @@ -144,7 +207,9 @@ impl InitCache { let key = CacheKey::new(module_hash, version, debug); let mut cache = cache!(); if let Some(item) = cache.long_term.remove(&key) { - cache.lru.put(key, item); + if cache.lru.put_with_weight(key, item).is_err() { + eprintln!("{}", Self::DOES_NOT_FIT_MSG); + } } } @@ -155,7 +220,48 @@ impl InitCache { let mut cache = cache!(); let cache = &mut *cache; for (key, item) in cache.long_term.drain() { - cache.lru.put(key, item); // not all will fit, just a heuristic + // not all will fit, just a heuristic + if cache.lru.put_with_weight(key, item).is_err() { + eprintln!("{}", Self::DOES_NOT_FIT_MSG); + } } } + + pub fn get_lru_metrics() -> LruCacheMetrics { + let mut cache = cache!(); + + let count = cache.lru.len(); + let metrics = LruCacheMetrics { + // add 1 to each entry to account that we subtracted 1 in the weight calculation + size_bytes: (cache.lru.weight() + count).try_into().unwrap(), + + count: count.try_into().unwrap(), + + hits: cache.lru_counters.hits, + misses: cache.lru_counters.misses, + does_not_fit: cache.lru_counters.does_not_fit, + }; + + // Empty counters. + // go side, which is the only consumer of this function besides tests, + // will read those counters and increment its own prometheus counters with them. + cache.lru_counters = LruCounters { + hits: 0, + misses: 0, + does_not_fit: 0, + }; + + metrics + } + + // only used for testing + pub fn clear_lru_cache() { + let mut cache = cache!(); + cache.lru.clear(); + cache.lru_counters = LruCounters { + hits: 0, + misses: 0, + does_not_fit: 0, + }; + } } diff --git a/arbitrator/stylus/src/lib.rs b/arbitrator/stylus/src/lib.rs index 052bfc7229..abea428167 100644 --- a/arbitrator/stylus/src/lib.rs +++ b/arbitrator/stylus/src/lib.rs @@ -11,7 +11,7 @@ use arbutil::{ format::DebugBytes, Bytes32, }; -use cache::InitCache; +use cache::{deserialize_module, InitCache, LruCacheMetrics}; use evm_api::NativeRequestHandler; use eyre::ErrReport; use native::NativeInstance; @@ -309,10 +309,10 @@ pub unsafe extern "C" fn stylus_call( status } -/// resize lru +/// set lru cache capacity #[no_mangle] -pub extern "C" fn stylus_cache_lru_resize(size: u32) { - InitCache::set_lru_size(size); +pub extern "C" fn stylus_set_cache_lru_capacity(capacity_bytes: u64) { + InitCache::set_lru_capacity(capacity_bytes); } /// Caches an activated user program. @@ -363,3 +363,32 @@ pub unsafe extern "C" fn stylus_drop_vec(vec: RustBytes) { mem::drop(vec.into_vec()) } } + +/// Gets lru cache metrics. +#[no_mangle] +pub extern "C" fn stylus_get_lru_cache_metrics() -> LruCacheMetrics { + InitCache::get_lru_metrics() +} + +/// Clears lru cache. +/// Only used for testing purposes. +#[no_mangle] +pub extern "C" fn stylus_clear_lru_cache() { + InitCache::clear_lru_cache() +} + +/// Gets lru entry size in bytes. +/// Only used for testing purposes. +#[no_mangle] +pub extern "C" fn stylus_get_lru_entry_size_estimate_bytes( + module: GoSliceData, + version: u16, + debug: bool, +) -> u64 { + match deserialize_module(module.slice(), version, debug) { + Err(error) => panic!("tried to get invalid asm!: {error}"), + Ok((_, _, lru_entry_size_estimate_bytes)) => { + lru_entry_size_estimate_bytes.try_into().unwrap() + } + } +} diff --git a/arbitrator/stylus/tests/write-result-len.wat b/arbitrator/stylus/tests/write-result-len.wat new file mode 100644 index 0000000000..4c9ad35087 --- /dev/null +++ b/arbitrator/stylus/tests/write-result-len.wat @@ -0,0 +1,24 @@ +;; Copyright 2024, Offchain Labs, Inc. +;; For license information, see https://github.com/nitro/blob/master/LICENSE + +(module + (import "vm_hooks" "read_args" (func $read_args (param i32))) + (import "vm_hooks" "write_result" (func $write_result (param i32 i32))) + (memory (export "memory") 2 2) + (func $main (export "user_entrypoint") (param $args_len i32) (result i32) + (local $len i32) + + ;; write args to 0x0 + (call $read_args (i32.const 0)) + + ;; treat first 4 bytes as size to write + (i32.load (i32.const 0)) + local.set $len + + ;; call write + (call $write_result (i32.const 0) (local.get $len)) + + ;; return success + i32.const 0 + ) +) diff --git a/arbnode/inbox_test.go b/arbnode/inbox_test.go index d579b7c278..e588ef399b 100644 --- a/arbnode/inbox_test.go +++ b/arbnode/inbox_test.go @@ -74,7 +74,7 @@ func NewTransactionStreamerForTest(t *testing.T, ownerAddress common.Address) (* } stylusTargetConfig := &gethexec.DefaultStylusTargetConfig Require(t, stylusTargetConfig.Validate()) // pre-processes config (i.a. parses wasmTargets) - if err := execEngine.Initialize(gethexec.DefaultCachingConfig.StylusLRUCache, stylusTargetConfig); err != nil { + if err := execEngine.Initialize(gethexec.DefaultCachingConfig.StylusLRUCacheCapacity, &gethexec.DefaultStylusTargetConfig); err != nil { Fail(t, err) } execSeq := &execClientWrapper{execEngine, t} diff --git a/arbos/programs/api.go b/arbos/programs/api.go index 504289322f..3e59031b2d 100644 --- a/arbos/programs/api.go +++ b/arbos/programs/api.go @@ -400,9 +400,9 @@ func newApiClosures( } startInk := takeU64() endInk := takeU64() - nameLen := takeU16() - argsLen := takeU16() - outsLen := takeU16() + nameLen := takeU32() + argsLen := takeU32() + outsLen := takeU32() name := string(takeFixed(int(nameLen))) args := takeFixed(int(argsLen)) outs := takeFixed(int(outsLen)) diff --git a/arbos/programs/native.go b/arbos/programs/native.go index b13cfef2cf..5fbc512211 100644 --- a/arbos/programs/native.go +++ b/arbos/programs/native.go @@ -7,7 +7,7 @@ package programs /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #cgo LDFLAGS: ${SRCDIR}/../../target/lib/libstylus.a -ldl -lm #include "arbitrator.h" @@ -29,6 +29,7 @@ import ( "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/metrics" "github.com/offchainlabs/nitro/arbos/burn" "github.com/offchainlabs/nitro/arbos/util" "github.com/offchainlabs/nitro/arbutil" @@ -45,6 +46,14 @@ type bytes32 = C.Bytes32 type rustBytes = C.RustBytes type rustSlice = C.RustSlice +var ( + stylusLRUCacheSizeBytesGauge = metrics.NewRegisteredGauge("arb/arbos/stylus/cache/lru/size_bytes", nil) + stylusLRUCacheSizeCountGauge = metrics.NewRegisteredGauge("arb/arbos/stylus/cache/lru/count", nil) + stylusLRUCacheSizeHitsCounter = metrics.NewRegisteredCounter("arb/arbos/stylus/cache/lru/hits", nil) + stylusLRUCacheSizeMissesCounter = metrics.NewRegisteredCounter("arb/arbos/stylus/cache/lru/misses", nil) + stylusLRUCacheSizeDoesNotFitCounter = metrics.NewRegisteredCounter("arb/arbos/stylus/cache/lru/does_not_fit", nil) +) + func activateProgram( db vm.StateDB, program common.Address, @@ -320,8 +329,39 @@ func init() { } } -func ResizeWasmLruCache(size uint32) { - C.stylus_cache_lru_resize(u32(size)) +func SetWasmLruCacheCapacity(capacityBytes uint64) { + C.stylus_set_cache_lru_capacity(u64(capacityBytes)) +} + +// exported for testing +type WasmLruCacheMetrics struct { + SizeBytes uint64 + Count uint32 +} + +func GetWasmLruCacheMetrics() *WasmLruCacheMetrics { + metrics := C.stylus_get_lru_cache_metrics() + + stylusLRUCacheSizeBytesGauge.Update(int64(metrics.size_bytes)) + stylusLRUCacheSizeCountGauge.Update(int64(metrics.count)) + stylusLRUCacheSizeHitsCounter.Inc(int64(metrics.hits)) + stylusLRUCacheSizeMissesCounter.Inc(int64(metrics.misses)) + stylusLRUCacheSizeDoesNotFitCounter.Inc(int64(metrics.does_not_fit)) + + return &WasmLruCacheMetrics{ + SizeBytes: uint64(metrics.size_bytes), + Count: uint32(metrics.count), + } +} + +// Used for testing +func ClearWasmLruCache() { + C.stylus_clear_lru_cache() +} + +// Used for testing +func GetLruEntrySizeEstimateBytes(module []byte, version uint16, debug bool) uint64 { + return uint64(C.stylus_get_lru_entry_size_estimate_bytes(goSlice(module), u16(version), cbool(debug))) } const DefaultTargetDescriptionArm = "arm64-linux-unknown+neon" diff --git a/arbos/programs/native_api.go b/arbos/programs/native_api.go index 6fbb630ef3..6cecb8ef63 100644 --- a/arbos/programs/native_api.go +++ b/arbos/programs/native_api.go @@ -7,7 +7,7 @@ package programs /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #cgo LDFLAGS: ${SRCDIR}/../../target/lib/libstylus.a -ldl -lm #include "arbitrator.h" diff --git a/arbos/programs/programs.go b/arbos/programs/programs.go index 80fc139a9d..06ff4137da 100644 --- a/arbos/programs/programs.go +++ b/arbos/programs/programs.go @@ -127,6 +127,7 @@ func (p Programs) ActivateProgram(evm *vm.EVM, address common.Address, arbosVers if err != nil { return 0, codeHash, common.Hash{}, nil, true, err } + evictProgram(statedb, oldModuleHash, currentVersion, debugMode, runMode, expired) } if err := p.moduleHashes.Set(codeHash, info.moduleHash); err != nil { diff --git a/arbos/programs/testcompile.go b/arbos/programs/testcompile.go index 1daf470620..615b0f3f72 100644 --- a/arbos/programs/testcompile.go +++ b/arbos/programs/testcompile.go @@ -9,7 +9,7 @@ package programs // This file exists because cgo isn't allowed in tests /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #include "arbitrator.h" typedef uint16_t u16; diff --git a/arbos/programs/testconstants.go b/arbos/programs/testconstants.go index 1ab0e6e93b..44f69a52de 100644 --- a/arbos/programs/testconstants.go +++ b/arbos/programs/testconstants.go @@ -9,7 +9,7 @@ package programs // This file exists because cgo isn't allowed in tests /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #include "arbitrator.h" */ import "C" diff --git a/arbos/programs/wasm.go b/arbos/programs/wasm.go index 8f79944fbe..12c23a724c 100644 --- a/arbos/programs/wasm.go +++ b/arbos/programs/wasm.go @@ -154,6 +154,8 @@ func callProgram( return retData, err } +func GetWasmLruCacheMetrics() {} + func CallProgramLoop( moduleHash common.Hash, calldata []byte, diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index b08c7c5d30..d6c35339f6 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -532,6 +532,20 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { refund := func(refundFrom common.Address, amount *big.Int) { const errLog = "fee address doesn't have enough funds to give user refund" + logMissingRefund := func(err error) { + if !errors.Is(err, vm.ErrInsufficientBalance) { + log.Error("unexpected error refunding balance", "err", err, "feeAddress", refundFrom) + return + } + logLevel := log.Error + isContract := p.evm.StateDB.GetCodeSize(refundFrom) > 0 + if isContract { + // It's expected that the balance might not still be in this address if it's a contract. + logLevel = log.Debug + } + logLevel(errLog, "err", err, "feeAddress", refundFrom) + } + // Refund funds to the fee refund address without overdrafting the L1 deposit. toRefundAddr := takeFunds(maxRefund, amount) err = util.TransferBalance(&refundFrom, &inner.RefundTo, toRefundAddr, p.evm, scenario, "refund") @@ -539,13 +553,13 @@ func (p *TxProcessor) EndTxHook(gasLeft uint64, success bool) { // Normally the network fee address should be holding any collected fees. // However, in theory, they could've been transferred out during the redeem attempt. // If the network fee address doesn't have the necessary balance, log an error and don't give a refund. - log.Error(errLog, "err", err, "feeAddress", refundFrom) + logMissingRefund(err) } // Any extra refund can't be given to the fee refund address if it didn't come from the L1 deposit. // Instead, give the refund to the retryable from address. err = util.TransferBalance(&refundFrom, &inner.From, arbmath.BigSub(amount, toRefundAddr), p.evm, scenario, "refund") if err != nil { - log.Error(errLog, "err", err, "feeAddress", refundFrom) + logMissingRefund(err) } } diff --git a/arbos/util/storage_cache.go b/arbos/util/storage_cache.go index bf05a5824d..9573d1ffc7 100644 --- a/arbos/util/storage_cache.go +++ b/arbos/util/storage_cache.go @@ -5,6 +5,7 @@ package util import ( "github.com/ethereum/go-ethereum/common" + "slices" ) type storageCacheEntry struct { @@ -67,6 +68,10 @@ func (s *storageCache) Flush() []storageCacheStores { }) } } + sortFunc := func(a, b storageCacheStores) int { + return a.Key.Cmp(b.Key) + } + slices.SortFunc(stores, sortFunc) return stores } diff --git a/arbos/util/storage_cache_test.go b/arbos/util/storage_cache_test.go index 1cc4ea14ec..9fd452851d 100644 --- a/arbos/util/storage_cache_test.go +++ b/arbos/util/storage_cache_test.go @@ -4,7 +4,6 @@ package util import ( - "bytes" "slices" "testing" @@ -76,7 +75,7 @@ func TestStorageCache(t *testing.T) { {Key: keys[2], Value: values[2]}, } sortFunc := func(a, b storageCacheStores) int { - return bytes.Compare(a.Key.Bytes(), b.Key.Bytes()) + return a.Key.Cmp(b.Key) } slices.SortFunc(stores, sortFunc) slices.SortFunc(expected, sortFunc) diff --git a/cmd/nitro/nitro.go b/cmd/nitro/nitro.go index 1078f44808..bc2155a475 100644 --- a/cmd/nitro/nitro.go +++ b/cmd/nitro/nitro.go @@ -249,7 +249,7 @@ func mainImpl() int { // If sequencer and signing is enabled or batchposter is enabled without // external signing sequencer will need a key. sequencerNeedsKey := (nodeConfig.Node.Sequencer && !nodeConfig.Node.Feed.Output.DisableSigning) || - (nodeConfig.Node.BatchPoster.Enable && nodeConfig.Node.BatchPoster.DataPoster.ExternalSigner.URL == "") + (nodeConfig.Node.BatchPoster.Enable && (nodeConfig.Node.BatchPoster.DataPoster.ExternalSigner.URL == "" || nodeConfig.Node.DataAvailability.Enable)) validatorNeedsKey := nodeConfig.Node.Staker.OnlyCreateWalletContract || (nodeConfig.Node.Staker.Enable && !strings.EqualFold(nodeConfig.Node.Staker.Strategy, "watchtower") && nodeConfig.Node.Staker.DataPoster.ExternalSigner.URL == "") @@ -1010,14 +1010,15 @@ func initReorg(initConfig conf.InitConfig, chainConfig *params.ChainConfig, inbo return nil } // Reorg out the batch containing the next message - var missing bool + var found bool var err error - batchCount, missing, err = inboxTracker.FindInboxBatchContainingMessage(messageIndex + 1) + batchCount, found, err = inboxTracker.FindInboxBatchContainingMessage(messageIndex + 1) if err != nil { return err } - if missing { - return fmt.Errorf("cannot reorg to unknown message index %v", messageIndex) + if !found { + log.Warn("init-reorg: no need to reorg, because message ahead of chain", "messageIndex", messageIndex) + return nil } } return inboxTracker.ReorgBatchesTo(batchCount) diff --git a/execution/gethexec/blockchain.go b/execution/gethexec/blockchain.go index 98ce4811d1..9b0c1a6f2f 100644 --- a/execution/gethexec/blockchain.go +++ b/execution/gethexec/blockchain.go @@ -37,7 +37,7 @@ type CachingConfig struct { SnapshotRestoreGasLimit uint64 `koanf:"snapshot-restore-gas-limit"` MaxNumberOfBlocksToSkipStateSaving uint32 `koanf:"max-number-of-blocks-to-skip-state-saving"` MaxAmountOfGasToSkipStateSaving uint64 `koanf:"max-amount-of-gas-to-skip-state-saving"` - StylusLRUCache uint32 `koanf:"stylus-lru-cache"` + StylusLRUCacheCapacity uint32 `koanf:"stylus-lru-cache-capacity"` StateScheme string `koanf:"state-scheme"` StateHistory uint64 `koanf:"state-history"` } @@ -54,7 +54,7 @@ func CachingConfigAddOptions(prefix string, f *flag.FlagSet) { f.Uint64(prefix+".snapshot-restore-gas-limit", DefaultCachingConfig.SnapshotRestoreGasLimit, "maximum gas rolled back to recover snapshot") f.Uint32(prefix+".max-number-of-blocks-to-skip-state-saving", DefaultCachingConfig.MaxNumberOfBlocksToSkipStateSaving, "maximum number of blocks to skip state saving to persistent storage (archive node only) -- warning: this option seems to cause issues") f.Uint64(prefix+".max-amount-of-gas-to-skip-state-saving", DefaultCachingConfig.MaxAmountOfGasToSkipStateSaving, "maximum amount of gas in blocks to skip saving state to Persistent storage (archive node only) -- warning: this option seems to cause issues") - f.Uint32(prefix+".stylus-lru-cache", DefaultCachingConfig.StylusLRUCache, "initialized stylus programs to keep in LRU cache") + f.Uint32(prefix+".stylus-lru-cache-capacity", DefaultCachingConfig.StylusLRUCacheCapacity, "capacity, in megabytes, of the LRU cache that keeps initialized stylus programs") f.String(prefix+".state-scheme", DefaultCachingConfig.StateScheme, "scheme to use for state trie storage (hash, path)") f.Uint64(prefix+".state-history", DefaultCachingConfig.StateHistory, "number of recent blocks to retain state history for (path state-scheme only)") } @@ -76,7 +76,7 @@ var DefaultCachingConfig = CachingConfig{ SnapshotRestoreGasLimit: 300_000_000_000, MaxNumberOfBlocksToSkipStateSaving: 0, MaxAmountOfGasToSkipStateSaving: 0, - StylusLRUCache: 256, + StylusLRUCacheCapacity: 256, StateScheme: rawdb.HashScheme, StateHistory: getStateHistory(DefaultSequencerConfig.MaxBlockSpeed), } diff --git a/execution/gethexec/executionengine.go b/execution/gethexec/executionengine.go index 8d6484e3c9..a0f3a2f59a 100644 --- a/execution/gethexec/executionengine.go +++ b/execution/gethexec/executionengine.go @@ -7,7 +7,7 @@ package gethexec /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #cgo LDFLAGS: ${SRCDIR}/../../target/lib/libstylus.a -ldl -lm #include "arbitrator.h" */ @@ -182,9 +182,9 @@ func PopulateStylusTargetCache(targetConfig *StylusTargetConfig) error { return nil } -func (s *ExecutionEngine) Initialize(rustCacheSize uint32, targetConfig *StylusTargetConfig) error { - if rustCacheSize != 0 { - programs.ResizeWasmLruCache(rustCacheSize) +func (s *ExecutionEngine) Initialize(rustCacheCapacityMB uint32, targetConfig *StylusTargetConfig) error { + if rustCacheCapacityMB != 0 { + programs.SetWasmLruCacheCapacity(arbmath.SaturatingUMul(uint64(rustCacheCapacityMB), 1024*1024)) } if err := PopulateStylusTargetCache(targetConfig); err != nil { return fmt.Errorf("error populating stylus target cache: %w", err) @@ -963,4 +963,15 @@ func (s *ExecutionEngine) Start(ctx_in context.Context) { } } }) + // periodically update stylus lru cache metrics + s.LaunchThread(func(ctx context.Context) { + for { + select { + case <-ctx.Done(): + return + case <-time.After(time.Minute): + programs.GetWasmLruCacheMetrics() + } + } + }) } diff --git a/execution/gethexec/node.go b/execution/gethexec/node.go index a88cbf15b9..5a1efc6d08 100644 --- a/execution/gethexec/node.go +++ b/execution/gethexec/node.go @@ -314,7 +314,7 @@ func (n *ExecutionNode) MarkFeedStart(to arbutil.MessageIndex) { func (n *ExecutionNode) Initialize(ctx context.Context) error { config := n.ConfigFetcher() - err := n.ExecEngine.Initialize(config.Caching.StylusLRUCache, &config.StylusTarget) + err := n.ExecEngine.Initialize(config.Caching.StylusLRUCacheCapacity, &config.StylusTarget) if err != nil { return fmt.Errorf("error initializing execution engine: %w", err) } diff --git a/system_tests/common_test.go b/system_tests/common_test.go index 9125f00e04..ed098351e5 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -167,7 +167,7 @@ var TestCachingConfig = gethexec.CachingConfig{ SnapshotRestoreGasLimit: 300_000_000_000, MaxNumberOfBlocksToSkipStateSaving: 0, MaxAmountOfGasToSkipStateSaving: 0, - StylusLRUCache: 0, + StylusLRUCacheCapacity: 0, StateScheme: env.GetTestStateScheme(), } diff --git a/system_tests/program_test.go b/system_tests/program_test.go index aa5c96ef3c..cf8cd72559 100644 --- a/system_tests/program_test.go +++ b/system_tests/program_test.go @@ -2012,3 +2012,128 @@ func checkWasmStoreContent(t *testing.T, wasmDb ethdb.KeyValueStore, targets []s } } } + +func deployWasmAndGetLruEntrySizeEstimateBytes( + t *testing.T, + builder *NodeBuilder, + auth bind.TransactOpts, + wasmName string, +) (common.Address, uint64) { + ctx := builder.ctx + l2client := builder.L2.Client + + wasm, _ := readWasmFile(t, rustFile(wasmName)) + arbWasm, err := pgen.NewArbWasm(types.ArbWasmAddress, l2client) + Require(t, err, ", wasmName:", wasmName) + + programAddress := deployContract(t, ctx, auth, l2client, wasm) + tx, err := arbWasm.ActivateProgram(&auth, programAddress) + Require(t, err, ", wasmName:", wasmName) + receipt, err := EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err, ", wasmName:", wasmName) + + if len(receipt.Logs) != 1 { + Fatal(t, "expected 1 log while activating, got ", len(receipt.Logs), ", wasmName:", wasmName) + } + log, err := arbWasm.ParseProgramActivated(*receipt.Logs[0]) + Require(t, err, ", wasmName:", wasmName) + + statedb, err := builder.L2.ExecNode.Backend.ArbInterface().BlockChain().State() + Require(t, err, ", wasmName:", wasmName) + + module, err := statedb.TryGetActivatedAsm(rawdb.LocalTarget(), log.ModuleHash) + Require(t, err, ", wasmName:", wasmName) + + lruEntrySizeEstimateBytes := programs.GetLruEntrySizeEstimateBytes(module, log.Version, true) + // just a sanity check + if lruEntrySizeEstimateBytes == 0 { + Fatal(t, "lruEntrySizeEstimateBytes is 0, wasmName:", wasmName) + } + return programAddress, lruEntrySizeEstimateBytes +} + +func TestWasmLruCache(t *testing.T) { + builder, auth, cleanup := setupProgramTest(t, true) + ctx := builder.ctx + l2info := builder.L2Info + l2client := builder.L2.Client + defer cleanup() + + auth.GasLimit = 32000000 + auth.Value = oneEth + + fallibleProgramAddress, fallibleLruEntrySizeEstimateBytes := deployWasmAndGetLruEntrySizeEstimateBytes(t, builder, auth, "fallible") + keccakProgramAddress, keccakLruEntrySizeEstimateBytes := deployWasmAndGetLruEntrySizeEstimateBytes(t, builder, auth, "keccak") + mathProgramAddress, mathLruEntrySizeEstimateBytes := deployWasmAndGetLruEntrySizeEstimateBytes(t, builder, auth, "math") + t.Log( + "lruEntrySizeEstimateBytes, ", + "fallible:", fallibleLruEntrySizeEstimateBytes, + "keccak:", keccakLruEntrySizeEstimateBytes, + "math:", mathLruEntrySizeEstimateBytes, + ) + + programs.ClearWasmLruCache() + lruMetrics := programs.GetWasmLruCacheMetrics() + if lruMetrics.Count != 0 { + t.Fatalf("lruMetrics.Count, expected: %v, actual: %v", 0, lruMetrics.Count) + } + if lruMetrics.SizeBytes != 0 { + t.Fatalf("lruMetrics.SizeBytes, expected: %v, actual: %v", 0, lruMetrics.SizeBytes) + } + + programs.SetWasmLruCacheCapacity(fallibleLruEntrySizeEstimateBytes - 1) + // fallible wasm program will not be cached since its size is greater than lru cache capacity + tx := l2info.PrepareTxTo("Owner", &fallibleProgramAddress, l2info.TransferGas, nil, []byte{0x01}) + Require(t, l2client.SendTransaction(ctx, tx)) + _, err := EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err) + lruMetrics = programs.GetWasmLruCacheMetrics() + if lruMetrics.Count != 0 { + t.Fatalf("lruMetrics.Count, expected: %v, actual: %v", 0, lruMetrics.Count) + } + if lruMetrics.SizeBytes != 0 { + t.Fatalf("lruMetrics.SizeBytes, expected: %v, actual: %v", 0, lruMetrics.SizeBytes) + } + + programs.SetWasmLruCacheCapacity( + fallibleLruEntrySizeEstimateBytes + keccakLruEntrySizeEstimateBytes + mathLruEntrySizeEstimateBytes - 1, + ) + // fallible wasm program will be cached + tx = l2info.PrepareTxTo("Owner", &fallibleProgramAddress, l2info.TransferGas, nil, []byte{0x01}) + Require(t, l2client.SendTransaction(ctx, tx)) + _, err = EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err) + lruMetrics = programs.GetWasmLruCacheMetrics() + if lruMetrics.Count != 1 { + t.Fatalf("lruMetrics.Count, expected: %v, actual: %v", 1, lruMetrics.Count) + } + if lruMetrics.SizeBytes != fallibleLruEntrySizeEstimateBytes { + t.Fatalf("lruMetrics.SizeBytes, expected: %v, actual: %v", fallibleLruEntrySizeEstimateBytes, lruMetrics.SizeBytes) + } + + // keccak wasm program will be cached + tx = l2info.PrepareTxTo("Owner", &keccakProgramAddress, l2info.TransferGas, nil, []byte{0x01}) + Require(t, l2client.SendTransaction(ctx, tx)) + _, err = EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err) + lruMetrics = programs.GetWasmLruCacheMetrics() + if lruMetrics.Count != 2 { + t.Fatalf("lruMetrics.Count, expected: %v, actual: %v", 2, lruMetrics.Count) + } + if lruMetrics.SizeBytes != fallibleLruEntrySizeEstimateBytes+keccakLruEntrySizeEstimateBytes { + t.Fatalf("lruMetrics.SizeBytes, expected: %v, actual: %v", fallibleLruEntrySizeEstimateBytes+keccakLruEntrySizeEstimateBytes, lruMetrics.SizeBytes) + } + + // math wasm program will be cached, but fallible will be evicted since (fallible + keccak + math) > lruCacheCapacity + tx = l2info.PrepareTxTo("Owner", &mathProgramAddress, l2info.TransferGas, nil, []byte{0x01}) + Require(t, l2client.SendTransaction(ctx, tx)) + _, err = EnsureTxSucceeded(ctx, l2client, tx) + Require(t, err) + lruMetrics = programs.GetWasmLruCacheMetrics() + if lruMetrics.Count != 2 { + t.Fatalf("lruMetrics.Count, expected: %v, actual: %v", 2, lruMetrics.Count) + } + if lruMetrics.SizeBytes != keccakLruEntrySizeEstimateBytes+mathLruEntrySizeEstimateBytes { + t.Fatalf("lruMetrics.SizeBytes, expected: %v, actual: %v", keccakLruEntrySizeEstimateBytes+mathLruEntrySizeEstimateBytes, lruMetrics.SizeBytes) + } +} diff --git a/system_tests/stylus_trace_test.go b/system_tests/stylus_trace_test.go index 5c4463d9f7..52039df460 100644 --- a/system_tests/stylus_trace_test.go +++ b/system_tests/stylus_trace_test.go @@ -6,6 +6,7 @@ package arbtest import ( "bytes" "encoding/binary" + "math" "math/big" "testing" @@ -478,3 +479,17 @@ func TestStylusOpcodeTraceEquivalence(t *testing.T) { checkOpcode(t, wasmResult, 12, vm.RETURN, offset, returnLen) checkOpcode(t, evmResult, 5078, vm.RETURN, offset, returnLen) } + +func TestStylusHugeWriteResultTrace(t *testing.T) { + const jit = false + builder, auth, cleanup := setupProgramTest(t, jit) + ctx := builder.ctx + l2client := builder.L2.Client + defer cleanup() + + program := deployWasm(t, ctx, auth, l2client, watFile("write-result-len")) + const returnLen = math.MaxUint16 + 1 + args := binary.LittleEndian.AppendUint32(nil, returnLen) + result := sendAndTraceTransaction(t, builder, program, nil, args) + checkOpcode(t, result, 3, vm.RETURN, nil, intToBe32(returnLen)) +} diff --git a/validator/server_arb/machine.go b/validator/server_arb/machine.go index adca9695e2..1e73e6b212 100644 --- a/validator/server_arb/machine.go +++ b/validator/server_arb/machine.go @@ -4,7 +4,7 @@ package server_arb /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #include "arbitrator.h" ResolvedPreimage preimageResolverC(size_t context, uint8_t preimageType, const uint8_t* hash); diff --git a/validator/server_arb/nitro_machine.go b/validator/server_arb/nitro_machine.go index 2b2cb230b6..926b1e8930 100644 --- a/validator/server_arb/nitro_machine.go +++ b/validator/server_arb/nitro_machine.go @@ -4,7 +4,7 @@ package server_arb /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #include "arbitrator.h" #include */ diff --git a/validator/server_arb/preimage_resolver.go b/validator/server_arb/preimage_resolver.go index cd4ea40e28..f01d79f4dd 100644 --- a/validator/server_arb/preimage_resolver.go +++ b/validator/server_arb/preimage_resolver.go @@ -4,7 +4,7 @@ package server_arb /* -#cgo CFLAGS: -g -Wall -I../../target/include/ +#cgo CFLAGS: -g -I../../target/include/ #include "arbitrator.h" extern ResolvedPreimage preimageResolver(size_t context, uint8_t preimageType, const uint8_t* hash); diff --git a/validator/server_arb/prover_interface.go b/validator/server_arb/prover_interface.go index bdd81ed588..3010d2138d 100644 --- a/validator/server_arb/prover_interface.go +++ b/validator/server_arb/prover_interface.go @@ -4,7 +4,7 @@ package server_arb /* -#cgo CFLAGS: -g -Wall -I../target/include/ +#cgo CFLAGS: -g -I../target/include/ #cgo LDFLAGS: ${SRCDIR}/../../target/lib/libstylus.a -ldl -lm #include "arbitrator.h" #include