From 18e4b90ce000ea4579780ac8745cfbfe1fa34b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Mon, 17 Apr 2023 08:26:50 +0200 Subject: [PATCH 1/4] Append ExtraVanity to the beginning of the marshaled extra data --- command/genesis/polybft_params.go | 2 +- consensus/polybft/checkpoint_manager_test.go | 5 ++--- consensus/polybft/consensus_runtime_test.go | 19 +++++++------------ consensus/polybft/extra.go | 13 ++++++------- consensus/polybft/extra_test.go | 2 +- consensus/polybft/fsm.go | 4 ++-- consensus/polybft/fsm_test.go | 7 ++----- consensus/polybft/hash_test.go | 4 ++-- consensus/polybft/polybft_test.go | 5 ++--- consensus/polybft/runtime_helpers_test.go | 4 ++-- consensus/polybft/validators_snapshot_test.go | 2 +- 11 files changed, 28 insertions(+), 39 deletions(-) diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go index cff626fbba..4d0852a7a5 100644 --- a/command/genesis/polybft_params.go +++ b/command/genesis/polybft_params.go @@ -324,7 +324,7 @@ func generateExtraDataPolyBft(validators []*polybft.ValidatorMetadata) ([]byte, extra := polybft.Extra{Validators: delta, Checkpoint: &polybft.CheckpointData{}} - return append(make([]byte, polybft.ExtraVanity), extra.MarshalRLPTo(nil)...), nil + return extra.MarshalRLPTo(nil), nil } func stringSliceToAddressSlice(addrs []string) []types.Address { diff --git a/consensus/polybft/checkpoint_manager_test.go b/consensus/polybft/checkpoint_manager_test.go index 34595a8d71..7494ef328f 100644 --- a/consensus/polybft/checkpoint_manager_test.go +++ b/consensus/polybft/checkpoint_manager_test.go @@ -18,7 +18,6 @@ import ( "github.com/stretchr/testify/require" "github.com/umbracle/ethgo" - "github.com/0xPolygon/polygon-edge/consensus/ibft/signer" "github.com/0xPolygon/polygon-edge/consensus/polybft/bitmap" bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer" "github.com/0xPolygon/polygon-edge/consensus/polybft/wallet" @@ -80,7 +79,7 @@ func TestCheckpointManager_SubmitCheckpoint(t *testing.T) { extra.Checkpoint = checkpoint extra.Committed = &Signature{Bitmap: bitmap, AggregatedSignature: signature} header = &types.Header{ - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } epochNumber++ } else { @@ -152,7 +151,7 @@ func TestCheckpointManager_abiEncodeCheckpointBlock(t *testing.T) { AggregatedSignature: aggSignature, Bitmap: bmp, } - header.ExtraData = append(make([]byte, signer.IstanbulExtraVanity), extra.MarshalRLPTo(nil)...) + header.ExtraData = extra.MarshalRLPTo(nil) header.ComputeHash() backendMock := new(polybftBackendMock) diff --git a/consensus/polybft/consensus_runtime_test.go b/consensus/polybft/consensus_runtime_test.go index 105e33d664..1bbed2a081 100644 --- a/consensus/polybft/consensus_runtime_test.go +++ b/consensus/polybft/consensus_runtime_test.go @@ -323,7 +323,7 @@ func TestConsensusRuntime_FSM_NotEndOfEpoch_NotEndOfSprint(t *testing.T) { } lastBlock := &types.Header{ Number: 1, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } validators := newTestValidators(t, 3) @@ -730,7 +730,7 @@ func TestConsensusRuntime_IsValidProposalHash(t *testing.T) { block := &types.Block{ Header: &types.Header{ Number: 10, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), }, } block.Header.ComputeHash() @@ -759,7 +759,7 @@ func TestConsensusRuntime_IsValidProposalHash_InvalidProposalHash(t *testing.T) block := &types.Block{ Header: &types.Header{ Number: 10, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), }, } @@ -767,7 +767,7 @@ func TestConsensusRuntime_IsValidProposalHash_InvalidProposalHash(t *testing.T) require.NoError(t, err) extra.Checkpoint.BlockRound = 2 // change it so it is not the same as in proposal hash - block.Header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + block.Header.ExtraData = extra.MarshalRLPTo(nil) block.Header.ComputeHash() runtime := &consensusRuntime{ @@ -846,7 +846,7 @@ func TestConsensusRuntime_HasQuorum(t *testing.T) { lastBuildBlock := &types.Header{ Number: 1, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } blockchainMock := new(blockchainMock) @@ -1079,7 +1079,7 @@ func createTestBlocks(t *testing.T, numberOfBlocks, defaultEpochSize uint64, genesisBlock := &types.Header{ Number: 0, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } parentHash := types.BytesToHash(big.NewInt(0).Bytes()) @@ -1150,12 +1150,7 @@ func createTestExtraForAccounts(t *testing.T, epoch uint64, validators AccountSe Checkpoint: &CheckpointData{EpochNumber: epoch}, } - marshaled := extraData.MarshalRLPTo(nil) - result := make([]byte, ExtraVanity+len(marshaled)) - - copy(result[ExtraVanity:], marshaled) - - return result + return extraData.MarshalRLPTo(nil) } func encodeExitEvents(t *testing.T, exitEvents []*ExitEvent) [][]byte { diff --git a/consensus/polybft/extra.go b/consensus/polybft/extra.go index 36612873b0..a385e69876 100644 --- a/consensus/polybft/extra.go +++ b/consensus/polybft/extra.go @@ -37,7 +37,7 @@ type Extra struct { func (i *Extra) MarshalRLPTo(dst []byte) []byte { ar := &fastrlp.Arena{} - return i.MarshalRLPWith(ar).MarshalTo(dst) + return append(make([]byte, ExtraVanity), i.MarshalRLPWith(ar).MarshalTo(dst)...) } // MarshalRLPWith defines the marshal function implementation for Extra @@ -77,7 +77,7 @@ func (i *Extra) MarshalRLPWith(ar *fastrlp.Arena) *fastrlp.Value { // UnmarshalRLP defines the unmarshal function wrapper for Extra func (i *Extra) UnmarshalRLP(input []byte) error { - return fastrlp.UnmarshalRLP(input, i) + return fastrlp.UnmarshalRLP(input[ExtraVanity:], i) } // UnmarshalRLPWith defines the unmarshal implementation for Extra @@ -681,15 +681,14 @@ func GetIbftExtraClean(extraRaw []byte) ([]byte, error) { } // GetIbftExtra returns the istanbul extra data field from the passed in header -func GetIbftExtra(extraB []byte) (*Extra, error) { - if len(extraB) < ExtraVanity { - return nil, fmt.Errorf("wrong extra size: %d", len(extraB)) +func GetIbftExtra(extraRaw []byte) (*Extra, error) { + if len(extraRaw) < ExtraVanity { + return nil, fmt.Errorf("wrong extra size: %d", len(extraRaw)) } - data := extraB[ExtraVanity:] extra := &Extra{} - if err := extra.UnmarshalRLP(data); err != nil { + if err := extra.UnmarshalRLP(extraRaw); err != nil { return nil, err } diff --git a/consensus/polybft/extra_test.go b/consensus/polybft/extra_test.go index 546c93f3b3..cbcc269064 100644 --- a/consensus/polybft/extra_test.go +++ b/consensus/polybft/extra_test.go @@ -675,7 +675,7 @@ func TestExtra_InitGenesisValidatorsDelta(t *testing.T) { Config: &chain.Params{Engine: map[string]interface{}{ "polybft": polyBftConfig, }}, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } genesisExtra, err := GetIbftExtra(genesis.ExtraData) diff --git a/consensus/polybft/fsm.go b/consensus/polybft/fsm.go index 8ad9aa66a2..f0cbd86fea 100644 --- a/consensus/polybft/fsm.go +++ b/consensus/polybft/fsm.go @@ -167,7 +167,7 @@ func (f *fsm) BuildProposal(currentRound uint64) ([]byte, error) { "Next validators hash", nextValidatorsHash) stateBlock, err := f.blockBuilder.Build(func(h *types.Header) { - h.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + h.ExtraData = extra.MarshalRLPTo(nil) h.MixHash = PolyBFTMixDigest }) @@ -532,7 +532,7 @@ func (f *fsm) Insert(proposal []byte, committedSeals []*messages.CommittedSeal) } // Write extra data to header - newBlock.Block.Header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + newBlock.Block.Header.ExtraData = extra.MarshalRLPTo(nil) if err := f.backend.CommitBlock(newBlock); err != nil { return nil, err diff --git a/consensus/polybft/fsm_test.go b/consensus/polybft/fsm_test.go index 5faf7f54c3..15fb7550f5 100644 --- a/consensus/polybft/fsm_test.go +++ b/consensus/polybft/fsm_test.go @@ -1218,7 +1218,7 @@ func TestFSM_Validate_FailToVerifySignatures(t *testing.T) { extra.Checkpoint = &CheckpointData{CurrentValidatorsHash: validatorsHash, NextValidatorsHash: validatorsHash} parent := &types.Header{ Number: parentBlockNumber, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } parent.ComputeHash() @@ -1278,11 +1278,8 @@ func createTestExtra( parentSignaturesCount int, ) []byte { extraData := createTestExtraObject(allAccounts, previousValidatorSet, validatorsCount, committedSignaturesCount, parentSignaturesCount) - marshaled := extraData.MarshalRLPTo(nil) - result := make([]byte, ExtraVanity+len(marshaled)) - copy(result[ExtraVanity:], marshaled) - return result + return extraData.MarshalRLPTo(nil) } func createTestCommitment(t *testing.T, accounts []*wallet.Account) *CommitmentMessageSigned { diff --git a/consensus/polybft/hash_test.go b/consensus/polybft/hash_test.go index d5b7228bc0..6637aa72af 100644 --- a/consensus/polybft/hash_test.go +++ b/consensus/polybft/hash_test.go @@ -24,11 +24,11 @@ func Test_setupHeaderHashFunc(t *testing.T) { Timestamp: 18, } - header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + header.ExtraData = extra.MarshalRLPTo(nil) notFullExtraHash := types.HeaderHash(header) extra.Committed = createSignature(t, []*wallet.Account{generateTestAccount(t)}, types.ZeroHash, bls.DomainCheckpointManager) - header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + header.ExtraData = extra.MarshalRLPTo(nil) fullExtraHash := types.HeaderHash(header) assert.Equal(t, notFullExtraHash, fullExtraHash) diff --git a/consensus/polybft/polybft_test.go b/consensus/polybft/polybft_test.go index 38d93ca1b1..a60522324e 100644 --- a/consensus/polybft/polybft_test.go +++ b/consensus/polybft/polybft_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/0xPolygon/polygon-edge/consensus" - "github.com/0xPolygon/polygon-edge/consensus/ibft/signer" bls "github.com/0xPolygon/polygon-edge/consensus/polybft/signer" "github.com/0xPolygon/polygon-edge/consensus/polybft/wallet" "github.com/0xPolygon/polygon-edge/helper/progress" @@ -47,7 +46,7 @@ func TestPolybft_VerifyHeader(t *testing.T) { extra.Checkpoint = &CheckpointData{} } - header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) + header.ExtraData = extra.MarshalRLPTo(nil) header.ComputeHash() if len(committedAccounts) > 0 { @@ -55,7 +54,7 @@ func TestPolybft_VerifyHeader(t *testing.T) { require.NoError(t, err) extra.Committed = createSignature(t, committedAccounts, checkpointHash, bls.DomainCheckpointManager) - header.ExtraData = append(make([]byte, signer.IstanbulExtraVanity), extra.MarshalRLPTo(nil)...) + header.ExtraData = extra.MarshalRLPTo(nil) } return extra.Committed diff --git a/consensus/polybft/runtime_helpers_test.go b/consensus/polybft/runtime_helpers_test.go index fdf1fa1e43..5a837c9984 100644 --- a/consensus/polybft/runtime_helpers_test.go +++ b/consensus/polybft/runtime_helpers_test.go @@ -52,7 +52,7 @@ func TestHelpers_isEpochEndingBlock_EpochsNotTheSame(t *testing.T) { nextBlockExtra := &Extra{Checkpoint: &CheckpointData{EpochNumber: 3}, Validators: &ValidatorSetDelta{}} nextBlock := &types.Header{ Number: 21, - ExtraData: append(make([]byte, ExtraVanity), nextBlockExtra.MarshalRLPTo(nil)...), + ExtraData: nextBlockExtra.MarshalRLPTo(nil), } blockchainMock.On("GetHeaderByNumber", mock.Anything).Return(nextBlock, true) @@ -73,7 +73,7 @@ func TestHelpers_isEpochEndingBlock_EpochsAreTheSame(t *testing.T) { nextBlockExtra := &Extra{Checkpoint: &CheckpointData{EpochNumber: 2}, Validators: &ValidatorSetDelta{}} nextBlock := &types.Header{ Number: 16, - ExtraData: append(make([]byte, ExtraVanity), nextBlockExtra.MarshalRLPTo(nil)...), + ExtraData: nextBlockExtra.MarshalRLPTo(nil), } blockchainMock.On("GetHeaderByNumber", mock.Anything).Return(nextBlock, true) diff --git a/consensus/polybft/validators_snapshot_test.go b/consensus/polybft/validators_snapshot_test.go index f22b633d9d..ec17f75182 100644 --- a/consensus/polybft/validators_snapshot_test.go +++ b/consensus/polybft/validators_snapshot_test.go @@ -282,7 +282,7 @@ func createValidatorDeltaHeader(t *testing.T, blockNumber, epoch uint64, oldVali return &types.Header{ Number: blockNumber, - ExtraData: append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...), + ExtraData: extra.MarshalRLPTo(nil), } } From 9ff27d2918d45d3eb193f08a67a5c1e7bf5d9a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Tue, 18 Apr 2023 09:14:07 +0200 Subject: [PATCH 2/4] Build unit tests fix --- consensus/polybft/extra_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/consensus/polybft/extra_test.go b/consensus/polybft/extra_test.go index cbcc269064..75c37c56b4 100644 --- a/consensus/polybft/extra_test.go +++ b/consensus/polybft/extra_test.go @@ -859,7 +859,6 @@ func Test_GetIbftExtraClean(t *testing.T) { }, }, }, - Seal: []byte{}, Committed: &Signature{ AggregatedSignature: []byte{23, 24}, Bitmap: []byte{11}, From 051d42f1b918c97de8e63bd07d11fa2aeec3db2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Tue, 18 Apr 2023 09:44:34 +0200 Subject: [PATCH 3/4] Fix test --- consensus/polybft/extra_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/consensus/polybft/extra_test.go b/consensus/polybft/extra_test.go index 75c37c56b4..13d313b0e3 100644 --- a/consensus/polybft/extra_test.go +++ b/consensus/polybft/extra_test.go @@ -876,9 +876,7 @@ func Test_GetIbftExtraClean(t *testing.T) { }, } - extraBytes := append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...) - - extraClean, err := GetIbftExtraClean(extraBytes) + extraClean, err := GetIbftExtraClean(extra.MarshalRLPTo(nil)) require.NoError(t, err) extraTwo := &Extra{} From 025879dc57ad4b3218e6b64601ef4fa367c6278f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Tue, 18 Apr 2023 10:12:16 +0200 Subject: [PATCH 4/4] Fix by not adding ExtraVanity twice --- consensus/polybft/extra.go | 2 +- consensus/polybft/extra_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/polybft/extra.go b/consensus/polybft/extra.go index a385e69876..f0ed67df5a 100644 --- a/consensus/polybft/extra.go +++ b/consensus/polybft/extra.go @@ -677,7 +677,7 @@ func GetIbftExtraClean(extraRaw []byte) ([]byte, error) { Committed: &Signature{}, } - return append(make([]byte, ExtraVanity), ibftExtra.MarshalRLPTo(nil)...), nil + return ibftExtra.MarshalRLPTo(nil), nil } // GetIbftExtra returns the istanbul extra data field from the passed in header diff --git a/consensus/polybft/extra_test.go b/consensus/polybft/extra_test.go index 13d313b0e3..585210db82 100644 --- a/consensus/polybft/extra_test.go +++ b/consensus/polybft/extra_test.go @@ -880,7 +880,7 @@ func Test_GetIbftExtraClean(t *testing.T) { require.NoError(t, err) extraTwo := &Extra{} - require.NoError(t, extraTwo.UnmarshalRLP(extraClean[ExtraVanity:])) + require.NoError(t, extraTwo.UnmarshalRLP(extraClean)) require.True(t, extra.Validators.Equals(extra.Validators)) require.Equal(t, extra.Checkpoint.BlockRound, extraTwo.Checkpoint.BlockRound) require.Equal(t, extra.Checkpoint.EpochNumber, extraTwo.Checkpoint.EpochNumber)