Implement SszBitvector #3611

Nashatyrev · 2021-02-16T17:39:06Z

PR Description

Add SszBitvector and SszBitvectorSchema
Replace legacy Bitvector usages with SszBitvector
Though Bitvector is still internally used it is now not exposed (package private)
Add unit test set
Remove obsolete SszComplexSchemas.SszBitVectorSchema
Make lazy TreeNode calculation for SszBitvector to avoid overhead when using SszBitvector as just a bitvector structure

Benchmarks

Benchmark	Units	Error	Score Before	Score After	Error
t.p.t.benchmarks.SSZBenchmark.BeaconStateSerialization	ops/s	766.650	66.504	516.24	29.009
t.p.t.benchmarks.Sha256Benchmark.sha256of33byteArray	ops/s	1389075.321	32640.610	1398325.14	30074.439
t.p.t.benchmarks.Sha256Benchmark.sha256of33bytes	ops/s	1381940.655	38107.118	1359151.657	36160.893
t.p.t.benchmarks.SpecBenchmarks.integerSquareRoot	ops/s	1759424.409	78305.049	1801121.609	89565.032
t.p.t.benchmarks.VoteTrackerSerialize.genericDeserialization	ops/ms	1373.555	413.851	1695.345	416.488
t.p.t.benchmarks.VoteTrackerSerialize.genericSerialization	ops/ms	4180.761	437.056	4169.041	471.493
t.p.t.benchmarks.VoteTrackerSerialize.voteTrackerDeserialization	ops/ms	1089.923	473.447	1198.28	472.794
t.p.t.benchmarks.VoteTrackerSerialize.voteTrackerSerialization	ops/ms	3029.200	448.821	3107.089	432.355
t.p.t.benchmarks.util.backing.BeaconStateBenchmark.iterateBalances	ops/s	3051.709	60.499	2861.781	69.681
t.p.t.benchmarks.util.backing.BeaconStateBenchmark.iterateValidators	ops/s	2166.284	125.289	2103.228	143.122
t.p.t.benchmarks.util.backing.BeaconStateBenchmark.iterateValidatorsWithMethods	ops/s	76.144	1.896	70.494	2.783
t.p.t.benchmarks.util.backing.BeaconStateBenchmark.updateBalancesAndHash	ops/s	38.795	0.769	35.41	1.044
t.p.t.benchmarks.util.backing.BitlistBenchmark.fromCachedListView	ops/s	79688.884	3447.965	78636.604	4009.21
t.p.t.benchmarks.util.backing.BitlistBenchmark.fromNewListView	ops/s	23289.222	4359.102	21132.193	4437.679
t.p.t.benchmarks.util.backing.ListBenchmark.append	ops/s	6008475.831	2571404.811	6528390.779	3287514.796
t.p.t.benchmarks.util.backing.ListBenchmark.createDefaultUIntList	ops/s	3074719.821	709128.778	2417765.128	616435.746
t.p.t.benchmarks.util.backing.ListBenchmark.incrementalHash	ops/s	10753.118	1866.507	10199.532	1345.752
t.p.t.benchmarks.BeaconBlockBenchmark.hashFullBlocks	us/op	0.008	0.001	0.008	0.001
t.p.t.benchmarks.BeaconBlockBenchmark.hashSparseBlocks	us/op	0.008	0.001	0.008	0.001
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchCreate	us/op	12.707	0.425	15.308	4.414
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchCreateAndIterate	us/op	12.503	0.434	28.27	26.104
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchCreateAndSerialize	us/op	18.814	0.872	19.34	2.956
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchDeserialize	us/op	3.982	0.183	4.457	0.519
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchDeserializeAndIterate	us/op	9.006	0.921	9.003	0.994
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchDeserializeAndSerialize	us/op	9.381	0.684	10.798	0.742
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchIterate	us/op	0.088	0.004	0.091	0.008
t.p.t.benchmarks.ssz.SszAttestationBenchmark.benchSerialize	us/op	4.111	0.398	4.841	0.549
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchCreate	us/op	21.288	0.890	30.658	15.989
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchCreateAndIterate	us/op	24.468	0.865	23.744	6.313
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchCreateAndSerialize	us/op	75.895	2.549	150.194	78.934
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchDeserialize	us/op	46.920	3.074	58.92	3.423
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchDeserializeAndIterate	us/op	87.453	5.286	97.687	5.282
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchDeserializeAndSerialize	us/op	102.709	4.244	157.507	42.825
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchIterate	us/op	1.283	0.071	1.17	0.082
t.p.t.benchmarks.ssz.SszBeaconBlockBodyBenchmark.benchSerialize	us/op	50.365	2.299	70.457	2.291
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchCreate	us/op	0.303	0.021	0.286	0.006
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchCreateAndIterate	us/op	0.368	0.018	0.312	0.025
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchCreateAndSerialize	us/op	0.462	0.022	0.47	0.058
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchDeserialize	us/op	0.258	0.013	0.254	0.006
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchDeserializeAndIterate	us/op	0.362	0.015	0.368	0.009
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchDeserializeAndSerialize	us/op	0.396	0.025	0.381	0.008
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchIterate	us/op	0.013	0.001	0.012	0.001
t.p.t.benchmarks.ssz.SszCheckpointBenchmark.benchSerialize	us/op	0.131	0.014	0.126	0.008
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchCreate	us/op	12.147	1.352	11.33	0.205
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchCreateAndIterate	us/op	11.952	0.533	11.742	0.573
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchCreateAndSerialize	us/op	17.067	1.337	17.561	0.84
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchDeserialize	us/op	3.749	0.346	3.658	0.066
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchDeserializeAndIterate	us/op	7.119	0.434	7.003	0.108
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchDeserializeAndSerialize	us/op	8.391	0.284	9.19	0.199
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchIterate	us/op	0.094	0.004	0.097	0.003
t.p.t.benchmarks.ssz.SszPendingAttestationBenchmark.benchSerialize	us/op	3.661	0.240	4.206	0.177
t.p.t.benchmarks.TransitionBenchmark.Block.importBlock	s/op	0.050	0.003	0.05	0.003
t.p.t.benchmarks.TransitionBenchmark.Epoch.importBlock	s/op	0.178	0.011	0.166	0.01

