-
Notifications
You must be signed in to change notification settings - Fork 38
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
Host: Small cache of blocks by hash #1738
Changes from 2 commits
99448b2
609496e
70dd301
7904ffd
f083c16
7778ef7
8f564c5
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,6 +8,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"math/big" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"time" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/golang-lru/v2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"github.com/ethereum/go-ethereum/accounts/abi/bind" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gethlog "github.com/ethereum/go-ethereum/log" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -30,15 +32,17 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
connRetryInterval = 500 * time.Millisecond | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_maxRetryPriceIncreases = 5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_retryPriceMultiplier = 1.2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
_defaultBlockCacheSize = 51 // enough for 50 request batch size and one for previous block | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// gethRPCClient implements the EthClient interface and allows connection to a real ethereum node | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type gethRPCClient struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
client *ethclient.Client // the underlying eth rpc client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
l2ID gethcommon.Address // the address of the Obscuro node this client is dedicated to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
timeout time.Duration // the timeout for connecting to, or communicating with, the L1 node | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger gethlog.Logger | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rpcURL string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
client *ethclient.Client // the underlying eth rpc client | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
l2ID gethcommon.Address // the address of the Obscuro node this client is dedicated to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
timeout time.Duration // the timeout for connecting to, or communicating with, the L1 node | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger gethlog.Logger | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rpcURL string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
blockCache *lru.Cache[gethcommon.Hash, *types.Block] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// NewEthClientFromURL instantiates a new ethadapter.EthClient that connects to an ethereum node | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -49,12 +53,17 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger.Trace(fmt.Sprintf("Initialized eth node connection - addr: %s", rpcURL)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// cache recent blocks to avoid re-fetching them (they are often re-used for checking for forks etc.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
blkCache, _ := lru.New[gethcommon.Hash, *types.Block](_defaultBlockCacheSize) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. The cache is initialized without error handling. While - blkCache, _ := lru.New[gethcommon.Hash, *types.Block](_defaultBlockCacheSize)
+ blkCache, err := lru.New[gethcommon.Hash, *types.Block](_defaultBlockCacheSize)
+ if err != nil {
+ return nil, fmt.Errorf("failed to create block cache: %w", err)
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return &gethRPCClient{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
client: client, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
l2ID: l2ID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
timeout: timeout, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger: logger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rpcURL: rpcURL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
client: client, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
l2ID: l2ID, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
timeout: timeout, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
logger: logger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rpcURL: rpcURL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
blockCache: blkCache, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -181,10 +190,21 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (e *gethRPCClient) BlockByHash(hash gethcommon.Hash) (*types.Block, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
block, found := e.blockCache.Get(hash) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if found { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return block, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// not in cache, fetch from RPC | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ctx, cancel := context.WithTimeout(context.Background(), e.timeout) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
defer cancel() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return e.client.BlockByHash(ctx, hash) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
block, err := e.client.BlockByHash(ctx, hash) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
e.blockCache.Add(hash, block) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. The block is added to the cache without checking if the block is nil. Adding a nil block to the cache could lead to unexpected behavior when the cache is accessed later. It would be prudent to add a nil check before caching the block. + if block != nil {
+ e.blockCache.Add(hash, block)
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return block, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+192
to
+206
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. The - block, found := e.blockCache.Get(hash)
+ cachedBlock, found := e.blockCache.Get(hash)
+ if found {
+ block, ok := cachedBlock.(*types.Block)
+ if !ok {
+ e.logger.Error("cache returned a non-block type")
+ return nil, fmt.Errorf("cache integrity error: expected *types.Block, got %T", cachedBlock)
+ }
+ return block, nil
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (e *gethRPCClient) CallContract(msg ethereum.CallMsg) ([]byte, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
This is just a finger in the air placeholder, I tried it with 200 and it was fine but doesn't give you many more cache hits so I went for a reasonable minimum.
We can make this configurable at some point if it ever becomes useful.