Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EVM-736]: Quorum calculation discrepancy between SC and Edge #1723

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 32 additions & 28 deletions chain/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,16 @@ func (p *Params) GetEngine() string {

// predefined forks
const (
Homestead = "homestead"
Byzantium = "byzantium"
Constantinople = "constantinople"
Petersburg = "petersburg"
Istanbul = "istanbul"
London = "london"
EIP150 = "EIP150"
EIP158 = "EIP158"
EIP155 = "EIP155"
Homestead = "homestead"
Byzantium = "byzantium"
Constantinople = "constantinople"
Petersburg = "petersburg"
Istanbul = "istanbul"
London = "london"
EIP150 = "EIP150"
EIP158 = "EIP158"
EIP155 = "EIP155"
QuorumCalcAlignment = "quorumcalcalignment"
)

// Forks is map which contains all forks and their starting blocks from genesis
Expand All @@ -111,15 +112,16 @@ func (f *Forks) RemoveFork(name string) {
// At returns ForksInTime instance that shows which supported forks are enabled for the block
func (f *Forks) At(block uint64) ForksInTime {
return ForksInTime{
Homestead: f.IsActive(Homestead, block),
Byzantium: f.IsActive(Byzantium, block),
Constantinople: f.IsActive(Constantinople, block),
Petersburg: f.IsActive(Petersburg, block),
Istanbul: f.IsActive(Istanbul, block),
London: f.IsActive(London, block),
EIP150: f.IsActive(EIP150, block),
EIP158: f.IsActive(EIP158, block),
EIP155: f.IsActive(EIP155, block),
Homestead: f.IsActive(Homestead, block),
Byzantium: f.IsActive(Byzantium, block),
Constantinople: f.IsActive(Constantinople, block),
Petersburg: f.IsActive(Petersburg, block),
Istanbul: f.IsActive(Istanbul, block),
London: f.IsActive(London, block),
EIP150: f.IsActive(EIP150, block),
EIP158: f.IsActive(EIP158, block),
EIP155: f.IsActive(EIP155, block),
QuorumCalcAlignment: f.IsActive(QuorumCalcAlignment, block),
}
}

Expand Down Expand Up @@ -164,18 +166,20 @@ type ForksInTime struct {
London,
EIP150,
EIP158,
EIP155 bool
EIP155,
QuorumCalcAlignment bool
}

// AllForksEnabled should contain all supported forks by current edge version
var AllForksEnabled = &Forks{
Homestead: NewFork(0),
EIP150: NewFork(0),
EIP155: NewFork(0),
EIP158: NewFork(0),
Byzantium: NewFork(0),
Constantinople: NewFork(0),
Petersburg: NewFork(0),
Istanbul: NewFork(0),
London: NewFork(0),
Homestead: NewFork(0),
EIP150: NewFork(0),
EIP155: NewFork(0),
EIP158: NewFork(0),
Byzantium: NewFork(0),
Constantinople: NewFork(0),
Petersburg: NewFork(0),
Istanbul: NewFork(0),
London: NewFork(0),
QuorumCalcAlignment: NewFork(0),
}
2 changes: 1 addition & 1 deletion consensus/polybft/consensus_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (c *consensusRuntime) FSM() error {
}

if isEndOfSprint {
commitment, err := c.stateSyncManager.Commitment()
commitment, err := c.stateSyncManager.Commitment(pendingBlockNumber)
if err != nil {
return err
}
Expand Down
12 changes: 7 additions & 5 deletions consensus/polybft/extra.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (i *Extra) ValidateFinalizedData(header *types.Header, parent *types.Header
}

// validate current block signatures
checkpointHash, err := i.Checkpoint.Hash(chainID, header.Number, header.Hash)
checkpointHash, err := i.Checkpoint.Hash(chainID, blockNumber, header.Hash)
if err != nil {
return fmt.Errorf("failed to calculate proposal hash: %w", err)
}
Expand All @@ -151,7 +151,7 @@ func (i *Extra) ValidateFinalizedData(header *types.Header, parent *types.Header
return fmt.Errorf("failed to validate header for block %d. could not retrieve block validators:%w", blockNumber, err)
}

if err := i.Committed.Verify(validators, checkpointHash, domain, logger); err != nil {
if err := i.Committed.Verify(blockNumber, validators, checkpointHash, domain, logger); err != nil {
return fmt.Errorf("failed to verify signatures for block %d (proposal hash %s): %w",
blockNumber, checkpointHash, err)
}
Expand Down Expand Up @@ -197,7 +197,8 @@ func (i *Extra) ValidateParentSignatures(blockNumber uint64, consensusBackend po
return fmt.Errorf("failed to calculate parent proposal hash: %w", err)
}

if err := i.Parent.Verify(parentValidators, parentCheckpointHash, domain, logger); err != nil {
parentBlockNumber := blockNumber - 1
if err := i.Parent.Verify(parentBlockNumber, parentValidators, parentCheckpointHash, domain, logger); err != nil {
return fmt.Errorf("failed to verify signatures for parent of block %d (proposal hash: %s): %w",
blockNumber, parentCheckpointHash, err)
}
Expand Down Expand Up @@ -256,14 +257,15 @@ func (s *Signature) UnmarshalRLPWith(v *fastrlp.Value) error {
}

// Verify is used to verify aggregated signature based on current validator set, message hash and domain
func (s *Signature) Verify(validators validator.AccountSet, hash types.Hash, domain []byte, logger hclog.Logger) error {
func (s *Signature) Verify(blockNumber uint64, validators validator.AccountSet,
hash types.Hash, domain []byte, logger hclog.Logger) error {
signers, err := validators.GetFilteredValidators(s.Bitmap)
if err != nil {
return err
}

validatorSet := validator.NewValidatorSet(validators, logger)
if !validatorSet.HasQuorum(signers.GetAddressesAsSet()) {
if !validatorSet.HasQuorum(blockNumber, signers.GetAddressesAsSet()) {
return fmt.Errorf("quorum not reached")
}

Expand Down
8 changes: 4 additions & 4 deletions consensus/polybft/extra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,10 @@ func TestSignature_Verify(t *testing.T) {
Bitmap: bitmap,
}

err = s.Verify(validatorsMetadata, msgHash, bls.DomainCheckpointManager, hclog.NewNullLogger())
err = s.Verify(10, validatorsMetadata, msgHash, bls.DomainCheckpointManager, hclog.NewNullLogger())
signers[val.Address()] = struct{}{}

if !validatorSet.HasQuorum(signers) {
if !validatorSet.HasQuorum(10, signers) {
assert.ErrorContains(t, err, "quorum not reached", "failed for %d", i)
} else {
assert.NoError(t, err)
Expand All @@ -415,7 +415,7 @@ func TestSignature_Verify(t *testing.T) {
bmp.Set(uint64(validatorSet.Len() + 1))
s := &Signature{Bitmap: bmp}

err := s.Verify(validatorSet, types.Hash{0x1}, bls.DomainCheckpointManager, hclog.NewNullLogger())
err := s.Verify(0, validatorSet, types.Hash{0x1}, bls.DomainCheckpointManager, hclog.NewNullLogger())
require.Error(t, err)
})
}
Expand Down Expand Up @@ -483,7 +483,7 @@ func TestSignature_VerifyRandom(t *testing.T) {
Bitmap: bitmap,
}

err = s.Verify(vals.GetPublicIdentities(), msgHash, bls.DomainCheckpointManager, hclog.NewNullLogger())
err = s.Verify(1, vals.GetPublicIdentities(), msgHash, bls.DomainCheckpointManager, hclog.NewNullLogger())
assert.NoError(t, err)
}

Expand Down
6 changes: 3 additions & 3 deletions consensus/polybft/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func (f *fsm) VerifyStateTransactions(transactions []*types.Transaction) error {

commitmentTxExists = true

if err = verifyBridgeCommitmentTx(tx.Hash, stateTxData, f.validators); err != nil {
if err = verifyBridgeCommitmentTx(f.Height(), tx.Hash, stateTxData, f.validators); err != nil {
return err
}
case *contractsapi.CommitEpochValidatorSetFn:
Expand Down Expand Up @@ -623,15 +623,15 @@ func (f *fsm) verifyDistributeRewardsTx(distributeRewardsTx *types.Transaction)
}

// verifyBridgeCommitmentTx validates bridge commitment transaction
func verifyBridgeCommitmentTx(txHash types.Hash,
func verifyBridgeCommitmentTx(blockNumber uint64, txHash types.Hash,
commitment *CommitmentMessageSigned,
validators validator.ValidatorSet) error {
signers, err := validators.Accounts().GetFilteredValidators(commitment.AggSignature.Bitmap)
if err != nil {
return fmt.Errorf("failed to retrieve signers for state tx (%s): %w", txHash, err)
}

if !validators.HasQuorum(signers.GetAddressesAsSet()) {
if !validators.HasQuorum(blockNumber, signers.GetAddressesAsSet()) {
return fmt.Errorf("quorum size not reached for state tx (%s)", txHash)
}

Expand Down
6 changes: 6 additions & 0 deletions consensus/polybft/fsm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ func TestFSM_VerifyStateTransactions_StateTransactionQuorumNotReached(t *testing
validatorSet := validator.NewValidatorSet(validators.GetPublicIdentities(), hclog.NewNullLogger())

fsm := &fsm{
parent: &types.Header{Number: 2},
isEndOfEpoch: true,
isEndOfSprint: true,
validators: validatorSet,
Expand Down Expand Up @@ -598,6 +599,7 @@ func TestFSM_VerifyStateTransactions_StateTransactionInvalidSignature(t *testing
validatorSet := validator.NewValidatorSet(validators.GetPublicIdentities(), hclog.NewNullLogger())

fsm := &fsm{
parent: &types.Header{Number: 1},
isEndOfEpoch: true,
isEndOfSprint: true,
validators: validatorSet,
Expand Down Expand Up @@ -1329,6 +1331,7 @@ func TestFSM_VerifyStateTransaction_ValidBothTypesOfStateTransactions(t *testing
}

f := &fsm{
parent: &types.Header{Number: 9},
isEndOfSprint: true,
validators: validators.ToValidatorSet(),
}
Expand Down Expand Up @@ -1372,6 +1375,7 @@ func TestFSM_VerifyStateTransaction_QuorumNotReached(t *testing.T) {
validators := validator.NewTestValidatorsWithAliases(t, []string{"A", "B", "C", "D", "E", "F"})
_, commitmentMessageSigned, _ := buildCommitmentAndStateSyncs(t, 10, uint64(3), 2)
f := &fsm{
parent: &types.Header{Number: 9},
isEndOfSprint: true,
validators: validators.ToValidatorSet(),
}
Expand Down Expand Up @@ -1400,6 +1404,7 @@ func TestFSM_VerifyStateTransaction_InvalidSignature(t *testing.T) {
validators := validator.NewTestValidatorsWithAliases(t, []string{"A", "B", "C", "D", "E", "F"})
_, commitmentMessageSigned, _ := buildCommitmentAndStateSyncs(t, 10, uint64(3), 2)
f := &fsm{
parent: &types.Header{Number: 9},
isEndOfSprint: true,
validators: validators.ToValidatorSet(),
}
Expand Down Expand Up @@ -1437,6 +1442,7 @@ func TestFSM_VerifyStateTransaction_TwoCommitmentMessages(t *testing.T) {
validatorSet := validator.NewValidatorSet(validators.GetPublicIdentities(), hclog.NewNullLogger())

f := &fsm{
parent: &types.Header{Number: 9},
isEndOfSprint: true,
validators: validatorSet,
}
Expand Down
1 change: 1 addition & 0 deletions consensus/polybft/polybft.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ func (p *Polybft) PreCommitState(block *types.Block, _ *state.Transition) error
commitmentTxExists = true

if err := verifyBridgeCommitmentTx(
block.Number(),
tx.Hash,
signedCommitment,
validator.NewValidatorSet(validators, p.logger)); err != nil {
Expand Down
22 changes: 12 additions & 10 deletions consensus/polybft/state_sync_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type StateSyncProof struct {
type StateSyncManager interface {
Init() error
Close()
Commitment() (*CommitmentMessageSigned, error)
Commitment(blockNumber uint64) (*CommitmentMessageSigned, error)
GetStateSyncProof(stateSyncID uint64) (types.Proof, error)
PostBlock(req *PostBlockRequest) error
PostEpoch(req *PostEpochRequest) error
Expand All @@ -47,11 +47,13 @@ var _ StateSyncManager = (*dummyStateSyncManager)(nil)
// dummyStateSyncManager is used when bridge is not enabled
type dummyStateSyncManager struct{}

func (n *dummyStateSyncManager) Init() error { return nil }
func (n *dummyStateSyncManager) Close() {}
func (n *dummyStateSyncManager) Commitment() (*CommitmentMessageSigned, error) { return nil, nil }
func (n *dummyStateSyncManager) PostBlock(req *PostBlockRequest) error { return nil }
func (n *dummyStateSyncManager) PostEpoch(req *PostEpochRequest) error { return nil }
func (n *dummyStateSyncManager) Init() error { return nil }
func (n *dummyStateSyncManager) Close() {}
func (n *dummyStateSyncManager) Commitment(blockNumber uint64) (*CommitmentMessageSigned, error) {
return nil, nil
}
func (n *dummyStateSyncManager) PostBlock(req *PostBlockRequest) error { return nil }
func (n *dummyStateSyncManager) PostEpoch(req *PostEpochRequest) error { return nil }
func (n *dummyStateSyncManager) GetStateSyncProof(stateSyncID uint64) (types.Proof, error) {
return types.Proof{}, nil
}
Expand Down Expand Up @@ -268,7 +270,7 @@ func (s *stateSyncManager) AddLog(eventLog *ethgo.Log) error {
}

// Commitment returns a commitment to be submitted if there is a pending commitment with quorum
func (s *stateSyncManager) Commitment() (*CommitmentMessageSigned, error) {
func (s *stateSyncManager) Commitment(blockNumber uint64) (*CommitmentMessageSigned, error) {
s.lock.RLock()
defer s.lock.RUnlock()

Expand All @@ -277,7 +279,7 @@ func (s *stateSyncManager) Commitment() (*CommitmentMessageSigned, error) {
// we start from the end, since last pending commitment is the largest one
for i := len(s.pendingCommitments) - 1; i >= 0; i-- {
commitment := s.pendingCommitments[i]
aggregatedSignature, publicKeys, err := s.getAggSignatureForCommitmentMessage(commitment)
aggregatedSignature, publicKeys, err := s.getAggSignatureForCommitmentMessage(blockNumber, commitment)

if err != nil {
if errors.Is(err, errQuorumNotReached) {
Expand Down Expand Up @@ -306,7 +308,7 @@ func (s *stateSyncManager) Commitment() (*CommitmentMessageSigned, error) {

// getAggSignatureForCommitmentMessage checks if pending commitment has quorum,
// and if it does, aggregates the signatures
func (s *stateSyncManager) getAggSignatureForCommitmentMessage(
func (s *stateSyncManager) getAggSignatureForCommitmentMessage(blockNumber uint64,
commitment *PendingCommitment) (Signature, [][]byte, error) {
validatorSet := s.validatorSet

Expand Down Expand Up @@ -352,7 +354,7 @@ func (s *stateSyncManager) getAggSignatureForCommitmentMessage(
signers[types.StringToAddress(vote.From)] = struct{}{}
}

if !validatorSet.HasQuorum(signers) {
if !validatorSet.HasQuorum(blockNumber, signers) {
return Signature{}, nil, errQuorumNotReached
}

Expand Down
6 changes: 3 additions & 3 deletions consensus/polybft/state_sync_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func TestStateSyncManager_BuildCommitment(t *testing.T) {
s.validatorSet = vals.ToValidatorSet()

// commitment is empty
commitment, err := s.Commitment()
commitment, err := s.Commitment(1)
require.NoError(t, err)
require.Nil(t, commitment)

Expand Down Expand Up @@ -262,7 +262,7 @@ func TestStateSyncManager_BuildCommitment(t *testing.T) {
require.NoError(t, s.saveVote(signedMsg1))
require.NoError(t, s.saveVote(signedMsg2))

commitment, err = s.Commitment()
commitment, err = s.Commitment(1)
require.NoError(t, err) // there is no error if quorum is not met, since its a valid case
require.Nil(t, commitment)

Expand All @@ -277,7 +277,7 @@ func TestStateSyncManager_BuildCommitment(t *testing.T) {
require.NoError(t, s.saveVote(signedMsg1))
require.NoError(t, s.saveVote(signedMsg2))

commitment, err = s.Commitment()
commitment, err = s.Commitment(1)
require.NoError(t, err)
require.NotNil(t, commitment)
}
Expand Down
Loading