Fixed Issue(s)

Part of #3540

Documentation

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

Changelog

I thought about adding a changelog entry, and added one if I deemed necessary.

…t type (different maxLength) fails now

…hema. SszListSchema.create() would create the SszBitlistSchema if element type is SszBit

…used like a plain structure

…y on Ssz structures

…ectly on Ssz structures

…ist-etc

…ature/ssz-bitlist-etc

…ist-etc # Conflicts: # data/provider/src/test/java/tech/pegasys/teku/api/ValidatorDataProviderTest.java # data/serializer/src/test/java/tech/pegasys/teku/provider/JsonProviderTest.java # eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/ssz/SszAttestationBenchmark.java # eth-benchmark-tests/src/jmh/java/tech/pegasys/teku/benchmarks/ssz/SszPendingAttestationBenchmark.java # ethereum/datastructures/src/test/java/tech/pegasys/teku/datastructures/operations/AttestationTest.java # ethereum/datastructures/src/test/java/tech/pegasys/teku/datastructures/state/PendingAttestationTest.java # ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java # ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregateAttestationBuilderTest.java # ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AggregatingAttestationPoolTest.java # ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/AttestationManagerTest.java # ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/attestation/MatchingDataAttestationGroupTest.java # ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/forkchoice/ForkChoiceTest.java # services/beaconchain/src/test/java/tech/pegasys/teku/services/beaconchain/BeaconChainMetricsTest.java # validator/client/src/test/java/tech/pegasys/teku/validator/client/duties/AttestationProductionDutyTest.java # validator/coordinator/src/test/java/tech/pegasys/teku/validator/coordinator/ValidatorApiHandlerTest.java

…structures

…ist-etc

# Conflicts: # data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v1/node/GetIdentityIntegrationTest.java # data/serializer/src/test/java/tech/pegasys/teku/provider/JsonProviderTest.java # ethereum/datastructures/src/test/java/tech/pegasys/teku/datastructures/networking/libp2p/rpc/MetadataMessageTest.java # ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/BeaconStateBuilder.java # ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java # networking/p2p/src/test/java/tech/pegasys/teku/networking/p2p/discovery/DiscoveryNetworkTest.java # networking/p2p/src/test/java/tech/pegasys/teku/networking/p2p/discovery/discv5/NodeRecordConverterTest.java

…ector # Conflicts: # data/serializer/src/main/java/tech/pegasys/teku/provider/JsonProvider.java # data/serializer/src/test/java/tech/pegasys/teku/provider/JsonProviderTest.java # ethereum/datastructures/src/test/java/tech/pegasys/teku/datastructures/networking/libp2p/rpc/MetadataMessageTest.java # ethereum/spec/src/testFixtures/java/tech/pegasys/teku/spec/util/DataStructureUtil.java # ssz/src/main/java/tech/pegasys/teku/ssz/backing/schema/AbstractSszListSchema.java # ssz/src/main/java/tech/pegasys/teku/ssz/backing/schema/SszComplexSchemas.java # ssz/src/main/java/tech/pegasys/teku/ssz/backing/view/SszUtils.java # ssz/src/test/java/tech/pegasys/teku/ssz/backing/collections/BitvectorImplTest.java

ajsutton

