From e034abecae5eaaa8b032cdf8a52f0cf273b1972e Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Tue, 14 Nov 2023 00:07:28 +0000 Subject: [PATCH] Synchronize access to block header (#6143) * Synchronize access to block header Signed-off-by: Matthew Whitehead * Add 'safe' version of getBlockHeader Signed-off-by: Matthew Whitehead * Move retry with lock into getChainHeadBlockHeader() Signed-off-by: Matthew Whitehead * Reinstate 'final' modifier Signed-off-by: Matthew Whitehead --------- Signed-off-by: Matthew Whitehead Signed-off-by: Justin Florentine --- .../hyperledger/besu/ethereum/chain/Blockchain.java | 9 +++++++++ .../besu/ethereum/chain/DefaultBlockchain.java | 10 ++++++++++ .../ethereum/eth/transactions/TransactionPool.java | 9 ++++++++- .../referencetests/ReferenceTestBlockchain.java | 5 +++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java index 17a9a57743c..84e92ff019b 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java @@ -148,6 +148,15 @@ default boolean contains(final Hash blockHash) { */ Optional getBlockHeader(Hash blockHeaderHash); + /** + * Safe version of {@code getBlockHeader} (it should take any locks necessary to ensure any block + * updates that might be taking place have been completed first) + * + * @param blockHeaderHash The hash of the block whose header we want to retrieve. + * @return The block header corresponding to this block hash. + */ + Optional getBlockHeaderSafe(Hash blockHeaderHash); + /** * Returns the block body corresponding to the given block header hash. Associated block is not * necessarily on the canonical chain. diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java index cbde737684b..a92be73bf7c 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java @@ -302,6 +302,16 @@ public Optional getBlockHeader(final Hash blockHeaderHash) { .orElseGet(() -> blockchainStorage.getBlockHeader(blockHeaderHash)); } + @Override + public synchronized Optional getBlockHeaderSafe(final Hash blockHeaderHash) { + return blockHeadersCache + .map( + cache -> + Optional.ofNullable(cache.getIfPresent(blockHeaderHash)) + .or(() -> blockchainStorage.getBlockHeader(blockHeaderHash))) + .orElseGet(() -> blockchainStorage.getBlockHeader(blockHeaderHash)); + } + @Override public Optional getBlockBody(final Hash blockHeaderHash) { return blockBodiesCache diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java index 28f1110499b..494db70ec3e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java @@ -551,7 +551,14 @@ public Optional getTransactionByHash(final Hash hash) { private Optional getChainHeadBlockHeader() { final MutableBlockchain blockchain = protocolContext.getBlockchain(); - return blockchain.getBlockHeader(blockchain.getChainHeadHash()); + + // Optimistically get the block header for the chain head without taking a lock, + // but revert to the safe implementation if it returns an empty optional. (It's + // possible the chain head has been updated but the block is still being persisted + // to storage/cache under the lock). + return blockchain + .getBlockHeader(blockchain.getChainHeadHash()) + .or(() -> blockchain.getBlockHeaderSafe(blockchain.getChainHeadHash())); } private boolean isLocalSender(final Address sender) { diff --git a/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java b/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java index 716c81d5d14..7e84ccbaf87 100644 --- a/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java +++ b/ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java @@ -132,6 +132,11 @@ public Optional getBlockHeader(final Hash blockHeaderHash) { return Optional.ofNullable(hashToHeader.get(blockHeaderHash)); } + @Override + public synchronized Optional getBlockHeaderSafe(final Hash blockHeaderHash) { + return Optional.ofNullable(hashToHeader.get(blockHeaderHash)); + } + @Override public Optional getBlockBody(final Hash blockHeaderHash) { // Deterministic, but just not implemented.