From 47053b95dfb3e9b516a177f70c91c5ccc176cef1 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 25 Jan 2024 13:08:33 -0700 Subject: [PATCH 1/4] In delayed sequencer, check accumulator against safe block hash --- arbnode/delayed.go | 41 +++++++++++++++++++++++++++++++----- arbnode/delayed_sequencer.go | 10 ++++++--- arbnode/inbox_reader.go | 3 ++- arbutil/wait_for_l1.go | 1 + 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/arbnode/delayed.go b/arbnode/delayed.go index 498aa0475f..2a1745c540 100644 --- a/arbnode/delayed.go +++ b/arbnode/delayed.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "fmt" "math/big" "sort" @@ -14,6 +15,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" @@ -28,6 +30,7 @@ var messageDeliveredID common.Hash var inboxMessageDeliveredID common.Hash var inboxMessageFromOriginID common.Hash var l2MessageFromOriginCallABI abi.Method +var delayedInboxAccsCallABI abi.Method func init() { parsedIBridgeABI, err := bridgegen.IBridgeMetaData.GetAbi() @@ -35,6 +38,7 @@ func init() { panic(err) } messageDeliveredID = parsedIBridgeABI.Events["MessageDelivered"].ID + delayedInboxAccsCallABI = parsedIBridgeABI.Methods["delayedInboxAccs"] parsedIMessageProviderABI, err := bridgegen.IDelayedMessageProviderMetaData.GetAbi() if err != nil { @@ -95,12 +99,39 @@ func (b *DelayedBridge) GetMessageCount(ctx context.Context, blockNumber *big.In return bigRes.Uint64(), nil } -func (b *DelayedBridge) GetAccumulator(ctx context.Context, sequenceNumber uint64, blockNumber *big.Int) (common.Hash, error) { - opts := &bind.CallOpts{ - Context: ctx, - BlockNumber: blockNumber, +// Uses blockHash if nonzero, otherwise uses blockNumber +func (b *DelayedBridge) GetAccumulator(ctx context.Context, sequenceNumber uint64, blockNumber *big.Int, blockHash common.Hash) (common.Hash, error) { + calldata := append([]byte{}, delayedInboxAccsCallABI.ID...) + inputs, err := delayedInboxAccsCallABI.Inputs.Pack(arbmath.UintToBig(sequenceNumber)) + if err != nil { + return common.Hash{}, err + } + calldata = append(calldata, inputs...) + msg := ethereum.CallMsg{ + To: &b.address, + Data: calldata, + } + var result hexutil.Bytes + if blockHash != (common.Hash{}) { + result, err = b.client.CallContractAtHash(ctx, msg, blockHash) + } else { + result, err = b.client.CallContract(ctx, msg, blockNumber) + } + if err != nil { + return common.Hash{}, err + } + values, err := delayedInboxAccsCallABI.Outputs.Unpack(result) + if err != nil { + return common.Hash{}, err + } + if len(values) != 1 { + return common.Hash{}, fmt.Errorf("expected 1 return value from %v, got %v", delayedInboxAccsCallABI.Name, len(values)) + } + hash, ok := values[0].([32]byte) + if !ok { + return common.Hash{}, fmt.Errorf("expected [32]uint8 return value from %v, got %T", delayedInboxAccsCallABI.Name, values[0]) } - return b.con.DelayedInboxAccs(opts, new(big.Int).SetUint64(sequenceNumber)) + return hash, nil } type DelayedInboxMessage struct { diff --git a/arbnode/delayed_sequencer.go b/arbnode/delayed_sequencer.go index f1b912e0f7..8cbb094c16 100644 --- a/arbnode/delayed_sequencer.go +++ b/arbnode/delayed_sequencer.go @@ -100,16 +100,20 @@ func (d *DelayedSequencer) sequenceWithoutLockout(ctx context.Context, lastBlock } var finalized uint64 + var finalizedHash common.Hash if config.UseMergeFinality && headerreader.HeaderIndicatesFinalitySupport(lastBlockHeader) { + var header *types.Header var err error if config.RequireFullFinality { - finalized, err = d.l1Reader.LatestFinalizedBlockNr(ctx) + header, err = d.l1Reader.LatestFinalizedBlockHeader(ctx) } else { - finalized, err = d.l1Reader.LatestSafeBlockNr(ctx) + header, err = d.l1Reader.LatestSafeBlockHeader(ctx) } if err != nil { return err } + finalized = header.Number.Uint64() + finalizedHash = header.Hash() } else { currentNum := lastBlockHeader.Number.Int64() if currentNum < config.FinalizeDistance { @@ -167,7 +171,7 @@ func (d *DelayedSequencer) sequenceWithoutLockout(ctx context.Context, lastBlock // Sequence the delayed messages, if any if len(messages) > 0 { - delayedBridgeAcc, err := d.bridge.GetAccumulator(ctx, pos-1, new(big.Int).SetUint64(finalized)) + delayedBridgeAcc, err := d.bridge.GetAccumulator(ctx, pos-1, new(big.Int).SetUint64(finalized), finalizedHash) if err != nil { return err } diff --git a/arbnode/inbox_reader.go b/arbnode/inbox_reader.go index 9c830e3c89..7a2ffa505c 100644 --- a/arbnode/inbox_reader.go +++ b/arbnode/inbox_reader.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "time" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" flag "github.com/spf13/pflag" @@ -299,7 +300,7 @@ func (r *InboxReader) run(ctx context.Context, hadError bool) error { } if checkingDelayedCount > 0 { checkingDelayedSeqNum := checkingDelayedCount - 1 - l1DelayedAcc, err := r.delayedBridge.GetAccumulator(ctx, checkingDelayedSeqNum, currentHeight) + l1DelayedAcc, err := r.delayedBridge.GetAccumulator(ctx, checkingDelayedSeqNum, currentHeight, common.Hash{}) if err != nil { return err } diff --git a/arbutil/wait_for_l1.go b/arbutil/wait_for_l1.go index b66710dbf0..a80502610b 100644 --- a/arbutil/wait_for_l1.go +++ b/arbutil/wait_for_l1.go @@ -23,6 +23,7 @@ type L1Interface interface { ethereum.TransactionReader TransactionSender(ctx context.Context, tx *types.Transaction, block common.Hash, index uint) (common.Address, error) BlockNumber(ctx context.Context) (uint64, error) + CallContractAtHash(ctx context.Context, msg ethereum.CallMsg, blockHash common.Hash) ([]byte, error) PendingCallContract(ctx context.Context, msg ethereum.CallMsg) ([]byte, error) ChainID(ctx context.Context) (*big.Int, error) } From 03138e0c783fb794afc0241b81f908869fd89cd7 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 1 Feb 2024 14:28:37 -0700 Subject: [PATCH 2/4] Pull in geth changes to parse ABI errors --- go-ethereum | 2 +- precompiles/precompile.go | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/go-ethereum b/go-ethereum index 1acd9c64ac..a0685a71f3 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 1acd9c64ac5804729475ef60aa578b4ec52fa0e6 +Subproject commit a0685a71f31c14f414c01cb5c7c91170fd0e84be diff --git a/precompiles/precompile.go b/precompiles/precompile.go index 5d2ecce745..175bb21902 100644 --- a/precompiles/precompile.go +++ b/precompiles/precompile.go @@ -96,12 +96,8 @@ func RenderSolError(solErr abi.Error, data []byte) (string, error) { if err != nil { return "", err } - valsRange, ok := vals.([]interface{}) - if !ok { - return "", errors.New("unexpected unpack result") - } - strVals := make([]string, 0, len(valsRange)) - for _, val := range valsRange { + strVals := make([]string, 0, len(vals)) + for _, val := range vals { strVals = append(strVals, fmt.Sprintf("%v", val)) } return fmt.Sprintf("error %v(%v)", solErr.Name, strings.Join(strVals, ", ")), nil From 74692529c3883e1f8341cede2f585da71323559a Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 1 Feb 2024 16:39:46 -0700 Subject: [PATCH 3/4] Fix tests --- system_tests/precompile_test.go | 8 ++++++-- system_tests/retryable_test.go | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/system_tests/precompile_test.go b/system_tests/precompile_test.go index e0a9c2ce78..0ad0f8f1e4 100644 --- a/system_tests/precompile_test.go +++ b/system_tests/precompile_test.go @@ -5,6 +5,7 @@ package arbtest import ( "context" + "fmt" "math/big" "testing" @@ -67,7 +68,9 @@ func TestCustomSolidityErrors(t *testing.T) { Fatal(t, "customRevert call should have errored") } observedMessage := customError.Error() - expectedMessage := "execution reverted: error Custom(1024, This spider family wards off bugs: /\\oo/\\ //\\(oo)/\\ /\\oo/\\, true)" + expectedError := "Custom(1024, This spider family wards off bugs: /\\oo/\\ //\\(oo)/\\ /\\oo/\\, true)" + // The first error is server side. The second error is client side ABI decoding. + expectedMessage := fmt.Sprintf("execution reverted: error %v: %v", expectedError, expectedError) if observedMessage != expectedMessage { Fatal(t, observedMessage) } @@ -79,7 +82,8 @@ func TestCustomSolidityErrors(t *testing.T) { Fatal(t, "out of range ArbBlockHash call should have errored") } observedMessage = customError.Error() - expectedMessage = "execution reverted: error InvalidBlockNumber(1000000000, 1)" + expectedError = "InvalidBlockNumber(1000000000, 1)" + expectedMessage = fmt.Sprintf("execution reverted: error %v: %v", expectedError, expectedError) if observedMessage != expectedMessage { Fatal(t, observedMessage) } diff --git a/system_tests/retryable_test.go b/system_tests/retryable_test.go index 4619671700..4e7bd2c7d8 100644 --- a/system_tests/retryable_test.go +++ b/system_tests/retryable_test.go @@ -121,7 +121,8 @@ func TestRetryableNoExist(t *testing.T) { arbRetryableTx, err := precompilesgen.NewArbRetryableTx(common.HexToAddress("6e"), builder.L2.Client) Require(t, err) _, err = arbRetryableTx.GetTimeout(&bind.CallOpts{}, common.Hash{}) - if err.Error() != "execution reverted: error NoTicketWithID()" { + // The first error is server side. The second error is client side ABI decoding. + if err.Error() != "execution reverted: error NoTicketWithID(): NoTicketWithID()" { Fatal(t, "didn't get expected NoTicketWithID error") } } From acc85b37dd2796fc49de3f15a3fd30c4749850e3 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Fri, 2 Feb 2024 10:31:44 -0700 Subject: [PATCH 4/4] Gate GetScheduledUpgrade to ArbOS 20 --- precompiles/precompile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/precompiles/precompile.go b/precompiles/precompile.go index 5d2ecce745..5a16a1f903 100644 --- a/precompiles/precompile.go +++ b/precompiles/precompile.go @@ -560,6 +560,7 @@ func Precompiles() map[addr]ArbosPrecompile { ArbOwnerPublic.methodsByName["GetInfraFeeAccount"].arbosVersion = 5 ArbOwnerPublic.methodsByName["RectifyChainOwner"].arbosVersion = 11 ArbOwnerPublic.methodsByName["GetBrotliCompressionLevel"].arbosVersion = 20 + ArbOwnerPublic.methodsByName["GetScheduledUpgrade"].arbosVersion = 20 ArbRetryableImpl := &ArbRetryableTx{Address: types.ArbRetryableTxAddress} ArbRetryable := insert(MakePrecompile(templates.ArbRetryableTxMetaData, ArbRetryableImpl))