Synchronize access to block header (hyperledger#6143)

* Synchronize access to block header

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Add 'safe' version of getBlockHeader

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Move retry with lock into getChainHeadBlockHeader()

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Reinstate 'final' modifier

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java

@@ -148,6 +148,15 @@ default boolean contains(final Hash blockHash) {
    */
   Optional<BlockHeader> 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<BlockHeader> getBlockHeaderSafe(Hash blockHeaderHash);
+
   /**
    * Returns the block body corresponding to the given block header hash. Associated block is not
    * necessarily on the canonical chain.

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

@@ -302,6 +302,16 @@ public Optional<BlockHeader> getBlockHeader(final Hash blockHeaderHash) {
         .orElseGet(() -> blockchainStorage.getBlockHeader(blockHeaderHash));
   }
 
+  @Override
+  public synchronized Optional<BlockHeader> getBlockHeaderSafe(final Hash blockHeaderHash) {
+    return blockHeadersCache
+        .map(
+            cache ->
+                Optional.ofNullable(cache.getIfPresent(blockHeaderHash))
+                    .or(() -> blockchainStorage.getBlockHeader(blockHeaderHash)))
+        .orElseGet(() -> blockchainStorage.getBlockHeader(blockHeaderHash));
+  }
+
   @Override
   public Optional<BlockBody> getBlockBody(final Hash blockHeaderHash) {
     return blockBodiesCache

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java

@@ -551,7 +551,14 @@ public Optional<Transaction> getTransactionByHash(final Hash hash) {
 
   private Optional<BlockHeader> 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) {

ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java

@@ -132,6 +132,11 @@ public Optional<BlockHeader> getBlockHeader(final Hash blockHeaderHash) {
     return Optional.ofNullable(hashToHeader.get(blockHeaderHash));
   }
 
+  @Override
+  public synchronized Optional<BlockHeader> getBlockHeaderSafe(final Hash blockHeaderHash) {
+    return Optional.ofNullable(hashToHeader.get(blockHeaderHash));
+  }
+
   @Override
   public Optional<BlockBody> getBlockBody(final Hash blockHeaderHash) {
     // Deterministic, but just not implemented.