diff --git a/consensus/XDPoS/engines/engine_v2/engine.go b/consensus/XDPoS/engines/engine_v2/engine.go index 633467a4862b..9bf32670677c 100644 --- a/consensus/XDPoS/engines/engine_v2/engine.go +++ b/consensus/XDPoS/engines/engine_v2/engine.go @@ -1,14 +1,12 @@ package engine_v2 import ( - "context" "encoding/json" "errors" "fmt" "math/big" "os" "path/filepath" - "runtime" "strconv" "sync" "time" @@ -27,7 +25,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/log" "github.com/XinFinOrg/XDPoSChain/params" "github.com/XinFinOrg/XDPoSChain/trie" - "golang.org/x/sync/errgroup" ) type XDPoS_v2 struct { @@ -743,53 +740,27 @@ func (x *XDPoS_v2) verifyQC(blockChainReader consensus.ChainReader, quorumCert * return errors.New("fail to verify QC due to failure in getting epoch switch info") } - signatures, duplicates := UniqueSignatures(quorumCert.Signatures) - if len(duplicates) != 0 { - for _, d := range duplicates { - log.Warn("[verifyQC] duplicated signature in QC", "duplicate", common.Bytes2Hex(d)) - } + signedVoteObj := types.VoteSigHash(&types.VoteForSign{ + ProposedBlockInfo: quorumCert.ProposedBlockInfo, + GapNumber: quorumCert.GapNumber, + }) + start := time.Now() + numValidSignatures, err := x.countValidSignatures(signedVoteObj, quorumCert.Signatures, epochInfo.Masternodes) + elapsed := time.Since(start) + log.Debug("[verifyQC] time verify message signatures of qc", "elapsed", elapsed) + if err != nil { + log.Error("[verifyQC] Error while verfying QC message signatures", "Error", err) + return err } qcRound := quorumCert.ProposedBlockInfo.Round certThreshold := x.config.V2.Config(uint64(qcRound)).CertThreshold - if (qcRound > 0) && (signatures == nil || float64(len(signatures)) < float64(epochInfo.MasternodesLen)*certThreshold) { + if (qcRound > 0) && (float64(numValidSignatures) < float64(epochInfo.MasternodesLen)*certThreshold) { //First V2 Block QC, QC Signatures is initial nil - log.Warn("[verifyHeader] Invalid QC Signature is nil or less then config", "QCNumber", quorumCert.ProposedBlockInfo.Number, "LenSignatures", len(signatures), "CertThreshold", float64(epochInfo.MasternodesLen)*certThreshold) + log.Warn("[verifyHeader] Invalid QC Signature is nil or less then config", "QCNumber", quorumCert.ProposedBlockInfo.Number, "numValidSignatures", numValidSignatures, "CertThreshold", float64(epochInfo.MasternodesLen)*certThreshold) return utils.ErrInvalidQCSignatures } - start := time.Now() - - eg, ctx := errgroup.WithContext(context.Background()) - eg.SetLimit(runtime.NumCPU()) - for _, sig := range signatures { - eg.Go(func() error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - verified, _, err := x.verifyMsgSignature(types.VoteSigHash(&types.VoteForSign{ - ProposedBlockInfo: quorumCert.ProposedBlockInfo, - GapNumber: quorumCert.GapNumber, - }), sig, epochInfo.Masternodes) - if err != nil { - log.Error("[verifyQC] Error while verfying QC message signatures", "Error", err) - return errors.New("error while verfying QC message signatures") - } - if !verified { - log.Warn("[verifyQC] Signature not verified doing QC verification", "QC", quorumCert) - return errors.New("fail to verify QC due to signature mis-match") - } - return nil - } - }) - } - err = eg.Wait() - elapsed := time.Since(start) - log.Debug("[verifyQC] time verify message signatures of qc", "elapsed", elapsed) - if err != nil { - return err - } epochSwitchNumber := epochInfo.EpochSwitchBlockInfo.Number.Uint64() gapNumber := epochSwitchNumber - epochSwitchNumber%x.config.Epoch if gapNumber > x.config.Gap { diff --git a/consensus/XDPoS/engines/engine_v2/timeout.go b/consensus/XDPoS/engines/engine_v2/timeout.go index 15f030cdd90d..d5292a09b6e5 100644 --- a/consensus/XDPoS/engines/engine_v2/timeout.go +++ b/consensus/XDPoS/engines/engine_v2/timeout.go @@ -1,10 +1,8 @@ package engine_v2 import ( - "context" "errors" "fmt" - "runtime" "strconv" "strings" "time" @@ -14,7 +12,6 @@ import ( "github.com/XinFinOrg/XDPoSChain/consensus/XDPoS/utils" "github.com/XinFinOrg/XDPoSChain/core/types" "github.com/XinFinOrg/XDPoSChain/log" - "golang.org/x/sync/errgroup" ) func (x *XDPoS_v2) VerifyTimeoutMessage(chain consensus.ChainReader, timeoutMsg *types.Timeout) (bool, error) { @@ -168,60 +165,29 @@ func (x *XDPoS_v2) verifyTC(chain consensus.ChainReader, timeoutCert *types.Time return errors.New("empty master node lists from snapshot") } - signatures, duplicates := UniqueSignatures(timeoutCert.Signatures) - if len(duplicates) != 0 { - for _, d := range duplicates { - log.Warn("[verifyTC] duplicated signature in QC", "duplicate", common.Bytes2Hex(d)) - } - } + signedTimeoutObj := types.TimeoutSigHash(&types.TimeoutForSign{ + Round: timeoutCert.Round, + GapNumber: timeoutCert.GapNumber, + }) + // TODO: snap.NextEpochCandidates should be replaced with epochInfo.Masternodes in another PR + numValidSignatures, err := x.countValidSignatures(signedTimeoutObj, timeoutCert.Signatures, snap.NextEpochCandidates) + if err != nil { + log.Error("[verifyTC] Error while verfying TC message signatures", "tcRound", timeoutCert.Round, "tcGapNumber", timeoutCert.GapNumber, "tcSignLen", len(timeoutCert.Signatures), "Error", err) + return err + } epochInfo, err := x.getTCEpochInfo(chain, timeoutCert) if err != nil { return err } certThreshold := x.config.V2.Config(uint64(timeoutCert.Round)).CertThreshold - if float64(len(signatures)) < float64(epochInfo.MasternodesLen)*certThreshold { + if float64(numValidSignatures) < float64(epochInfo.MasternodesLen)*certThreshold { log.Warn("[verifyTC] Invalid TC Signature is less or empty", "tcRound", timeoutCert.Round, "tcGapNumber", timeoutCert.GapNumber, "tcSignLen", len(timeoutCert.Signatures), "certThreshold", float64(epochInfo.MasternodesLen)*certThreshold) return utils.ErrInvalidTCSignatures } - signedTimeoutObj := types.TimeoutSigHash(&types.TimeoutForSign{ - Round: timeoutCert.Round, - GapNumber: timeoutCert.GapNumber, - }) - - eg, ctx := errgroup.WithContext(context.Background()) - eg.SetLimit(runtime.NumCPU()) - - for _, sig := range signatures { - eg.Go(func() error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - verified, _, err := x.verifyMsgSignature(signedTimeoutObj, sig, snap.NextEpochCandidates) - if err != nil { - log.Error("[verifyTC] Error while verifying TC message signatures", - "tcRound", timeoutCert.Round, - "tcGapNumber", timeoutCert.GapNumber, - "tcSignLen", len(signatures), - "error", err) - return fmt.Errorf("error while verifying TC message signatures: %w", err) - } - if !verified { - log.Warn("[verifyTC] Signature not verified during TC verification", - "tcRound", timeoutCert.Round, - "tcGapNumber", timeoutCert.GapNumber, - "tcSignLen", len(signatures)) - return errors.New("fail to verify TC due to signature mis-match") - } - return nil - } - }) - } - - return eg.Wait() + return nil } /* diff --git a/consensus/XDPoS/engines/engine_v2/utils.go b/consensus/XDPoS/engines/engine_v2/utils.go index 74a557b6704a..d2eb8d46f9a9 100644 --- a/consensus/XDPoS/engines/engine_v2/utils.go +++ b/consensus/XDPoS/engines/engine_v2/utils.go @@ -1,9 +1,11 @@ package engine_v2 import ( + "context" "errors" "fmt" "math/big" + "runtime" "github.com/XinFinOrg/XDPoSChain/accounts" "github.com/XinFinOrg/XDPoSChain/common" @@ -14,6 +16,7 @@ import ( "github.com/XinFinOrg/XDPoSChain/log" "github.com/XinFinOrg/XDPoSChain/rlp" "golang.org/x/crypto/sha3" + "golang.org/x/sync/errgroup" ) func sigHash(header *types.Header) (hash common.Hash) { @@ -76,22 +79,6 @@ func decodeMasternodesFromHeaderExtra(checkpointHeader *types.Header) []common.A return masternodes } -func UniqueSignatures(signatureSlice []types.Signature) ([]types.Signature, []types.Signature) { - keys := make(map[string]bool) - list := []types.Signature{} - duplicates := []types.Signature{} - for _, signature := range signatureSlice { - hexOfSig := common.Bytes2Hex(signature) - if _, value := keys[hexOfSig]; !value { - keys[hexOfSig] = true - list = append(list, signature) - } else { - duplicates = append(duplicates, signature) - } - } - return list, duplicates -} - func (x *XDPoS_v2) signSignature(signingHash common.Hash) (types.Signature, error) { // Don't hold the signFn for the whole signing operation x.signLock.RLock() @@ -108,6 +95,53 @@ func (x *XDPoS_v2) signSignature(signingHash common.Hash) (types.Signature, erro return signedHash, nil } +func (x *XDPoS_v2) countValidSignatures(messageHash common.Hash, signatures []types.Signature, candidates []common.Address) (int, error) { + signatureList := make([]types.Signature, len(signatures)) + pubkeys := make([]common.Address, len(signatures)) + + eg, ctx := errgroup.WithContext(context.Background()) + eg.SetLimit(runtime.NumCPU()) + for i, signature := range signatures { + eg.Go(func() error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + verified, signerAddress, err := x.verifyMsgSignature(messageHash, signature, candidates) + if err != nil { + log.Error("[verifySignatures] Error while verfying QC message signatures", "error", err) + return err + } + if !verified { + return fmt.Errorf("signature verification failed, signer is not part of masternode list. Signature: %v, SignedMessage: %v, SignerAddress: %v, Masternodes: %v", signature, messageHash.Hex(), signerAddress, candidates) + } + signatureList[i] = signature + pubkeys[i] = signerAddress + + return nil + } + }) + } + err := eg.Wait() + if err != nil { + return 0, err + } + + // check uniqueness + keys := make(map[common.Address]struct{}, len(signatureList)) + for i := range signatureList { + pubkeyHex := pubkeys[i] + if _, ok := keys[pubkeyHex]; !ok { + keys[pubkeyHex] = struct{}{} + } else { + log.Warn("[verifySignatures] duplicate signing found", "pubkey", pubkeyHex, "signedMessage", messageHash.Hex(), "signature", signatureList[i]) + return 0, fmt.Errorf("duplicate signing found, pubkey: %v, message: %v, signature: %v", pubkeyHex, messageHash.Hex(), common.Bytes2Hex(signatureList[i])) + } + } + + return len(keys), nil +} + func (x *XDPoS_v2) verifyMsgSignature(signedHashToBeVerified common.Hash, signature types.Signature, masternodes []common.Address) (bool, common.Address, error) { var signerAddress common.Address if len(masternodes) == 0 { @@ -126,7 +160,7 @@ func (x *XDPoS_v2) verifyMsgSignature(signedHashToBeVerified common.Hash, signat } } - log.Warn("[verifyMsgSignature] signer is not part of masternode list", "signer", signerAddress, "masternodes", masternodes) + log.Warn("[verifyMsgSignature] signer is not part of masternode list", "signer", signerAddress, "masternodes", masternodes, "signature", common.Bytes2Hex(signature), "signedMessage", signedHashToBeVerified.Hex()) return false, signerAddress, nil } diff --git a/consensus/tests/engine_v2_tests/verify_header_test.go b/consensus/tests/engine_v2_tests/verify_header_test.go index 68f9617a445d..c7f494bbefe1 100644 --- a/consensus/tests/engine_v2_tests/verify_header_test.go +++ b/consensus/tests/engine_v2_tests/verify_header_test.go @@ -394,7 +394,7 @@ func TestShouldFailIfNotEnoughQCSignatures(t *testing.T) { headerWithDuplicatedSignatures.Extra = extraInBytes // Happy path err = adaptor.VerifyHeader(blockchain, headerWithDuplicatedSignatures, true) - assert.Equal(t, utils.ErrInvalidQCSignatures, err) + assert.ErrorContains(t, err, "duplicate signing found") }