From f0275d23fcef9c51738a87fff3c6e3e24c1eb2e3 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Thu, 21 Mar 2024 12:47:30 +1000 Subject: [PATCH 1/6] Only add new hash for BWS if ready for BWS Signed-off-by: Jason Frame --- .../sync/backwardsync/BackwardSyncContext.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index bd9c493ec40..457065843d5 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -125,11 +125,11 @@ public synchronized void maybeUpdateTargetHeight(final Hash headHash) { } public synchronized CompletableFuture syncBackwardsUntil(final Hash newBlockHash) { - if (!isTrusted(newBlockHash)) { - backwardChain.addNewHash(newBlockHash); - } - if (isReady()) { + if (!isTrusted(newBlockHash)) { + backwardChain.addNewHash(newBlockHash); + } + final Status status = getOrStartSyncSession(); backwardChain .getBlock(newBlockHash) @@ -142,11 +142,11 @@ public synchronized CompletableFuture syncBackwardsUntil(final Hash newBlo } public synchronized CompletableFuture syncBackwardsUntil(final Block newPivot) { - if (!isTrusted(newPivot.getHash())) { - backwardChain.appendTrustedBlock(newPivot); - } - if (isReady()) { + if (!isTrusted(newPivot.getHash())) { + backwardChain.appendTrustedBlock(newPivot); + } + final Status status = getOrStartSyncSession(); status.updateTargetHeight(newPivot.getHeader().getNumber()); return status.currentFuture; From eaece101c72c32d1349698e7c8f842ae1f3ad665 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Thu, 21 Mar 2024 16:57:56 +1000 Subject: [PATCH 2/6] Only make isReady changes to syncBackwardsUntil with blockHash Signed-off-by: Jason Frame --- .../eth/sync/backwardsync/BackwardSyncContext.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index 457065843d5..acff602c649 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -142,11 +142,11 @@ public synchronized CompletableFuture syncBackwardsUntil(final Hash newBlo } public synchronized CompletableFuture syncBackwardsUntil(final Block newPivot) { - if (isReady()) { - if (!isTrusted(newPivot.getHash())) { - backwardChain.appendTrustedBlock(newPivot); - } + if (!isTrusted(newPivot.getHash())) { + backwardChain.appendTrustedBlock(newPivot); + } + if (isReady()) { final Status status = getOrStartSyncSession(); status.updateTargetHeight(newPivot.getHeader().getNumber()); return status.currentFuture; From bf07f4300587e3d352cd640e0e1eb40f07cb5174 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Thu, 21 Mar 2024 16:58:55 +1000 Subject: [PATCH 3/6] update gradle.properties for build from source Signed-off-by: Jason Frame --- gradle.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/gradle.properties b/gradle.properties index 265507cea43..5545aed091a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,3 +14,4 @@ org.gradle.jvmargs=-Xmx4g \ --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED # Could be moved to sonar properties after https://sonarsource.atlassian.net/browse/SONARGRADL-134 systemProp.sonar.gradle.skipCompile=true +version=24.3.1-SNAPSHOT From 1cb33cc0451035c24abcca33e48c5456b29782e7 Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 26 Mar 2024 10:37:58 +1000 Subject: [PATCH 4/6] Revert "update gradle.properties for build from source" This reverts commit bf07f4300587e3d352cd640e0e1eb40f07cb5174. Signed-off-by: Jason Frame --- gradle.properties | 1 - 1 file changed, 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 5545aed091a..265507cea43 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,4 +14,3 @@ org.gradle.jvmargs=-Xmx4g \ --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED # Could be moved to sonar properties after https://sonarsource.atlassian.net/browse/SONARGRADL-134 systemProp.sonar.gradle.skipCompile=true -version=24.3.1-SNAPSHOT From bcd79f9f3434fbf69eb7759b353a4437e47ac8cb Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 26 Mar 2024 11:46:38 +1000 Subject: [PATCH 5/6] add unit test Signed-off-by: Jason Frame --- .../sync/backwardsync/BackwardSyncContextTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java index 622d66587c7..a949abaca1a 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java @@ -228,6 +228,20 @@ public void shouldSyncUntilHash() throws Exception { assertThat(localBlockchain.getChainHeadBlock()).isEqualTo(remoteBlockchain.getChainHeadBlock()); } + @Test + public void shouldNotSyncUntilHashWhenNotInSync() { + doReturn(false).when(context).isReady(); + final Hash hash = getBlockByNumber(REMOTE_HEIGHT).getHash(); + final CompletableFuture future = context.syncBackwardsUntil(hash); + + respondUntilFutureIsDone(future); + + assertThatThrownBy(future::get) + .isInstanceOf(ExecutionException.class) + .hasMessageContaining("Backward sync is not ready"); + assertThat(backwardChain.getFirstHashToAppend()).isEmpty(); + } + @Test public void shouldSyncUntilRemoteBranch() throws Exception { From 562f7cd9dd7453ec41826eec4d1d65819347b7ee Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Tue, 26 Mar 2024 12:39:16 +1000 Subject: [PATCH 6/6] Move "Appending new head block hash" to BackwardSyncContext so that it is only logged when are actually adding it Signed-off-by: Jason Frame --- .../besu/consensus/merge/blockcreation/MergeCoordinator.java | 4 ---- .../ethereum/eth/sync/backwardsync/BackwardSyncContext.java | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 512d2dc21fd..e0b0559795d 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -501,10 +501,6 @@ public Optional getOrSyncHeadByHash(final Hash headHash, final Hash .addArgument(maybeHeadHeader.get()::toLogString) .log(); } else { - LOG.atDebug() - .setMessage("Appending new head block hash {} to backward sync") - .addArgument(headHash::toHexString) - .log(); backwardSyncContext.maybeUpdateTargetHeight(headHash); backwardSyncContext .syncBackwardsUntil(headHash) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index acff602c649..9a3371412e7 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -127,6 +127,10 @@ public synchronized void maybeUpdateTargetHeight(final Hash headHash) { public synchronized CompletableFuture syncBackwardsUntil(final Hash newBlockHash) { if (isReady()) { if (!isTrusted(newBlockHash)) { + LOG.atDebug() + .setMessage("Appending new head block hash {} to backward sync") + .addArgument(newBlockHash::toHexString) + .log(); backwardChain.addNewHash(newBlockHash); }