Skip to content

Commit

Permalink
Append ExtraVanity to the beginning of the marshaled ExtraData (#…
Browse files Browse the repository at this point in the history
…1391)

* Append ExtraVanity to the beginning of the marshaled extra data

* Build unit tests fix

* Fix test

* Fix by not adding ExtraVanity twice
  • Loading branch information
Stefan-Ethernal committed Apr 18, 2023
1 parent e37c9da commit f0f2716
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 45 deletions.
2 changes: 1 addition & 1 deletion command/genesis/polybft_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions consensus/polybft/checkpoint_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
19 changes: 7 additions & 12 deletions consensus/polybft/consensus_runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -759,15 +759,15 @@ 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),
},
}

proposalHash, err := extra.Checkpoint.Hash(0, block.Number(), block.Hash())
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{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 7 additions & 8 deletions consensus/polybft/extra.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -677,19 +677,18 @@ 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
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
}

Expand Down
9 changes: 3 additions & 6 deletions consensus/polybft/extra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -859,7 +859,6 @@ func Test_GetIbftExtraClean(t *testing.T) {
},
},
},
Seal: []byte{},
Committed: &Signature{
AggregatedSignature: []byte{23, 24},
Bitmap: []byte{11},
Expand All @@ -877,13 +876,11 @@ 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{}
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)
Expand Down
4 changes: 2 additions & 2 deletions consensus/polybft/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})

Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions consensus/polybft/fsm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions consensus/polybft/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions consensus/polybft/polybft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -47,15 +46,15 @@ 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 {
checkpointHash, err := extra.Checkpoint.Hash(0, header.Number, header.Hash)
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
Expand Down
4 changes: 2 additions & 2 deletions consensus/polybft/runtime_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/validators_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}

Expand Down

0 comments on commit f0f2716

Please sign in to comment.