-
Notifications
You must be signed in to change notification settings - Fork 444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Timeboost Bulk BlockMetadata API #2754
base: add-timeboosted-broadcastfeedmessage
Are you sure you want to change the base?
Changes from all commits
d60de53
712b3ad
ee5bfd6
3bb87ee
952cd37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package gethexec | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/ethereum/go-ethereum/common/hexutil" | ||
"github.com/offchainlabs/nitro/arbos/arbostypes" | ||
"github.com/offchainlabs/nitro/arbutil" | ||
"github.com/offchainlabs/nitro/util/containers" | ||
) | ||
|
||
type BlockMetadataFetcher interface { | ||
BlockMetadataAtCount(count arbutil.MessageIndex) (arbostypes.BlockMetadata, error) | ||
BlockNumberToMessageIndex(blockNum uint64) (arbutil.MessageIndex, error) | ||
MessageIndexToBlockNumber(messageNum arbutil.MessageIndex) uint64 | ||
} | ||
|
||
type BulkBlockMetadataFetcher struct { | ||
fetcher BlockMetadataFetcher | ||
cache *containers.LruCache[arbutil.MessageIndex, arbostypes.BlockMetadata] | ||
} | ||
|
||
func NewBulkBlockMetadataFetcher(fetcher BlockMetadataFetcher, cacheSize int) *BulkBlockMetadataFetcher { | ||
var cache *containers.LruCache[arbutil.MessageIndex, arbostypes.BlockMetadata] | ||
if cacheSize != 0 { | ||
cache = containers.NewLruCache[arbutil.MessageIndex, arbostypes.BlockMetadata](cacheSize) | ||
} | ||
return &BulkBlockMetadataFetcher{ | ||
fetcher: fetcher, | ||
cache: cache, | ||
} | ||
} | ||
|
||
func (b *BulkBlockMetadataFetcher) Fetch(fromBlock, toBlock hexutil.Uint64) ([]NumberAndBlockMetadata, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a reason for using |
||
start, err := b.fetcher.BlockNumberToMessageIndex(uint64(fromBlock)) | ||
if err != nil { | ||
return nil, fmt.Errorf("error converting fromBlock blocknumber to message index: %w", err) | ||
} | ||
end, err := b.fetcher.BlockNumberToMessageIndex(uint64(toBlock)) | ||
if err != nil { | ||
return nil, fmt.Errorf("error converting toBlock blocknumber to message index: %w", err) | ||
} | ||
var result []NumberAndBlockMetadata | ||
for i := start; i <= end; i++ { | ||
var data arbostypes.BlockMetadata | ||
var found bool | ||
if b.cache != nil { | ||
data, found = b.cache.Get(i) | ||
} | ||
if !found { | ||
data, err = b.fetcher.BlockMetadataAtCount(i + 1) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if data != nil && b.cache != nil { | ||
b.cache.Add(i, data) | ||
} | ||
} | ||
if data != nil { | ||
result = append(result, NumberAndBlockMetadata{ | ||
BlockNumber: b.fetcher.MessageIndexToBlockNumber(i), | ||
RawMetadata: (hexutil.Bytes)(data), | ||
}) | ||
} | ||
} | ||
return result, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,7 @@ type Config struct { | |
EnablePrefetchBlock bool `koanf:"enable-prefetch-block"` | ||
SyncMonitor SyncMonitorConfig `koanf:"sync-monitor"` | ||
StylusTarget StylusTargetConfig `koanf:"stylus-target"` | ||
BlockMetadataApiCacheSize int `koanf:"block-metadata-api-cache-size"` | ||
|
||
forwardingTarget string | ||
} | ||
|
@@ -82,6 +83,9 @@ func (c *Config) Validate() error { | |
if c.forwardingTarget != "" && c.Sequencer.Enable { | ||
return errors.New("ForwardingTarget set and sequencer enabled") | ||
} | ||
if c.BlockMetadataApiCacheSize < 0 { | ||
return errors.New("block-metadata-api-cache-size cannot be negative") | ||
} | ||
return nil | ||
} | ||
|
||
|
@@ -99,6 +103,9 @@ func ConfigAddOptions(prefix string, f *flag.FlagSet) { | |
f.Uint64(prefix+".tx-lookup-limit", ConfigDefault.TxLookupLimit, "retain the ability to lookup transactions by hash for the past N blocks (0 = all blocks)") | ||
f.Bool(prefix+".enable-prefetch-block", ConfigDefault.EnablePrefetchBlock, "enable prefetching of blocks") | ||
StylusTargetConfigAddOptions(prefix+".stylus-target", f) | ||
f.Int(prefix+".block-metadata-api-cache-size", ConfigDefault.BlockMetadataApiCacheSize, "size of lru cache storing the blockMetadata to service arb_getRawBlockMetadata.\n"+ | ||
"Note: setting a non-zero value would mean the blockMetadata might be outdated (if the block was reorged out).\n"+ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we somehow trigger clearing the cache on reorg? |
||
"Default is set to 0 which disables caching") | ||
} | ||
|
||
var ConfigDefault = Config{ | ||
|
@@ -114,6 +121,7 @@ var ConfigDefault = Config{ | |
Forwarder: DefaultNodeForwarderConfig, | ||
EnablePrefetchBlock: true, | ||
StylusTarget: DefaultStylusTargetConfig, | ||
BlockMetadataApiCacheSize: 0, | ||
} | ||
|
||
type ConfigFetcher func() *Config | ||
|
@@ -221,7 +229,7 @@ func CreateExecutionNode( | |
apis := []rpc.API{{ | ||
Namespace: "arb", | ||
Version: "1.0", | ||
Service: NewArbAPI(txPublisher), | ||
Service: NewArbAPI(txPublisher, NewBulkBlockMetadataFetcher(execEngine, config.BlockMetadataApiCacheSize)), | ||
Public: false, | ||
}} | ||
apis = append(apis, rpc.API{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
package arbtest | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"crypto/ecdsa" | ||
"encoding/binary" | ||
"fmt" | ||
"math/big" | ||
"net" | ||
|
@@ -41,6 +43,90 @@ import ( | |
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestTimeboosBulkBlockMetadataAPI(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
|
||
builder := NewNodeBuilder(ctx).DefaultConfig(t, false) | ||
cleanup := builder.Build(t) | ||
defer cleanup() | ||
|
||
arbDb := builder.L2.ConsensusNode.ArbDB | ||
dbKey := func(prefix []byte, pos uint64) []byte { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: looks like |
||
var key []byte | ||
key = append(key, prefix...) | ||
data := make([]byte, 8) | ||
binary.BigEndian.PutUint64(data, pos) | ||
key = append(key, data...) | ||
return key | ||
} | ||
|
||
start := 1 | ||
end := 20 | ||
var sampleBulkData []gethexec.NumberAndBlockMetadata | ||
for i := start; i <= end; i += 2 { | ||
sampleData := gethexec.NumberAndBlockMetadata{ | ||
BlockNumber: uint64(i), | ||
RawMetadata: []byte{0, uint8(i)}, | ||
} | ||
sampleBulkData = append(sampleBulkData, sampleData) | ||
arbDb.Put(dbKey([]byte("t"), sampleData.BlockNumber), sampleData.RawMetadata) | ||
} | ||
|
||
l2rpc := builder.L2.Stack.Attach() | ||
var result []gethexec.NumberAndBlockMetadata | ||
err := l2rpc.CallContext(ctx, &result, "arb_getRawBlockMetadata", hexutil.Uint64(start), hexutil.Uint64(end)) | ||
Require(t, err) | ||
|
||
if len(result) != len(sampleBulkData) { | ||
t.Fatalf("number of entries in arb_getRawBlockMetadata is incorrect. Got: %d, Want: %d", len(result), len(sampleBulkData)) | ||
} | ||
for i, data := range result { | ||
if data.BlockNumber != sampleBulkData[i].BlockNumber { | ||
t.Fatalf("BlockNumber mismatch. Got: %d, Want: %d", data.BlockNumber, sampleBulkData[i].BlockNumber) | ||
} | ||
if !bytes.Equal(data.RawMetadata, sampleBulkData[i].RawMetadata) { | ||
t.Fatalf("RawMetadata. Got: %s, Want: %s", data.RawMetadata, sampleBulkData[i].RawMetadata) | ||
} | ||
} | ||
|
||
// Test that without cache the result returned is always in sync with ArbDB | ||
sampleBulkData[0].RawMetadata = []byte{1, 11} | ||
arbDb.Put(dbKey([]byte("t"), 1), sampleBulkData[0].RawMetadata) | ||
|
||
err = l2rpc.CallContext(ctx, &result, "arb_getRawBlockMetadata", hexutil.Uint64(1), hexutil.Uint64(1)) | ||
Require(t, err) | ||
if len(result) != 1 { | ||
t.Fatal("result returned with more than one entry") | ||
} | ||
if !bytes.Equal(sampleBulkData[0].RawMetadata, result[0].RawMetadata) { | ||
t.Fatal("BlockMetadata gotten from API doesn't match the latest entry in ArbDB") | ||
} | ||
|
||
// Test that LRU caching works | ||
builder.execConfig.BlockMetadataApiCacheSize = 10 | ||
builder.RestartL2Node(t) | ||
l2rpc = builder.L2.Stack.Attach() | ||
err = l2rpc.CallContext(ctx, &result, "arb_getRawBlockMetadata", hexutil.Uint64(start), hexutil.Uint64(end)) | ||
Require(t, err) | ||
|
||
arbDb = builder.L2.ConsensusNode.ArbDB | ||
updatedBlockMetadata := []byte{2, 12} | ||
arbDb.Put(dbKey([]byte("t"), 1), updatedBlockMetadata) | ||
|
||
err = l2rpc.CallContext(ctx, &result, "arb_getRawBlockMetadata", hexutil.Uint64(1), hexutil.Uint64(1)) | ||
Require(t, err) | ||
if len(result) != 1 { | ||
t.Fatal("result returned with more than one entry") | ||
} | ||
if bytes.Equal(updatedBlockMetadata, result[0].RawMetadata) { | ||
t.Fatal("BlockMetadata should've been fetched from cache and not the db") | ||
} | ||
if !bytes.Equal(sampleBulkData[0].RawMetadata, result[0].RawMetadata) { | ||
t.Fatal("incorrect caching of BlockMetadata") | ||
} | ||
} | ||
|
||
func TestSequencerFeed_ExpressLaneAuction_ExpressLaneTxsHaveAdvantage_TimeboostedFieldIsCorrect(t *testing.T) { | ||
t.Parallel() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly to
(b *BulkBlockMetadataFetcher) Fetch
, is there a reason to not userpc.BlockNumber
here?