Post Merge checkpoint sync by gfukushima · Pull Request #4735 · besu-eth/besu

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

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

+      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.

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

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

+      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

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

+    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

+              .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

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


nice, tests looking good!

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

+                .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

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

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 (besu-eth#4802)

3ae9e25

Docker release task for latest tags (besu-eth#4810)

406a069

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

Fix docker release workflow login (besu-eth#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 (besu-eth#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

Uh oh!

Conversation

gfukushima commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description

Fixed Issue(s)

Documentation

Changelog

Uh oh!

Uh oh!

siladu Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gfukushima commented Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gfukushima commented Nov 25, 2022 •

edited

Loading

siladu Dec 8, 2022 •

edited

Loading