LGTM.

…ector # Conflicts: # ethereum/spec/src/main/java/tech/pegasys/teku/spec/statetransition/epoch/EpochProcessor.java

Nashatyrev and others added 30 commits February 11, 2021 15:09

Initial SszBitlist stub integration to Attestation strcuture

daaba27

Draft but functional SszBitlist implementation

47e6935

Spotless

2f7399e

Adopt PendingAttestation to SszBitlist

3e10e66

Add JSON SszBitlist serializer

c69d406

Fix the tests

1f45757

Move unused method from prod to test

7af582a

Remove unused method

191c46d

Remove legacy Bitlist from a couple of classes

4d999fa

Continue migrating Bitlist usages to SszBitlist

29358fd

Migrate all Bitlist usages to SszBitlist

785542c

Fix two tests: attempt to create Attestation with incompatible Bitlis…

8de8a8f

…t type (different maxLength) fails now

Remove to/fromLegacy() from SszBitlist interfaces

2628be7

Move Bitlist specific code from AbstractSszListSchema to SszBitlistSc…

8f01f12

…hema. SszListSchema.create() would create the SszBitlistSchema if element type is SszBit

Remove SszComplexSchemas.SszBitListSchema

773942f

Minor refactor

0c5f49b

Make lazy TreeNode calculation to remove overhead when SszBitlist is …

09b7aec

…used like a plain structure

Make Bitlist package private for internal SszBitlist usage only

47aa2df

Avoid calling abstract method from constructor

4e22ab5

Fix the tests: junit usingRecursiveComparison() could work incorrectl…

8891ed6

…y on Ssz structures

Fix another tests: junit usingRecursiveComparison() could work incorr…

7a80390

…ectly on Ssz structures

Check size <= maxLength when creating SszBitlist from schema

389c939

Add SszBitlist and SszBitlistSchema minimal tests

9341728

Merge branch 'master' into feature/ssz-bitlist-etc

d1206b7

Merge remote-tracking branch 'ConsenSys/master' into feature/ssz-bitl…

a6169bd

…ist-etc

Fix AbstractSszComposite.hashCode(): derive it from hashTreeRoot()

d4d1c16

Fix: isSupersetOf() should correctly work with larger bitlist

c7080e5

Add SszBitlist javadoc

13394d7

Add more SszBitlist unit tests

5dd62fd

Merge remote-tracking branch 'origin/feature/ssz-bitlist-etc' into fe…

0a868e2

…ature/ssz-bitlist-etc

Nashatyrev added 25 commits February 17, 2021 12:36

Fix SszBitvectorImpl construction

5584762

Spotless

18e59f7

Remove unused method

a827fa2

Fix var namings after auto renaming

07444bc

Extract SszContainerSchema interface

83c17b2

Add SszDataAssert test fixture

2f30ead

Spotless

b804644

Fix test: should not compare hashCode() of mutable and immutable SSZ …

b7813c4

…structures

Temporarily disable ssz check in test due to issue Consensys#3618

2259a66

Add the test reproducing just found issue

e8e3d99

Fix merge artifacts

e62d869

Rename ssz tests

722e819

Disable comparing by ssz until Consensys#3618 is fixed

30700f2

Merge remote-tracking branch 'ConsenSys/master' into feature/ssz-bitl…

23a83f6

…ist-etc

Fix merge artifacts

2facce6

Fix var name

6e37d36

Add SszBitvectorSchemaTest, add a couple of argument checks

9d7fd72

Fix class name

0b7fb64

Fix toString() for SszBitVector[Schema]

b2b8387

Add more SszBitvector tests

2795b63

Add a bit more docs

f1eac47

Fix: bitlist with zero length not allowed

a25c188

Nashatyrev marked this pull request as ready for review February 17, 2021 19:38

ajsutton approved these changes Feb 17, 2021

View reviewed changes

Merge remote-tracking branch 'ConsenSys/master' into feature/ssz-bitv…

9371e25

…ector # Conflicts: # ethereum/spec/src/main/java/tech/pegasys/teku/spec/statetransition/epoch/EpochProcessor.java

Nashatyrev merged commit 5dc0f4c into Consensys:master Feb 18, 2021

Nashatyrev deleted the feature/ssz-bitvector branch February 18, 2021 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SszBitvector #3611

Implement SszBitvector #3611

Nashatyrev commented Feb 16, 2021 •

edited

ajsutton left a comment

Implement SszBitvector #3611

Implement SszBitvector #3611

Conversation

Nashatyrev commented Feb 16, 2021 • edited

PR Description

Benchmarks

Fixed Issue(s)

Documentation

Changelog

ajsutton left a comment

Choose a reason for hiding this comment

Nashatyrev commented Feb 16, 2021 •

edited