Unified protocol schedule - combine TimestampSchedule and MutableProtocolSchedule #5310

siladu · 2023-04-04T22:34:44Z

This is a necessarily large PR, but it's mostly type removal/renaming/unification changes.
I have added comments to highlight the interesting parts that require closer review.

~~Currently, one reference test file breaking due to invalid test inputs see #5310 (comment)~~

PR description

Combine MutableProtocolSchedule and DefaultTimestampSchedule into UnifiedProtocolSchedule.
Implement getByBlockHeader taking into account both timestamp-based and blockNumber-based forks
Unstitch timestampSchedule from TransitionProtocolSchedule
BftProtocolSchedule extends UnifiedProtocolSchedule (instead of MutableProtocolSchedule)
[DONE] ~~Optimise getByBlockHeader algorithm (once all tests are passing)~~

Follow up PR will cover the more minor tidy up changes left out of this PR for brevity: #5354

Should end up with this hierarchy:

                            ----> PrivacySupportingProtocolSchedule 
                           |
                           | 
                   ProtocolSchedule
                    - public getByBlockHeader
                           ^ 
                           |
         ----->    UnifiedProtocolSchedule  <------ ---------------------------------
        |           - public getByBlockHeader      |                                 |
        |           - private getByTimestamp       |                                 |
        |           - private getByBlockNumber     |                                 |
        |                                          |                                 |
        |                                          |                                 |
BftProtocolSchedule                MilestoneStreamingProtocolSchedule    TransitionProtocolSchedule
 - public getByBlockNumber          - public streamMilestones             - public getByBlockNumber

Fixed Issue(s)

#5260

Testing

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…erenceTests handling TODOs/comments tidy up now before going back to fixing tests as want to put on CI Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

github-actions · 2023-04-04T22:34:55Z

I thought about documentation and added the doc-change-required label to this PR if updates are required.
I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
I thought about the changelog and included a changelog update if required.

siladu · 2023-04-04T22:47:34Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolSchedule.java

