Update consensus to v0.5.1 #113

mkalinin · 2019-04-01T13:07:53Z

What was done?

This PR mainly updates consensus to v0.5.1 version.
Alongside this there are few other changes and fixes:

Get rid of field name in signed_root.
Use signed_root as a block id all over the code.
Fix simulator config with empty chainSpec section.
Break SpecHelpers down to several sub-interfaces.
get_shuffling2 fixed in order to pass shuffling tests.

Spec issues fixed in this PR

Spec has a few issues in crosslink processing part which are addressed by Rework crosslink processing ethereum/consensus-specs#855, add previous and current crosslinks ethereum/consensus-specs#874.
Overflow in process_exit_queue, Fix epoch overflow in process_exit_queue ethereum/consensus-specs#875.

…lass

…cipants

This reverts commit 3cdd341.

…nd_participants" This reverts commit bdfe260.

zilm13

Minor comments

zilm13 · 2019-04-01T15:45:10Z

chain/src/main/java/org/ethereum/beacon/chain/observer/ObservableStateProcessorImpl.java

   * @return new state, result of applied transition to the latest input state
   */
-  private BeaconStateEx applySlotTransitionsWithoutEpoch(
+  private BeaconStateEx applySlotTransitions(


Could we move this helper somewhere in public? I see 2 places where you use it, plus I have another, 3rd, in tests.

There was an idea to add a transition to handle this particular case. Will try to hash it out.

zilm13 · 2019-04-01T16:02:27Z

core/src/main/java/org/ethereum/beacon/core/types/Gwei.java

@@ -33,6 +33,23 @@ public Gwei minus(Gwei subtrahend) {
    return new Gwei(super.minus(subtrahend));
  }

+  public Gwei minusSat(Gwei subtrahend) {


what does it mean?

It's a reference to saturation arithmetic. Maybe you can come with a better naming to this method?

Just add javadoc comment to plusSat and minSat like in UInt64

zilm13 · 2019-04-01T16:05:39Z

start/simulator/src/main/resources/config/default-simulation-config.yml

-    blsVerify: false
-    blsVerifyProofOfPosession: false
-    blsSign: false
+    blsVerify: true


oh, committed by a mistake. nice catch! 👍

zilm13 · 2019-04-01T19:51:45Z

core/src/main/java/org/ethereum/beacon/core/types/Gwei.java

+
+  public Gwei plusSat(Gwei addend) {
+    Gwei res = this.plus(addend);
+    if (res.lessEqual(res) && addend.greater(Gwei.ZERO)) {


condition is always true and looks erroneous

another good catch!

zilm13 · 2019-04-01T20:37:17Z

consensus/src/main/java/org/ethereum/beacon/consensus/spec/EpochProcessing.java

+    } else {
+      // Dequeue if the minimum amount of time has passed
+      return get_current_epoch(state).greaterEqual(
+          validator.getExitEpoch().plus(getConstants().getMinValidatorWithdrawabilityDelay()));


we are getting overflow here.FAR_FUTURE which is usually exit epoch is highest possible number and we are adding MinValidatorWithdrawabilityDelay to it

Gonna fix it within the others overflows/underflows 👍

mkalinin · 2019-04-02T10:05:50Z

@zilm13 ready for the next round

zilm13

LGTM

Nashatyrev · 2019-04-02T14:33:46Z

consensus/src/main/java/org/ethereum/beacon/consensus/SpecHelpers.java

-          get_current_epoch_committee_count(state), constants.getShardCount()));
+      state.setCurrentShufflingStartShard(ShardNumber.of(
+          state.getCurrentShufflingStartShard().plus(get_current_epoch_committee_count(state))
+              .modulo(constants.getShardCount())));


Can't get what's the difference between fixed and original version?

BTW still wondering...

hm, I brought it back: https://github.com/harmony-dev/beacon-chain-java/pull/113/files#diff-393fe29c319fa1ad2601d34ab8c1e311R870

Nashatyrev · 2019-04-02T14:53:39Z

consensus/src/main/java/org/ethereum/beacon/consensus/SpecHelpers.java

@@ -2105,6 +2105,9 @@ private Gwei get_inactivity_penalty(BeaconState state, ValidatorIndex index, Epo
        new Gwei[state.getValidatorRegistry().size().getIntValue()],
        new Gwei[state.getValidatorRegistry().size().getIntValue()]
    };
+    for (ValidatorIndex index : state.getValidatorRegistry().size()) {
+      deltas[0][index.getIntValue()] = deltas[1][index.getIntValue()] = Gwei.ZERO;
+    }


M.b. Arrays.fill() would more laconic?

Definitely 👍

Nashatyrev · 2019-04-02T15:48:19Z

consensus/src/main/java/org/ethereum/beacon/consensus/util/CachingBeaconChainSpec.java

-    ObjectHasher<Hash32> sszHasher = SSZObjectHasher.create(hashFunction);
-    return new SpecHelpers(constants, hashFunction, sszHasher);
-  }
+public class CachingBeaconChainSpec extends DelegatingBeaconChainSpec {


Delegating wouldn't work for polymorphic invocations. E.g. CachingSpecHelpers.get_permuted_list would never be called

That's true. Once call reaches delegate it never goes back to a class originated this call. Let me think around a little bit.

mkalinin added 30 commits March 20, 2019 15:04

Add Bytes4 type

423cf02

Update core structures except state to v0.5.0

0df0510

Update beacon chain constants to v0.5.0

c6fa559

Update state structure and SpecHelpers to v0.5.1

84a4523

Fix #821 in SpecHelpers.update_registry_and_shuffling_data

fb15d73

Update validator routines to v0.5.0

779207c

Adjust state transitions with v0.5.1

012e1a7

Fix SSZ issues with BeaconState implementations

013563b

Adjust stats output to a new epoch transition

91ceb0a

Fix empty slot transitions

a358919

Fix index issues and NPEs in SpecHelpers

1bc7e9f

Fix parent hashes in DefaultBeaconChainTest

846a021

Fix empty slot transitions

358ada9

Add state caching to PerSlotTransitionTest

d49d170

Add state caching to PerEpochTransitionTest

e560b84

Fix validator tests

7b335de

Collect epoch summary within ExtendedSlotTransition class

2bdbdf1

Do not prepend already hashed container with its length

8d7ede5

Remove field name from signed_root calls

6d9d6eb

Use signed_root as a block hash function all over the codebase

87e9d12

Cache signed_root results alongside with hash_tree_root

38f5cf6

Merge branch 'develop' into spec/v0.5.0

0e0e703

Segregate SpecHelpers interface; use delegate in CachingSpecHelpers c…

d1f991c

…lass

Fix BeaconChainAttesterImpl.getSourceRoot

597ba10

Fix attestation inclusion interval in proposer routine

727a5f3

Set stateRoot to zeros in get_temporary_block_header

23d57fa

Fix BeaconChainProposerTest.proposeABlockWithOperations

2dfc80f

Show epoch summary output right after last slot of the epoch

32849b1

Do not count epoch before genesis in process_crosslinks

3cdd341

Filter out attestations by shard number in get_winning_root_and_parti…

bdfe260

…cipants

mkalinin added 8 commits March 27, 2019 20:05

Derive CachingSpecHelpers from a full SpecHelpers delegate

89d3e33

Fix blsVerifyProofOfPo(s)session in default-simulation-config

c0831ef

Fix previous_epoch helpers: current_epoch_attestations were used

add10ba

Revert "Do not count epoch before genesis in process_crosslinks"

01473d5

This reverts commit 3cdd341.

Revert "Filter out attestations by shard number in get_winning_root_a…

a127f52

…nd_participants" This reverts commit bdfe260.

Rework crosslink processing part of the spec

a1ee101

Break down spec into several interfaces

ac5f2ce

Bring epoch summary back

92a5a56

mkalinin requested review from zilm13 and Nashatyrev April 1, 2019 13:07

zilm13 approved these changes Apr 1, 2019

View reviewed changes

zilm13 reviewed Apr 1, 2019

View reviewed changes

mkalinin added 3 commits April 2, 2019 12:46

Fix saturation arithmetic for Gwei and UInt64

618d6d1

Revert accidental changes made to default-simulation-config.yml

4da678f

Fix epoch overflow in process_exit_queue

0dc8646

Implement EmptySlotTransition to share this logic accross the codebase

d9c7d6b

zilm13 approved these changes Apr 2, 2019

View reviewed changes

Fix int_to_bytes32: could return less than 32 bytes

8f56e39

Nashatyrev requested changes Apr 2, 2019

View reviewed changes

mkalinin added 4 commits April 3, 2019 12:20

Revert round order in optimized shuffling func

2207a5d

Use Arrays.fill to fill deltas with zeros

8c2b2eb

Create own spec instance for each launcher

c9bc3b7

Simplify crosslinks processing

0434156

Nashatyrev approved these changes Apr 3, 2019

View reviewed changes

mkalinin merged commit 427e00b into develop Apr 3, 2019

This was referenced Apr 9, 2019

Fix workaround with empty epoch transition #100

Closed

Update signed_root usage #111

Closed

mkalinin deleted the spec/v0.5.1 branch June 3, 2019 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update consensus to v0.5.1 #113

Update consensus to v0.5.1 #113

mkalinin commented Apr 1, 2019 •

edited

Loading

zilm13 left a comment

zilm13 Apr 1, 2019

mkalinin Apr 2, 2019

zilm13 Apr 1, 2019

mkalinin Apr 2, 2019

zilm13 Apr 2, 2019

zilm13 Apr 1, 2019

mkalinin Apr 2, 2019

zilm13 Apr 1, 2019

mkalinin Apr 2, 2019

zilm13 Apr 1, 2019

mkalinin Apr 2, 2019 •

edited

Loading

mkalinin commented Apr 2, 2019

zilm13 left a comment

Nashatyrev Apr 2, 2019

Nashatyrev Apr 3, 2019

mkalinin Apr 3, 2019

Nashatyrev Apr 3, 2019

Nashatyrev Apr 2, 2019

mkalinin Apr 3, 2019

Nashatyrev Apr 2, 2019

mkalinin Apr 3, 2019

Update consensus to v0.5.1 #113

Update consensus to v0.5.1 #113

Conversation

mkalinin commented Apr 1, 2019 • edited Loading

What was done?

Spec issues fixed in this PR

zilm13 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

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

mkalinin Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

mkalinin commented Apr 2, 2019

zilm13 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkalinin commented Apr 1, 2019 •

edited

Loading

mkalinin Apr 2, 2019 •

edited

Loading