Post Merge checkpoint sync #4735

gfukushima · 2022-11-25T04:21:00Z

PR description

Add post-merge checkpoint sync support

Fixed Issue(s)

Fixes #4573
Linked to #4703

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Changelog

I thought about the changelog and included a changelog update if required.

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

…troller Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

# Conflicts: # besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java # ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

…earhead-checkpoint-sync

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

…earhead-checkpoint-sync

besu/src/test/java/org/hyperledger/besu/cli/options/SynchronizerOptionsTest.java

siladu · 2022-12-08T07:16:41Z

besu/src/main/java/org/hyperledger/besu/controller/BesuController.java

+      return configOptions.getTerminalTotalDifficulty().isPresent()
+          && configOptions.getCheckpointOptions().isValid()
+          && (UInt256.fromHexString(configOptions.getCheckpointOptions().getTotalDifficulty().get())
+              .greaterThan(configOptions.getTerminalTotalDifficulty().get()));


What happens if we're using a devnet or a new public testnet that starts with TTD = 0? I think every block's TD will be 0 and this condition will never be met?

Added a test and another check for that case

As discussed current besu codebase requires a genesis block difficulty 1 so every block will have TotalDifficulty 1 in this case.

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java

siladu · 2022-12-11T23:24:21Z

besu/src/main/java/org/hyperledger/besu/controller/BesuController.java

+      return configOptions.getTerminalTotalDifficulty().isPresent()
+          && configOptions.getCheckpointOptions().isValid()
+          && (UInt256.fromHexString(configOptions.getCheckpointOptions().getTotalDifficulty().get())
+              .greaterOrEqualThan(configOptions.getTerminalTotalDifficulty().get()));


Does changing this back to >= mean @jframe's point about the TransitionController being needed on the boundary case become an issue again?
#4735 (comment)

Maybe we need a special case for TTD = 0.

Added a test and another check for that case

besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

# Conflicts: # besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java # consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/MergeContext.java # consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java # ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java # ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Signed-off-by: Jason Frame <jason.frame@consensys.net>

siladu · 2022-12-13T22:27:16Z

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/PostMergeContextTest.java

+    postMergeContext.setSyncState(null);
+    assertThat(postMergeContext.isSyncing()).isTrue();
+
+    // after setting a syncState things should progress as expected.


nit: I would split these up into separate tests, maybe one with null input, one for returning true, one for returning false.

siladu · 2022-12-13T22:30:25Z

besu/src/main/java/org/hyperledger/besu/controller/BesuController.java

+              .greaterThan(configOptions.getTerminalTotalDifficulty().get()));
+    }
+
+    private boolean isMergeAtGenesis(final Optional<UInt256> ttd) {


nit: can we avoid mentioning the merge? I think proofOfStakeAtGenesis would be better, if that makes sense here.

Appreciate merge is mentioned a lot elsewhere, but not in this class yet I don't think?

siladu · 2022-12-13T22:49:05Z

besu/src/test/java/org/hyperledger/besu/controller/BesuControllerTest.java

+  }
+
+  @Test
+  public void preMergeCheckpointSyncUsesTransitionControllerBuilder() {


nice, tests looking good!

siladu · 2022-12-15T00:05:37Z

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

+                .lessOrEqualThan(value)) {
+              throw new InvalidConfigurationException(
+                  "Near head checkpoint sync requires a block with total difficulty greater than the TTD");
+            }


Do we need a special case here as well for genesisTD = 0 = TTD?

Have you successfully tested a post-merge genesis with this code?

Signed-off-by: Jason Frame <jason.frame@consensys.net> Signed-off-by: Jason Frame <jason.frame@consensys.net>

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

# Conflicts: # besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java

gfukushima · 2022-12-21T04:18:16Z

Merged as #4844

gfukushima added 22 commits November 25, 2022 12:32

Add validations of new experimental flag

e28c563

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add check to call MergeController without the using the TransitionCon…

7ca71f9

…troller Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Use new flag to skip latestValidAncestorDescendsFromTerminal check

d926c17

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add experimental flag

737af27

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add parser

5a6afd4

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add isNearHeadCheckpointSync method to interface

c00c0c4

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add isNearHeadCheckpointSync implementations

c17c354

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add attribute for the new flag

d74f9cb

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Pass value to MergeContext

cd8bbd3

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add unit tests

a8026e1

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add resource for unit test

0038b37

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add check for optional

11d73af

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Spotless apply