+
+import com.google.common.annotations.VisibleForTesting;
+
+public class UnifiedProtocolSchedule implements ProtocolSchedule {


Plan to rename this DefaultProtocolSchedule but think Unified helps with clarity for the first pass review

siladu · 2023-04-05T01:21:41Z

...sus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java

  @Override
  public ProtocolSpec getByBlockHeader(final ProcessableBlockHeader blockHeader) {
-    return this.timestampSchedule
-        .getByTimestamp(blockHeader.getTimestamp())
-        .orElseGet(
-            () ->
-                transitionUtils.dispatchFunctionAccordingToMergeState(
-                    protocolSchedule -> protocolSchedule.getByBlockHeader(blockHeader)));


getByTimestamp and getByBlockNumber are stitched together inside UnifiedProtocolSchedule now, so complexity is encapsulated in that one place.

siladu · 2023-04-05T01:22:36Z

...sus/merge/src/main/java/org/hyperledger/besu/consensus/merge/TransitionProtocolSchedule.java

-  public ProtocolSpec getByBlockNumber(final long number) {
-
-    return Optional.ofNullable(protocolContext)
-        .map(ProtocolContext::getBlockchain)
-        .flatMap(blockchain -> blockchain.getBlockHeader(number))
-        .map(timestampSchedule::getByBlockHeader)
-        .orElseGet(
-            () ->
-                transitionUtils.dispatchFunctionAccordingToMergeState(
-                    protocolSchedule -> protocolSchedule.getByBlockNumber(number)));
-  }


getByTimestamp and getByBlockNumber are stitched together inside UnifiedProtocolSchedule.getByBlockHeader now, so complexity is encapsulated in that one place.

siladu · 2023-04-05T01:24:52Z

...ore/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractProtocolScheduleBuilder.java

AbstractProtocolScheduleBuilder only has one subclass now, ProtocolScheduleBuilder but will merge them in subsequent PR to minimise this already rather large changeset :)

siladu · 2023-04-05T01:26:08Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolScheduleBuilder.java

        validateForkOrder("GrayGlacier", config.getGrayGlacierBlockNumber(), lastForkBlock);
+    // Begin timestamp forks
+    lastForkBlock = validateForkOrder("Shanghai", config.getShanghaiTime(), lastForkBlock);
+    lastForkBlock = validateForkOrder("Cancun", config.getCancunTime(), lastForkBlock);
+    lastForkBlock = validateForkOrder("FutureEips", config.getFutureEipsTime(), lastForkBlock);
+    lastForkBlock =
+        validateForkOrder("ExperimentalEips", config.getExperimentalEipsTime(), lastForkBlock);


append timestamp forks to end of blockNumber forks

siladu · 2023-04-05T01:27:16Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolScheduleBuilder.java

+        // Timestamp Forks
+        createTimestampMilestone(config.getShanghaiTime(), specFactory.shanghaiDefinition(config)),
+        createTimestampMilestone(config.getCancunTime(), specFactory.cancunDefinition(config)),
+        createTimestampMilestone(
+            config.getFutureEipsTime(), specFactory.futureEipsDefinition(config)),
+        createTimestampMilestone(
+            config.getExperimentalEipsTime(), specFactory.experimentalEipsDefinition(config)),
+


append timestamp forks to end of blockNumber forks.

Using createTimestampMilestone because the getBlockByHeader algorithm needs to know the difference between timestamp and blockNumber forks.

siladu · 2023-04-05T01:29:17Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolSchedule.java

+
+  @Override
+  public ProtocolSpec getByBlockHeader(final ProcessableBlockHeader blockHeader) {
+    //    checkArgument(number >= 0, "number must be non-negative");


Think we can safely assume that a blockHeader will never have a negative number or timestamp

I think that's a reasonable assumption and the BlockHeaderBuilder checks the number and timestamp are >= 0

siladu · 2023-04-05T01:35:47Z

...h/src/main/java/org/hyperledger/besu/ethereum/retesteth/NoRewardProtocolScheduleWrapper.java

+  // TODO SLD - do we need this or a getByBlockHeader equivalent? Could be why referenceTests are
+  // failing?
+  //  @Override
+  //  public ProtocolSpec getByBlockNumber(final long number) {
+  //    final ProtocolSpec original = delegate.getByBlockNumber(number);
+  //    final BlockProcessor noRewardBlockProcessor =
+  //        new MainnetBlockProcessor(
+  //            original.getTransactionProcessor(),
+  //            original.getTransactionReceiptFactory(),
+  //            Wei.ZERO,
+  //            original.getMiningBeneficiaryCalculator(),
+  //            original.isSkipZeroBlockRewards(),
+  //            Optional.empty(),
+  //            delegate);
+  //    final BlockValidator noRewardBlockValidator =
+  //        new MainnetBlockValidator(
+  //            original.getBlockHeaderValidator(),
+  //            original.getBlockBodyValidator(),
+  //            noRewardBlockProcessor,
+  //            original.getBadBlocksManager());
+  //    final BlockImporter noRewardBlockImporter = new
+  // MainnetBlockImporter(noRewardBlockValidator);
+  //    return new ProtocolSpec(
+  //        original.getName(),
+  //        original.getEvm(),
+  //        original.getTransactionValidator(),
+  //        original.getTransactionProcessor(),
+  //        original.getPrivateTransactionProcessor(),
+  //        original.getBlockHeaderValidator(),
+  //        original.getOmmerHeaderValidator(),
+  //        original.getBlockBodyValidator(),
+  //        noRewardBlockProcessor,
+  //        noRewardBlockImporter,
+  //        noRewardBlockValidator,
+  //        original.getBlockHeaderFunctions(),
+  //        original.getTransactionReceiptFactory(),
+  //        original.getDifficultyCalculator(),
+  //        Wei.ZERO, // block reward
+  //        original.getMiningBeneficiaryCalculator(),
+  //        original.getPrecompileContractRegistry(),
+  //        original.isSkipZeroBlockRewards(),
+  //        original.getGasCalculator(),
+  //        original.getGasLimitCalculator(),
+  //        original.getFeeMarket(),
+  //        original.getBadBlocksManager(),
+  //        Optional.empty(),
+  //        original.getWithdrawalsValidator(),
+  //        original.getWithdrawalsProcessor(),
+  //        original.getDepositsValidator(),
+  //        original.isPoS());
+
+  //  }


Do we need this or a getByBlockHeader equivalent? ~~Could be why referenceTests are failing?~~

@shemnon I've essentially migrated getBlockByNumber to getBlockByHeader.
Do you know if the logic that I've commented out in here is actually used?

It's not covered by tests AFAICT, but I think it might be used if retesteth is run with certain inputs.

This will break retesteth and we won't be able to fill reference tests.

retesteth needs to have some blocks evaluated "without block reward" and this is how that feature is wired in.

I think I can just migrate this code into the getByBlockHeader method above then. I think the break has already occurred during this commit actually, since that implementation of getBlockByNumber is no longer used: 62f4a51

IIRC this was the beyoncé rule one 😉 😆

I've just migrated it here, hope this makes sense! b459bc3

what's the beyoncé rule??

“If you liked it, then you shoulda put a test on it.”
https://abseil.io/resources/swe-book/html/ch11.html#:~:text=The%20Beyonc%C3%A9%20Rule&text=The%20straightforward%20answer%20is%3A%20test,an%20automated%20test%20for%20it.

siladu · 2023-04-05T01:37:26Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSchedule.java

+  String listMilestones();
+
+  void putMilestone(
+      final boolean isTimestampMilestone,


Would rather find a more encapsulated way to deal with this, ideally private to UnifiedProtocolSchedule (only used by getByBlockHeader). It does make UnifiedProtocolSchedule.getByBlockHeader implementation quite neat though.

could move this into another interface like what was done with the mutable methods in the current implementation at least then it would only be exposed for the few places it's needed like the protocol schedule builder.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu · 2023-04-05T07:00:33Z

@shemnon There is a single referenceTest file that is breaking:

besu/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/mainnet/DifficultyCalculatorTests.java

Line 51 in 1665eb7

"/BasicTests/difficultyMainNetwork.json",

Now that I've combined the timestamp milestones into the same collection as the blockNumber milestone, there's a difficulty mismatch because the protocolSchedule retrieves Shanghai instead of the appropriate blockNumber milestone.

This is because the test inputs have timestamps far into the future so it always matches Shanghai.

For example, the first entry:

 "DifficultyTest1" : {
		"parentTimestamp" : "0x036d2f0c29",
		"parentDifficulty" : "0x17f87b030686276f",
		"parentUncles" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
		"currentTimestamp" : "0x036d2f0c29",
		"currentBlockNumber" : "0x0186a0",
		"currentDifficulty" : "0x17fb7a1266e6f833"
	},

The timestamp 0x036d2f0c29 is 14716701737 which is Fri May 09 2436 04:42:17

Any thoughts about how we could fix this?

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

This reverts commit 861ab1d. Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu · 2023-04-05T21:55:15Z

@shemnon I've opened an issue for now ethereum/tests#1207 but is it possible/appropriate to temporarily disable these while we wait so it doesn't block the PR?

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu · 2023-04-05T22:54:25Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolSchedule.java

+  public ProtocolSpec getByBlockHeader(final ProcessableBlockHeader blockHeader) {
+    // TODO SLD to remove once it's been discussed in PR review
+    //    checkArgument(number >= 0, "number must be non-negative");
+    checkArgument(
+        !protocolSpecs.isEmpty(), "At least 1 milestone must be provided to the protocol schedule");
+    checkArgument(
+        protocolSpecs.last().milestone() == 0, "There must be a milestone starting from block 0");
+
+    // protocolSpecs is sorted in descending block order, so the first one we find that's lower than
+    // the requested level will be the most appropriate spec
+    for (final ScheduledProtocolSpec scheduledProtocolSpec : protocolSpecs) {
+      if (scheduledProtocolSpec.isTimestampMilestone()) {
+        if (blockHeader.getTimestamp() >= scheduledProtocolSpec.milestone()) {
+          return scheduledProtocolSpec.spec();
+        }
+      } else {
+        if (blockHeader.getNumber() >= scheduledProtocolSpec.milestone()) {
+          return scheduledProtocolSpec.spec();
+        }


This is the most important change and represents a change to the algorithm for retrieving the protocol schedule: it still uses the original MutableScheduleProtocol.getByBlockNumber algorithm (running through the schedule in reverse order and returning as soon as our input milestone is beyond a spec), however it checks the timestamp if it's a timestampFork or else it checks the blockNumber.

This is the original MutableProtocolSchedule.getByBlockNumber algorithm for comparison:

@Override public ProtocolSpec getByBlockNumber(final long number) { checkArgument(number >= 0, "number must be non-negative"); checkArgument( !protocolSpecs.isEmpty(), "At least 1 milestone must be provided to the protocol schedule"); checkArgument( protocolSpecs.last().milestone() == 0, "There must be a milestone starting from block 0"); // protocolSpecs is sorted in descending block order, so the first one we find that's lower than // the requested level will be the most appropriate spec for (final ScheduledProtocolSpec s : protocolSpecs) { if (number >= s.milestone()) { return s.spec(); } } return null; }

nit: you could make this closer to the original and remove a little bit of dupe by doing something like

for (final ScheduledProtocolSpec scheduledProtocolSpec : protocolSpecs) { final long milestone = scheduledProtocolSpec.isTimestampMilestone() ? blockHeader.getTimestamp() : blockHeader.getNumber(); if (milestone >= scheduledProtocolSpec.milestone()) { return scheduledProtocolSpec.spec(); } }

shemnon · 2023-04-06T15:33:46Z

@shemnon I've opened an issue for now ethereum/tests#1207 but is it possible/appropriate to temporarily disable these while we wait so it doesn't block the PR?

Looks to like the tests for difficulty are not being maintained.

In the interrum we can change the shanghai timstamp in the mainnet config

  Arguments.of(
            "/BasicTests/difficultyMainNetwork.json",
            MainnetProtocolSchedule.fromConfig(
                GenesisConfigFile.mainnet()
                    .getConfigOptions(Map.of("shanghaiTime", "999999999999")),
                EvmConfiguration.DEFAULT)),

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…steth NoReward Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu · 2023-04-13T22:58:51Z

This failed testing because there's a bug where MainnetEVMs.paris is applied to shanghai due to #4788
I intend to play #4788 first or as part of this.

macfarla

a few comments

.../src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java

...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/WithdrawalsValidatorProvider.java

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSchedule.java

...um/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolScheduleTest.java

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…net/UnifiedProtocolScheduleTest.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Instead of applying mergeSpecificModifications to every post-merge fork, rely on what's configured in the MainnetProtocolsSpec. The exception being MergeBlockProcessor which still needs wiring in as modification due to circular dependency. Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

macfarla

LGTM

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSchedule.java

jframe · 2023-04-11T05:49:22Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolSchedule.java

+
+  @Override
+  public ProtocolSpec getByBlockHeader(final ProcessableBlockHeader blockHeader) {
+    //    checkArgument(number >= 0, "number must be non-negative");


I think that's a reasonable assumption and the BlockHeaderBuilder checks the number and timestamp are >= 0

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

jframe · 2023-04-17T06:10:48Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSchedule.java

+  String listMilestones();
+
+  void putMilestone(
+      final boolean isTimestampMilestone,


could move this into another interface like what was done with the mutable methods in the current implementation at least then it would only be exposed for the few places it's needed like the protocol schedule builder.

jframe · 2023-04-17T06:11:24Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ProtocolSchedule.java

+  String listMilestones();
+
+  void putMilestone(
+      final boolean isTimestampMilestone,


really don't like this boolean, maybe enum would make it clearer

Used inheritance instead as discussed 816137a

jframe · 2023-04-17T06:20:37Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/UnifiedProtocolSchedule.java

+  public ProtocolSpec getByBlockHeader(final ProcessableBlockHeader blockHeader) {
+    // TODO SLD to remove once it's been discussed in PR review
+    //    checkArgument(number >= 0, "number must be non-negative");
+    checkArgument(
+        !protocolSpecs.isEmpty(), "At least 1 milestone must be provided to the protocol schedule");
+    checkArgument(
+        protocolSpecs.last().milestone() == 0, "There must be a milestone starting from block 0");
+
+    // protocolSpecs is sorted in descending block order, so the first one we find that's lower than
+    // the requested level will be the most appropriate spec
+    for (final ScheduledProtocolSpec scheduledProtocolSpec : protocolSpecs) {
+      if (scheduledProtocolSpec.isTimestampMilestone()) {
+        if (blockHeader.getTimestamp() >= scheduledProtocolSpec.milestone()) {
+          return scheduledProtocolSpec.spec();
+        }
+      } else {
+        if (blockHeader.getNumber() >= scheduledProtocolSpec.milestone()) {
+          return scheduledProtocolSpec.spec();
+        }


nit: you could make this closer to the original and remove a little bit of dupe by doing something like

for (final ScheduledProtocolSpec scheduledProtocolSpec : protocolSpecs) { final long milestone = scheduledProtocolSpec.isTimestampMilestone() ? blockHeader.getTimestamp() : blockHeader.getNumber(); if (milestone >= scheduledProtocolSpec.milestone()) { return scheduledProtocolSpec.spec(); } }

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Move timestamp or blockNumber complexity into ScheduledProtocolSpec subclasses Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

jframe

LGTM

gfukushima · 2023-04-20T05:07:41Z

...i/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/JsonRpcMethodsFactory.java

@@ -107,8 +107,7 @@ public Map<String, JsonRpcMethod> methods(
                  blockchainQueries, protocolSchedule, transactionPool, privacyParameters),
              new ExecutionEngineJsonRpcMethods(
                  miningCoordinator,
-                  MergeProtocolSchedule.createTimestamp(
-                      genesisConfigOptions, privacyParameters, false),
+                  MergeProtocolSchedule.create(genesisConfigOptions, privacyParameters, false),


Now that the MergeProtocolShedule already contains the timestamp forks this creation might not be needed anymore as discussed. Might be useful to try use the existing one.

Created a task on the other PR #5354

…ocolSchedule (hyperledger#5310) - Combine MutableProtocolSchedule and DefaultTimestampSchedule into UnifiedProtocolSchedule. - Implement getByBlockHeader taking into account both timestamp-based and blockNumber-based forks - Unstitch timestampSchedule from TransitionProtocolSchedule - BftProtocolSchedule extends UnifiedProtocolSchedule (instead of MutableProtocolSchedule) - In MergeProtocolSchedule, unapply mergeSpecificModifications from Shanghai onwards - Migrate getByBlockNumber modifications into getByBlockHeader for retesteth NoReward - Temporarily fix reference tests by artificially increasing shanghaiTime Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

siladu added 3 commits April 3, 2023 12:08

Make it compile

c034727

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

WIP - most tests passing except ForkIdsNetworkConfigTest and some ref…

53a85f1

…erenceTests handling TODOs/comments tidy up now before going back to fixing tests as want to put on CI Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Remove TimestampScheduleBuilder and deal with most TODOs

742d567

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu commented Apr 4, 2023

View reviewed changes

siladu commented Apr 5, 2023

View reviewed changes

siladu added 3 commits April 5, 2023 11:40

Remove more commented out code

e6c734e

Signed-off-by: Simon Dudley <simon.dudley@consensys.net> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Merge remote-tracking branch 'upstream/main' into unified-protocol-sc…

08ccd0a

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Rename vars timestampSchedule -> protocolSchedule

5ed44bf

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu added 3 commits April 5, 2023 17:08

Temporarily enable non-mainnet ATs

861ab1d

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Merge remote-tracking branch 'upstream/main' into unified-protocol-sc…

e8f6030

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Revert "Temporarily enable non-mainnet ATs"

cef4fb7

This reverts commit 861ab1d. Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu added 3 commits April 6, 2023 08:40

Make ProtocolScheduleTests a bit more robust before refactoring

b777deb

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Simplify getBlockByHeader algorithm

45a5edd

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Add original comment back in

dda9a2d

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu commented Apr 5, 2023

View reviewed changes

siladu added 2 commits April 7, 2023 06:03

Temporarily fix reference tests by artificially increasing shanghaiTime

1030395

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Migrate getByBlockNumber modifications into getByBlockHeader for rete…

b459bc3

…steth NoReward Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

non-fungible-nelson mentioned this pull request Apr 12, 2023

WIP - default methods in Protocol Schedule #4687

Closed

macfarla reviewed Apr 14, 2023

View reviewed changes

siladu and others added 7 commits April 14, 2023 12:31

Merge branch 'main' into unified-protocol-schedule

4d593e0

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Fix copyright

e14261c

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Merge remote-tracking branch 'upstream/main' into unified-protocol-sc…

fe0c92d

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/main…

d82b8cc

…net/UnifiedProtocolScheduleTest.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Update ethereum/core/src/test/java/org/hyperledger/besu/ethereum/main…

e869eb5

…net/UnifiedProtocolScheduleTest.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Merge remote-tracking branch 'upstream/main' into unified-protocol-sc…

a895b04

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu marked this pull request as ready for review April 17, 2023 03:08

macfarla approved these changes Apr 17, 2023

View reviewed changes

jframe reviewed Apr 17, 2023

View reviewed changes

Rename blockOrTimestamp to milestone

3d5e72f

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu mentioned this pull request Apr 17, 2023

UnifiedProtocolSchedule Tidy up #5354

Closed

6 tasks

siladu added 4 commits April 18, 2023 15:22

Replace isTimestampMilestone with scheduledSpecFactory

816137a

Move timestamp or blockNumber complexity into ScheduledProtocolSpec subclasses Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Implement isOnMilestoneBoundary in ScheduledProtocolSpec

f36eae5

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Remove TODO

d1d3f95

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

Merge remote-tracking branch 'upstream/main' into unified-protocol-sc…

f1ad967

…hedule Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

siladu requested a review from jframe April 18, 2023 22:29

Merge branch 'main' into unified-protocol-schedule

b089995

jframe approved these changes Apr 19, 2023

View reviewed changes

Merge branch 'main' into unified-protocol-schedule

5f8fd43

gfukushima reviewed Apr 20, 2023

View reviewed changes

Merge branch 'main' into unified-protocol-schedule

783d2a6

siladu merged commit f8a7d87 into hyperledger:main Apr 20, 2023

siladu deleted the unified-protocol-schedule branch April 20, 2023 08:24

This was referenced Apr 21, 2023

Clean up: Remove applyMergeSpecificModifications #4788

Closed

Unified protocol schedule tidy up #5375

Merged


		import com.google.common.annotations.VisibleForTesting;

		public class UnifiedProtocolSchedule implements ProtocolSchedule {

Unified protocol schedule - combine TimestampSchedule and MutableProtocolSchedule #5310

Unified protocol schedule - combine TimestampSchedule and MutableProtocolSchedule #5310

Conversation

siladu commented Apr 4, 2023 • edited Loading

PR description

Fixed Issue(s)

Testing

github-actions bot commented Apr 4, 2023 • edited by siladu 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

siladu Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siladu Apr 6, 2023 • 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

siladu Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siladu commented Apr 5, 2023 • edited Loading

siladu commented Apr 5, 2023 • edited Loading

siladu Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shemnon commented Apr 6, 2023 • edited Loading

siladu commented Apr 13, 2023

macfarla left a comment

Choose a reason for hiding this comment

macfarla left a 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

jframe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siladu commented Apr 4, 2023 •

edited

Loading

github-actions bot commented Apr 4, 2023 •

edited by siladu

Loading

siladu Apr 5, 2023 •

edited

Loading

siladu Apr 6, 2023 •

edited

Loading

siladu Apr 5, 2023 •

edited

Loading

siladu commented Apr 5, 2023 •

edited

Loading

siladu commented Apr 5, 2023 •

edited

Loading

siladu Apr 5, 2023 •

edited

Loading

shemnon commented Apr 6, 2023 •

edited

Loading