diff --git a/README.md b/README.md index ba2fbf2687..2cfef3de6a 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,8 @@ advanced calldata compression, and more. See the live docs-site [here](https://developer.arbitrum.io/) (or [here](https://github.com/OffchainLabs/arbitrum-docs) for markdown docs source.) +See [here](./audits) for security audit reports. + The Nitro stack is built on several innovations. At its core is a new prover, which can do Arbitrum’s classic interactive fraud proofs over WASM code. That means the L2 Arbitrum engine can be written and compiled using standard languages and tools, replacing the custom-designed language and compiler used in previous Arbitrum diff --git a/arbos/addressSet/addressSet.go b/arbos/addressSet/addressSet.go index a2945fc6f9..ae2e6a34c1 100644 --- a/arbos/addressSet/addressSet.go +++ b/arbos/addressSet/addressSet.go @@ -109,7 +109,7 @@ func (aset *AddressSet) Add(addr common.Address) error { return err } -func (aset *AddressSet) Remove(addr common.Address) error { +func (aset *AddressSet) Remove(addr common.Address, arbosVersion uint64) error { addrAsHash := common.BytesToHash(addr.Bytes()) slot, err := aset.byAddress.GetUint64(addrAsHash) if slot == 0 || err != nil { @@ -132,6 +132,12 @@ func (aset *AddressSet) Remove(addr common.Address) error { if err != nil { return err } + if arbosVersion >= 11 { + err = aset.byAddress.Set(atSize, util.UintToHash(slot)) + if err != nil { + return err + } + } } err = aset.backingStorage.ClearByUint64(size) if err != nil { diff --git a/arbos/addressSet/addressSet_test.go b/arbos/addressSet/addressSet_test.go index 030b388628..4296531f41 100644 --- a/arbos/addressSet/addressSet_test.go +++ b/arbos/addressSet/addressSet_test.go @@ -4,13 +4,15 @@ package addressSet import ( + "fmt" + "math/rand" "testing" "github.com/ethereum/go-ethereum/common/math" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/state" - "github.com/ethereum/go-ethereum/crypto" "github.com/offchainlabs/nitro/arbos/burn" "github.com/offchainlabs/nitro/arbos/storage" "github.com/offchainlabs/nitro/util/colors" @@ -21,6 +23,7 @@ func TestEmptyAddressSet(t *testing.T) { sto := storage.NewMemoryBacked(burn.NewSystemBurner(nil, false)) Require(t, Initialize(sto)) aset := OpenAddressSet(sto) + version := params.ArbitrumDevTestParams().InitialArbOSVersion if size(t, aset) != 0 { Fail(t) @@ -28,7 +31,7 @@ func TestEmptyAddressSet(t *testing.T) { if isMember(t, aset, common.Address{}) { Fail(t) } - err := aset.Remove(common.Address{}) + err := aset.Remove(common.Address{}, version) Require(t, err) if size(t, aset) != 0 { Fail(t) @@ -43,26 +46,31 @@ func TestAddressSet(t *testing.T) { sto := storage.NewGeth(db, burn.NewSystemBurner(nil, false)) Require(t, Initialize(sto)) aset := OpenAddressSet(sto) + version := params.ArbitrumDevTestParams().InitialArbOSVersion statedb, _ := (db).(*state.StateDB) stateHashBeforeChanges := statedb.IntermediateRoot(false) - addr1 := common.BytesToAddress(crypto.Keccak256([]byte{1})[:20]) - addr2 := common.BytesToAddress(crypto.Keccak256([]byte{2})[:20]) - addr3 := common.BytesToAddress(crypto.Keccak256([]byte{3})[:20]) + addr1 := testhelpers.RandomAddress() + addr2 := testhelpers.RandomAddress() + addr3 := testhelpers.RandomAddress() + possibleAddresses := []common.Address{addr1, addr2, addr3} Require(t, aset.Add(addr1)) if size(t, aset) != 1 { Fail(t) } + checkAllMembers(t, aset, possibleAddresses) Require(t, aset.Add(addr2)) if size(t, aset) != 2 { Fail(t) } + checkAllMembers(t, aset, possibleAddresses) Require(t, aset.Add(addr1)) if size(t, aset) != 2 { Fail(t) } + checkAllMembers(t, aset, possibleAddresses) if !isMember(t, aset, addr1) { Fail(t) } @@ -73,10 +81,11 @@ func TestAddressSet(t *testing.T) { Fail(t) } - Require(t, aset.Remove(addr1)) + Require(t, aset.Remove(addr1, version)) if size(t, aset) != 1 { Fail(t) } + checkAllMembers(t, aset, possibleAddresses) if isMember(t, aset, addr1) { Fail(t) } @@ -88,10 +97,12 @@ func TestAddressSet(t *testing.T) { if size(t, aset) != 2 { Fail(t) } - Require(t, aset.Remove(addr3)) + checkAllMembers(t, aset, possibleAddresses) + Require(t, aset.Remove(addr3, version)) if size(t, aset) != 1 { Fail(t) } + checkAllMembers(t, aset, possibleAddresses) Require(t, aset.Add(addr1)) all, err := aset.AllMembers(math.MaxUint64) @@ -125,6 +136,77 @@ func TestAddressSet(t *testing.T) { } } +func TestAddressSetAllMembers(t *testing.T) { + db := storage.NewMemoryBackedStateDB() + sto := storage.NewGeth(db, burn.NewSystemBurner(nil, false)) + Require(t, Initialize(sto)) + aset := OpenAddressSet(sto) + version := params.ArbitrumDevTestParams().InitialArbOSVersion + + addr1 := testhelpers.RandomAddress() + addr2 := testhelpers.RandomAddress() + addr3 := testhelpers.RandomAddress() + possibleAddresses := []common.Address{addr1, addr2, addr3} + + Require(t, aset.Add(addr1)) + checkAllMembers(t, aset, possibleAddresses) + Require(t, aset.Add(addr2)) + checkAllMembers(t, aset, possibleAddresses) + Require(t, aset.Remove(addr1, version)) + checkAllMembers(t, aset, possibleAddresses) + Require(t, aset.Add(addr3)) + checkAllMembers(t, aset, possibleAddresses) + Require(t, aset.Remove(addr2, version)) + checkAllMembers(t, aset, possibleAddresses) + + for i := 0; i < 512; i++ { + rem := rand.Intn(2) == 1 + addr := possibleAddresses[rand.Intn(len(possibleAddresses))] + if rem { + fmt.Printf("removing %v\n", addr) + Require(t, aset.Remove(addr, version)) + } else { + fmt.Printf("adding %v\n", addr) + Require(t, aset.Add(addr)) + } + checkAllMembers(t, aset, possibleAddresses) + } +} + +func checkAllMembers(t *testing.T, aset *AddressSet, possibleAddresses []common.Address) { + allMembers, err := aset.AllMembers(1024) + Require(t, err) + + allMembersSet := make(map[common.Address]struct{}) + for _, addr := range allMembers { + allMembersSet[addr] = struct{}{} + } + + if len(allMembers) != len(allMembersSet) { + Fail(t, "allMembers contains duplicates:", allMembers) + } + + possibleAddressSet := make(map[common.Address]struct{}) + for _, addr := range possibleAddresses { + possibleAddressSet[addr] = struct{}{} + } + for _, addr := range allMembers { + _, isPossible := possibleAddressSet[addr] + if !isPossible { + Fail(t, "allMembers contains impossible address", addr) + } + } + + for _, possible := range possibleAddresses { + isMember, err := aset.IsMember(possible) + Require(t, err) + _, inSet := allMembersSet[possible] + if isMember != inSet { + Fail(t, "IsMember", isMember, "does not match whether it's in the allMembers list", inSet) + } + } +} + func isMember(t *testing.T, aset *AddressSet, address common.Address) bool { t.Helper() present, err := aset.IsMember(address) diff --git a/audits/ConsenSys_Diligence_Arbitrum_Contracts_11_2021.pdf b/audits/ConsenSys_Diligence_Arbitrum_Contracts_11_2021.pdf new file mode 100644 index 0000000000..4e93ced017 Binary files /dev/null and b/audits/ConsenSys_Diligence_Arbitrum_Contracts_11_2021.pdf differ diff --git a/audits/ConsenSys_Diligence_Nitro_Contracts_5_2022.pdf b/audits/ConsenSys_Diligence_Nitro_Contracts_5_2022.pdf new file mode 100644 index 0000000000..7fb9bc8f61 Binary files /dev/null and b/audits/ConsenSys_Diligence_Nitro_Contracts_5_2022.pdf differ diff --git a/audits/Trail_Of_Bits_Nitro_10_2022.pdf b/audits/Trail_Of_Bits_Nitro_10_2022.pdf new file mode 100644 index 0000000000..06a0516928 Binary files /dev/null and b/audits/Trail_Of_Bits_Nitro_10_2022.pdf differ diff --git a/precompiles/ArbOwner.go b/precompiles/ArbOwner.go index e18f0a4858..bc0d8705ae 100644 --- a/precompiles/ArbOwner.go +++ b/precompiles/ArbOwner.go @@ -5,9 +5,10 @@ package precompiles import ( "errors" - "github.com/offchainlabs/nitro/arbos/l1pricing" "math/big" + "github.com/offchainlabs/nitro/arbos/l1pricing" + "github.com/ethereum/go-ethereum/common" ) @@ -36,7 +37,7 @@ func (con ArbOwner) RemoveChainOwner(c ctx, evm mech, addr addr) error { if !member { return errors.New("tried to remove non-owner") } - return c.State.ChainOwners().Remove(addr) + return c.State.ChainOwners().Remove(addr, c.State.ArbOSVersion()) } // IsChainOwner checks if the account is a chain owner