ed0c55a

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'near-head-checkpoint-sync' into main-gfukushima

428f2ef

# Conflicts: # besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java # ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java

Use Default

216d3bd

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add new field to be ignored

6d514b9

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Fix params order

6ee5296

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Change validation order

ee19889

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Refactor validation

6eec853

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Spotless

2ff6786

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add extra unit test

4b9028d

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Rename variable

afeda6c

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima changed the title ~~Nearhead checkpoint sync~~ Near head checkpoint sync Nov 28, 2022

gfukushima added 3 commits November 29, 2022 14:04

Remove parser

af7f0d5

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Remove parser

d207c47

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'main' into nearhead-checkpoint-sync

7b2e85b

gfukushima marked this pull request as ready for review November 29, 2022 03:42

gfukushima marked this pull request as draft November 30, 2022 10:13

gfukushima added 2 commits December 1, 2022 00:17

Change validation to use total difficulty

6b454ba

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge remote-tracking branch 'origin/nearhead-checkpoint-sync' into n…

3a00e41

…earhead-checkpoint-sync

gfukushima added 2 commits December 8, 2022 13:48

spotless

54f603a

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge remote-tracking branch 'origin/nearhead-checkpoint-sync' into n…

288ed54

…earhead-checkpoint-sync

siladu reviewed Dec 8, 2022

View reviewed changes

gfukushima added 4 commits December 9, 2022 15:52

Fix logic for isSyncing method covering possible NPE

2979bb0

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add cover for TTD = TD of the checkpoint block (merge at genesis)

34b3f90

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Remove field from test

4b26c76

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Fix unit test

f824ff4

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe reviewed Dec 9, 2022

View reviewed changes

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionContext.java Show resolved Hide resolved

siladu reviewed Dec 11, 2022

View reviewed changes

gfukushima added 5 commits December 13, 2022 14:33

Add build method and set syncState

d2720e8

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Fix possible NUll Reference Exception and unit test to validate that

6cad478

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add edge case for merge at genesis and unit tests

42e5f4c

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge main into branch

cbd9d1a

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima mentioned this pull request Dec 13, 2022

Possible npe fix #4811

Closed

2 tasks

Gabriel-Trintinalia and others added 3 commits December 13, 2022 18:37

Add accessList field to Transaction Call Object (hyperledger#4802)

3ae9e25

Docker release task for latest tags (hyperledger#4810)

406a069

Signed-off-by: Jason Frame <jason.frame@consensys.net>

Fix docker release workflow login (hyperledger#4826)

8521822

Signed-off-by: Jason Frame <jason.frame@consensys.net>

siladu reviewed Dec 15, 2022

View reviewed changes

jframe and others added 8 commits December 16, 2022 09:17

Add download sha sums for 22.10.3 release (hyperledger#4827)

bd467a1

Signed-off-by: Jason Frame <jason.frame@consensys.net> Signed-off-by: Jason Frame <jason.frame@consensys.net>

Pass syncMode to BesuController.Builder to allow checkpointsync PoS

928f008

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Fix difficulty of invalid genesis

6810c3c

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add static method

18d29ca

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Pass syncMode to BesuController builder

34f0b9d

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

spotless

8c7411b

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge remote-tracking branch 'hyperledge/main'

3ccfec0

Merge branch 'main' into nearhead-checkpoint-sync

7b56639

# Conflicts: # besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java

gfukushima closed this Dec 21, 2022

gfukushima deleted the nearhead-checkpoint-sync branch January 13, 2023 07:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Post Merge checkpoint sync #4735

Post Merge checkpoint sync #4735

gfukushima commented Nov 25, 2022 •

edited

Loading

siladu Dec 8, 2022 •

edited

Loading

gfukushima Dec 13, 2022

gfukushima Dec 16, 2022

siladu Dec 11, 2022

gfukushima Dec 13, 2022

siladu Dec 13, 2022

siladu Dec 13, 2022

siladu Dec 13, 2022

siladu Dec 15, 2022

siladu Dec 15, 2022

gfukushima commented Dec 21, 2022

Post Merge checkpoint sync #4735

Post Merge checkpoint sync #4735

Conversation

gfukushima commented Nov 25, 2022 • edited Loading

PR description

Fixed Issue(s)

Documentation

Changelog

siladu Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfukushima commented Dec 21, 2022

gfukushima commented Nov 25, 2022 •

edited

Loading

siladu Dec 8, 2022 •

edited

Loading