From 7ca6a96b4f1a0f4839ce5b30359cc03efd66ceef Mon Sep 17 00:00:00 2001 From: Mostafa Date: Sun, 4 Aug 2024 21:46:20 +0800 Subject: [PATCH 1/2] refactor(util): remove ErrInvalidBlock code --- state/errors.go | 19 +++++++++++++++++++ state/execution.go | 13 ++++++------- state/state.go | 2 -- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/state/errors.go b/state/errors.go index 0bbbe8b03..cdc862a35 100644 --- a/state/errors.go +++ b/state/errors.go @@ -1,12 +1,31 @@ package state import ( + "errors" "fmt" + "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/certificate" "github.com/pactus-project/pactus/types/vote" ) +// ErrInvalidSubsidyTransaction indicates that the subsidy transaction is not valid. +var ErrInvalidSubsidyTransaction = errors.New("invalid subsidy transaction") + +// ErrDuplicatedSubsidyTransaction indicates that there is more than one subsidy transaction +// inside the block. +var ErrDuplicatedSubsidyTransaction = errors.New("duplicated subsidy transaction") + +// InvalidSubsidyAmountError is returned when the amount of the subsidy transaction is not as expected. +type InvalidSubsidyAmountError struct { + Expected amount.Amount + Got amount.Amount +} + +func (e InvalidSubsidyAmountError) Error() string { + return fmt.Sprintf("invalid subsidy amount, expected %v, got %v", e.Expected, e.Got) +} + // InvalidVoteForCertificateError is returned when an attempt to update // the last certificate with an invalid vote is made. type InvalidVoteForCertificateError struct { diff --git a/state/execution.go b/state/execution.go index 6199af4fc..205200de0 100644 --- a/state/execution.go +++ b/state/execution.go @@ -6,7 +6,6 @@ import ( "github.com/pactus-project/pactus/sandbox" "github.com/pactus-project/pactus/types/block" "github.com/pactus-project/pactus/types/tx" - "github.com/pactus-project/pactus/util/errors" ) func (st *state) executeBlock(b *block.Block, sb sandbox.Sandbox, check bool) error { @@ -16,13 +15,11 @@ func (st *state) executeBlock(b *block.Block, sb sandbox.Sandbox, check bool) er isSubsidyTx := (i == 0) if isSubsidyTx { if !trx.IsSubsidyTx() { - return errors.Errorf(errors.ErrInvalidTx, - "first transaction should be a subsidy transaction") + return ErrInvalidSubsidyTransaction } subsidyTrx = trx } else if trx.IsSubsidyTx() { - return errors.Errorf(errors.ErrInvalidTx, - "duplicated subsidy transaction") + return ErrDuplicatedSubsidyTransaction } if check { @@ -45,8 +42,10 @@ func (st *state) executeBlock(b *block.Block, sb sandbox.Sandbox, check bool) er accumulatedFee := sb.AccumulatedFee() subsidyAmt := st.params.BlockReward + sb.AccumulatedFee() if subsidyTrx.Payload().Value() != subsidyAmt { - return errors.Errorf(errors.ErrInvalidTx, - "invalid subsidy amount, expected %v, got %v", subsidyAmt, subsidyTrx.Payload().Value()) + return InvalidSubsidyAmountError{ + Expected: subsidyAmt, + Got: subsidyTrx.Payload().Value(), + } } // Claim accumulated fees diff --git a/state/state.go b/state/state.go index 97f590c5c..7ebeb10df 100644 --- a/state/state.go +++ b/state/state.go @@ -408,8 +408,6 @@ func (st *state) CommitBlock(blk *block.Block, cert *certificate.BlockCertificat st.logger.Panic("a possible fork is detected", "our hash", st.lastInfo.BlockHash(), "block hash", blk.Header().PrevBlockHash()) - - return errors.Error(errors.ErrInvalidBlock) } err = st.validateBlock(blk, cert.Round()) From fc5609c7f52b90fd2eba817bec95c743914347ba Mon Sep 17 00:00:00 2001 From: Mostafa Date: Tue, 24 Sep 2024 02:16:54 +0800 Subject: [PATCH 2/2] fix(other): define errors for the state --- state/errors.go | 48 ++++++++++++++++++++---- state/execution.go | 8 ++-- state/execution_test.go | 30 +++++++++------ state/state.go | 28 +++++++++----- state/validation.go | 26 ++++++------- state/validation_test.go | 65 +++++++++++++++++++++++---------- types/proposal/errors.go | 15 ++++++++ types/proposal/proposal.go | 13 +++---- types/proposal/proposal_test.go | 26 ++++++++++--- util/errors/errors.go | 8 ---- util/errors/errors_test.go | 15 -------- 11 files changed, 182 insertions(+), 100 deletions(-) create mode 100644 types/proposal/errors.go diff --git a/state/errors.go b/state/errors.go index cdc862a35..e81e6db41 100644 --- a/state/errors.go +++ b/state/errors.go @@ -4,11 +4,15 @@ import ( "errors" "fmt" + "github.com/pactus-project/pactus/crypto" + "github.com/pactus-project/pactus/crypto/hash" "github.com/pactus-project/pactus/types/amount" - "github.com/pactus-project/pactus/types/certificate" "github.com/pactus-project/pactus/types/vote" ) +// ErrInvalidBlockVersion indicates that the block version is not valid. +var ErrInvalidBlockVersion = errors.New("invalid block version") + // ErrInvalidSubsidyTransaction indicates that the subsidy transaction is not valid. var ErrInvalidSubsidyTransaction = errors.New("invalid subsidy transaction") @@ -16,6 +20,12 @@ var ErrInvalidSubsidyTransaction = errors.New("invalid subsidy transaction") // inside the block. var ErrDuplicatedSubsidyTransaction = errors.New("duplicated subsidy transaction") +// ErrInvalidSortitionSeed indicates that the block's sortition seed is either invalid or unverifiable. +var ErrInvalidSortitionSeed = errors.New("invalid sortition seed") + +// ErrInvalidCertificate indicates that the block certificate is invalid. +var ErrInvalidCertificate = errors.New("invalid certificate") + // InvalidSubsidyAmountError is returned when the amount of the subsidy transaction is not as expected. type InvalidSubsidyAmountError struct { Expected amount.Amount @@ -23,7 +33,7 @@ type InvalidSubsidyAmountError struct { } func (e InvalidSubsidyAmountError) Error() string { - return fmt.Sprintf("invalid subsidy amount, expected %v, got %v", e.Expected, e.Got) + return fmt.Sprintf("invalid subsidy amount, expected: %v, got: %v", e.Expected, e.Got) } // InvalidVoteForCertificateError is returned when an attempt to update @@ -37,12 +47,34 @@ func (e InvalidVoteForCertificateError) Error() string { e.Vote.String()) } -// InvalidBlockCertificateError is returned when the given certificate is invalid. -type InvalidBlockCertificateError struct { - Cert *certificate.BlockCertificate +// InvalidStateRootHashError is returned when the state root hash of the block +// does not match the current state root hash. +type InvalidStateRootHashError struct { + Expected hash.Hash + Got hash.Hash +} + +func (e InvalidStateRootHashError) Error() string { + return fmt.Sprintf("invalid state root hash, expected: %s, got: %s", + e.Expected, e.Got) +} + +// InvalidProposerError is returned when the block proposer is not as expected. +type InvalidProposerError struct { + Expected crypto.Address + Got crypto.Address +} + +func (e InvalidProposerError) Error() string { + return fmt.Sprintf("invalid block proposer, expected: %s, got: %s", + e.Expected, e.Got) +} + +// InvalidBlockTimeError is returned when the block time is not valid. +type InvalidBlockTimeError struct { + Reason string } -func (e InvalidBlockCertificateError) Error() string { - return fmt.Sprintf("invalid certificate for block %d", - e.Cert.Height()) +func (e InvalidBlockTimeError) Error() string { + return fmt.Sprintf("invalid block time: %s", e.Reason) } diff --git a/state/execution.go b/state/execution.go index 205200de0..0843960d3 100644 --- a/state/execution.go +++ b/state/execution.go @@ -1,6 +1,8 @@ package state import ( + "fmt" + "github.com/pactus-project/pactus/crypto" "github.com/pactus-project/pactus/execution" "github.com/pactus-project/pactus/sandbox" @@ -60,14 +62,12 @@ func (st *state) checkEd25519Fork(trx *tx.Tx) error { // TODO: remove me after enabling Ed255519 if trx.Payload().Signer().Type() == crypto.AddressTypeEd25519Account { if st.genDoc.ChainType().IsMainnet() { - return errors.Errorf(errors.ErrInvalidTx, - "ed255519 not supported yet") + return fmt.Errorf("ed255519 not supported yet") } if st.genDoc.ChainType().IsTestnet() { if st.lastInfo.BlockHeight() < 1_320_000 { - return errors.Errorf(errors.ErrInvalidTx, - "ed255519 not supported yet") + return fmt.Errorf("ed255519 not supported yet") } } } diff --git a/state/execution_test.go b/state/execution_test.go index 5e2a41f53..de6325fcb 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -5,6 +5,8 @@ import ( "time" "github.com/pactus-project/pactus/crypto" + "github.com/pactus-project/pactus/execution/executor" + "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/block" "github.com/pactus-project/pactus/types/tx" "github.com/pactus-project/pactus/util/testsuite" @@ -64,15 +66,19 @@ func TestExecuteBlock(t *testing.T) { assert.NoError(t, td.state.AddPendingTx(invSubsidyTx)) assert.NoError(t, td.state.AddPendingTx(validTx1)) - t.Run("Subsidy tx is invalid", func(t *testing.T) { + t.Run("Subsidy amount is invalid", func(t *testing.T) { txs := block.NewTxs() txs.Append(invSubsidyTx) + txs.Append(validTx1) invBlock := block.MakeBlock(1, time.Now(), txs, td.state.lastInfo.BlockHash(), td.state.stateRoot(), td.state.lastInfo.Certificate(), td.state.lastInfo.SortitionSeed(), proposerAddr) sb := td.state.concreteSandbox() - - assert.Error(t, td.state.executeBlock(invBlock, sb, true)) + err := td.state.executeBlock(invBlock, sb, true) + assert.ErrorIs(t, err, InvalidSubsidyAmountError{ + Expected: amount.Amount(1e9 + 1000), + Got: amount.Amount(1e9 + 1001), + }) }) t.Run("Has invalid tx", func(t *testing.T) { @@ -83,8 +89,10 @@ func TestExecuteBlock(t *testing.T) { td.state.stateRoot(), td.state.lastInfo.Certificate(), td.state.lastInfo.SortitionSeed(), proposerAddr) sb := td.state.concreteSandbox() - - assert.Error(t, td.state.executeBlock(invBlock, sb, true)) + err := td.state.executeBlock(invBlock, sb, true) + assert.ErrorIs(t, err, executor.AccountNotFoundError{ + Address: invTransferTx.Payload().Signer(), + }) }) t.Run("Subsidy is not first tx", func(t *testing.T) { @@ -95,8 +103,8 @@ func TestExecuteBlock(t *testing.T) { td.state.stateRoot(), td.state.lastInfo.Certificate(), td.state.lastInfo.SortitionSeed(), proposerAddr) sb := td.state.concreteSandbox() - - assert.Error(t, td.state.executeBlock(invBlock, sb, true)) + err := td.state.executeBlock(invBlock, sb, true) + assert.ErrorIs(t, err, ErrInvalidSubsidyTransaction) }) t.Run("Has no subsidy", func(t *testing.T) { @@ -106,8 +114,8 @@ func TestExecuteBlock(t *testing.T) { td.state.stateRoot(), td.state.lastInfo.Certificate(), td.state.lastInfo.SortitionSeed(), proposerAddr) sb := td.state.concreteSandbox() - - assert.Error(t, td.state.executeBlock(invBlock, sb, true)) + err := td.state.executeBlock(invBlock, sb, true) + assert.ErrorIs(t, err, ErrInvalidSubsidyTransaction) }) t.Run("Two subsidy transactions", func(t *testing.T) { @@ -118,8 +126,8 @@ func TestExecuteBlock(t *testing.T) { td.state.stateRoot(), td.state.lastInfo.Certificate(), td.state.lastInfo.SortitionSeed(), proposerAddr) sb := td.state.concreteSandbox() - - assert.Error(t, td.state.executeBlock(invBlock, sb, true)) + err := td.state.executeBlock(invBlock, sb, true) + assert.ErrorIs(t, err, ErrDuplicatedSubsidyTransaction) }) t.Run("OK", func(t *testing.T) { diff --git a/state/state.go b/state/state.go index 7ebeb10df..1b1906906 100644 --- a/state/state.go +++ b/state/state.go @@ -28,7 +28,6 @@ import ( "github.com/pactus-project/pactus/types/validator" "github.com/pactus-project/pactus/types/vote" "github.com/pactus-project/pactus/util" - "github.com/pactus-project/pactus/util/errors" "github.com/pactus-project/pactus/util/logger" "github.com/pactus-project/pactus/util/persistentmerkle" "github.com/pactus-project/pactus/util/simplemerkle" @@ -305,9 +304,9 @@ func (st *state) UpdateLastCertificate(v *vote.Vote) error { return nil } -func (st *state) createSubsidyTx(rewardAddr crypto.Address, fee amount.Amount) *tx.Tx { +func (st *state) createSubsidyTx(rewardAddr crypto.Address, accumulatedFee amount.Amount) *tx.Tx { lockTime := st.lastInfo.BlockHeight() + 1 - transaction := tx.NewSubsidyTx(lockTime, rewardAddr, st.params.BlockReward+fee) + transaction := tx.NewSubsidyTx(lockTime, rewardAddr, st.params.BlockReward+accumulatedFee) return transaction } @@ -345,7 +344,7 @@ func (st *state) ProposeBlock(valKey *bls.ValidatorKey, rewardAddr crypto.Addres // probably the node is shutting down. st.logger.Error("no subsidy transaction") - return nil, errors.Errorf(errors.ErrInvalidBlock, "no subsidy transaction") + return nil, ErrInvalidSubsidyTransaction } txs.Prepend(subsidyTx) prevSeed := st.lastInfo.SortitionSeed() @@ -549,19 +548,30 @@ func (st *state) commitSandbox(sb sandbox.Sandbox, round int16) { func (st *state) validateBlockTime(t time.Time) error { if t.Second()%st.params.BlockIntervalInSecond != 0 { - return errors.Errorf(errors.ErrInvalidBlock, "block time (%s) is not rounded", t.String()) + return InvalidBlockTimeError{ + Reason: fmt.Sprintf("block time (%s) is not rounded", + t.String()), + } } if t.Before(st.lastInfo.BlockTime()) { - return errors.Errorf(errors.ErrInvalidBlock, "block time (%s) is before the last block time", t.String()) + return InvalidBlockTimeError{ + Reason: fmt.Sprintf("block time (%s) is before the last block time (%s)", + t.String(), st.lastInfo.BlockTime()), + } } if t.Equal(st.lastInfo.BlockTime()) { - return errors.Errorf(errors.ErrInvalidBlock, "block time (%s) is same as the last block time", t.String()) + return InvalidBlockTimeError{ + Reason: fmt.Sprintf("block time (%s) is same as the last block time", + t.String()), + } } proposeTime := st.proposeNextBlockTime() threshold := st.params.BlockInterval() if t.After(proposeTime.Add(threshold)) { - return errors.Errorf(errors.ErrInvalidBlock, "block time (%s) is more than threshold (%s)", - t.String(), proposeTime.String()) + return InvalidBlockTimeError{ + Reason: fmt.Sprintf("block time (%s) is more than threshold (%s)", + t.String(), proposeTime.String()), + } } return nil diff --git a/state/validation.go b/state/validation.go index da9addfda..0ba3e6900 100644 --- a/state/validation.go +++ b/state/validation.go @@ -4,30 +4,33 @@ import ( "github.com/pactus-project/pactus/crypto/hash" "github.com/pactus-project/pactus/types/block" "github.com/pactus-project/pactus/types/certificate" - "github.com/pactus-project/pactus/util/errors" ) func (st *state) validateBlock(blk *block.Block, round int16) error { if blk.Header().Version() != st.params.BlockVersion { - return errors.Errorf(errors.ErrInvalidBlock, - "invalid version") + return ErrInvalidBlockVersion } if blk.Header().StateRoot() != st.stateRoot() { - return errors.Errorf(errors.ErrInvalidBlock, - "state root is not same as we expected, expected %v, got %v", st.stateRoot(), blk.Header().StateRoot()) + return InvalidStateRootHashError{ + Expected: st.stateRoot(), + Got: blk.Header().StateRoot(), + } } // Verify proposer proposer := st.committee.Proposer(round) if proposer.Address() != blk.Header().ProposerAddress() { - return errors.Errorf(errors.ErrInvalidBlock, - "invalid proposer, expected %s, got %s", proposer.Address(), blk.Header().ProposerAddress()) + return InvalidProposerError{ + Expected: proposer.Address(), + Got: blk.Header().ProposerAddress(), + } } + // Validate sortition seed seed := blk.Header().SortitionSeed() if !seed.Verify(proposer.PublicKey(), st.lastInfo.SortitionSeed()) { - return errors.Errorf(errors.ErrInvalidBlock, "invalid sortition seed") + return ErrInvalidSortitionSeed } return st.validatePrevCertificate(blk.PrevCertificate(), blk.Header().PrevBlockHash()) @@ -37,16 +40,13 @@ func (st *state) validateBlock(blk *block.Block, round int16) error { func (st *state) validatePrevCertificate(cert *certificate.BlockCertificate, blockHash hash.Hash) error { if cert == nil { if !st.lastInfo.BlockHash().IsUndef() { - return errors.Errorf(errors.ErrInvalidBlock, - "only genesis block has no certificate") + return ErrInvalidCertificate } } else { if cert.Round() != st.lastInfo.Certificate().Round() { // TODO: we should panic here? // It is impossible, unless we have a fork on the latest block - return InvalidBlockCertificateError{ - Cert: cert, - } + return ErrInvalidCertificate } err := cert.Validate(st.lastInfo.Validators(), blockHash) diff --git a/state/validation_test.go b/state/validation_test.go index 14cd3eacf..a6019f296 100644 --- a/state/validation_test.go +++ b/state/validation_test.go @@ -1,13 +1,14 @@ package state import ( + "fmt" "testing" "time" + "github.com/pactus-project/pactus/crypto" "github.com/pactus-project/pactus/sortition" "github.com/pactus-project/pactus/types/block" "github.com/pactus-project/pactus/types/certificate" - "github.com/pactus-project/pactus/util" "github.com/stretchr/testify/assert" ) @@ -28,17 +29,18 @@ func TestBlockValidation(t *testing.T) { blk0.Header().SortitionSeed(), blk0.Header().ProposerAddress()) cert := td.makeCertificateAndSign(t, blk.Hash(), round) - - assert.Error(t, td.state.ValidateBlock(blk, round)) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, ErrInvalidBlockVersion) // Receiving a block with version 2 and rejects it. - // It is possible that the same block would be considered valid by other nodes (Soft fork). - assert.Error(t, td.state.CommitBlock(blk, cert)) + // It is possible that the same block would be considered valid by other nodes (Hard Fork). + err = td.state.CommitBlock(blk, cert) + assert.ErrorIs(t, err, ErrInvalidBlockVersion) }) t.Run("Invalid time", func(t *testing.T) { blk0, _ := td.makeBlockAndCertificate(t, round) - invBlockTime := util.RoundNow(td.state.params.BlockIntervalInSecond).Add(30 * time.Second) + invBlockTime := td.state.LastBlockTime().Add(-10 * time.Second) blk := block.MakeBlock( blk0.Header().Version(), invBlockTime, @@ -49,9 +51,14 @@ func TestBlockValidation(t *testing.T) { blk0.Header().SortitionSeed(), blk0.Header().ProposerAddress()) cert := td.makeCertificateAndSign(t, blk.Hash(), round) - - assert.Error(t, td.state.ValidateBlock(blk, round)) - assert.NoError(t, td.state.CommitBlock(blk, cert)) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, InvalidBlockTimeError{ + Reason: fmt.Sprintf("block time (%s) is before the last block time (%s)", + invBlockTime, td.state.LastBlockTime()), + }) + + err = td.state.CommitBlock(blk, cert) + assert.NoError(t, err) }) t.Run("Invalid StateRoot", func(t *testing.T) { @@ -67,9 +74,17 @@ func TestBlockValidation(t *testing.T) { blk0.Header().SortitionSeed(), blk0.Header().ProposerAddress()) cert := td.makeCertificateAndSign(t, blk.Hash(), round) - - assert.Error(t, td.state.ValidateBlock(blk, round)) - assert.Error(t, td.state.CommitBlock(blk, cert)) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, InvalidStateRootHashError{ + Expected: td.state.stateRoot(), + Got: blk.Header().StateRoot(), + }) + + err = td.state.CommitBlock(blk, cert) + assert.ErrorIs(t, err, InvalidStateRootHashError{ + Expected: td.state.stateRoot(), + Got: blk.Header().StateRoot(), + }) }) t.Run("Invalid PrevCertificate", func(t *testing.T) { @@ -93,9 +108,11 @@ func TestBlockValidation(t *testing.T) { blk0.Header().SortitionSeed(), blk0.Header().ProposerAddress()) cert := td.makeCertificateAndSign(t, blk.Hash(), round) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, crypto.ErrInvalidSignature) - assert.Error(t, td.state.ValidateBlock(blk, round)) - assert.Error(t, td.state.CommitBlock(blk, cert)) + err = td.state.CommitBlock(blk, cert) + assert.ErrorIs(t, err, crypto.ErrInvalidSignature) }) t.Run("Invalid ProposerAddress", func(t *testing.T) { @@ -111,9 +128,17 @@ func TestBlockValidation(t *testing.T) { blk0.Header().SortitionSeed(), invProposerAddress) cert := td.makeCertificateAndSign(t, blk.Hash(), round) - - assert.Error(t, td.state.ValidateBlock(blk, round)) - assert.Error(t, td.state.CommitBlock(blk, cert)) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, InvalidProposerError{ + Expected: td.state.committee.Proposer(round).Address(), + Got: invProposerAddress, + }) + + err = td.state.CommitBlock(blk, cert) + assert.ErrorIs(t, err, InvalidProposerError{ + Expected: td.state.committee.Proposer(round).Address(), + Got: invProposerAddress, + }) }) t.Run("Invalid SortitionSeed", func(t *testing.T) { @@ -129,9 +154,11 @@ func TestBlockValidation(t *testing.T) { invSortitionSeed, blk0.Header().ProposerAddress()) cert := td.makeCertificateAndSign(t, blk.Hash(), round) + err := td.state.ValidateBlock(blk, round) + assert.ErrorIs(t, err, ErrInvalidSortitionSeed) - assert.Error(t, td.state.ValidateBlock(blk, round)) - assert.Error(t, td.state.CommitBlock(blk, cert)) + err = td.state.CommitBlock(blk, cert) + assert.ErrorIs(t, err, ErrInvalidSortitionSeed) }) t.Run("Ok", func(t *testing.T) { diff --git a/types/proposal/errors.go b/types/proposal/errors.go new file mode 100644 index 000000000..3e34f4cd5 --- /dev/null +++ b/types/proposal/errors.go @@ -0,0 +1,15 @@ +package proposal + +import "errors" + +// BasicCheckError is returned when the basic check on the proposal fails. +type BasicCheckError struct { + Reason string +} + +func (e BasicCheckError) Error() string { + return e.Reason +} + +// ErrNoSignature is returned when the proposal has no signature. +var ErrNoSignature = errors.New("no signature") diff --git a/types/proposal/proposal.go b/types/proposal/proposal.go index 2fdd4ac41..d5a459ea9 100644 --- a/types/proposal/proposal.go +++ b/types/proposal/proposal.go @@ -9,7 +9,6 @@ import ( "github.com/pactus-project/pactus/crypto/hash" "github.com/pactus-project/pactus/types/block" "github.com/pactus-project/pactus/util" - "github.com/pactus-project/pactus/util/errors" ) type Proposal struct { @@ -50,19 +49,19 @@ func (p *Proposal) Signature() *bls.Signature { func (p *Proposal) BasicCheck() error { if p.data.Block == nil { - return errors.Errorf(errors.ErrInvalidBlock, "no block") + return BasicCheckError{Reason: "no block"} } if p.data.Signature == nil { - return errors.Errorf(errors.ErrInvalidSignature, "no signature") + return BasicCheckError{Reason: "no signature"} } if err := p.data.Block.BasicCheck(); err != nil { - return err + return BasicCheckError{Reason: fmt.Sprintf("invalid block: %s", err.Error())} } if p.data.Height <= 0 { - return errors.Error(errors.ErrInvalidHeight) + return BasicCheckError{Reason: "invalid height"} } if p.data.Round < 0 { - return errors.Error(errors.ErrInvalidRound) + return BasicCheckError{Reason: "invalid round"} } return nil @@ -86,7 +85,7 @@ func (p *Proposal) UnmarshalCBOR(bs []byte) error { func (p *Proposal) Verify(pubKey crypto.PublicKey) error { if p.data.Signature == nil { - return errors.Errorf(errors.ErrInvalidProposal, "no signature") + return ErrNoSignature } if err := pubKey.VerifyAddress(p.data.Block.Header().ProposerAddress()); err != nil { return err diff --git a/types/proposal/proposal_test.go b/types/proposal/proposal_test.go index 241cd2958..da1390050 100644 --- a/types/proposal/proposal_test.go +++ b/types/proposal/proposal_test.go @@ -61,21 +61,30 @@ func TestBasicCheck(t *testing.T) { t.Run("No block", func(t *testing.T) { p := &proposal.Proposal{} - assert.Error(t, p.BasicCheck()) + err := p.BasicCheck() + assert.ErrorIs(t, err, proposal.BasicCheckError{ + Reason: "no block", + }) }) t.Run("Invalid height", func(t *testing.T) { blk, _ := ts.GenerateTestBlock(ts.RandHeight()) p := proposal.NewProposal(0, 0, blk) p.SetSignature(ts.RandBLSSignature()) - assert.Error(t, p.BasicCheck()) + err := p.BasicCheck() + assert.ErrorIs(t, err, proposal.BasicCheckError{ + Reason: "invalid height", + }) }) t.Run("Invalid round", func(t *testing.T) { blk, _ := ts.GenerateTestBlock(ts.RandHeight()) p := proposal.NewProposal(ts.RandHeight(), -1, blk) p.SetSignature(ts.RandBLSSignature()) - assert.Error(t, p.BasicCheck()) + err := p.BasicCheck() + assert.ErrorIs(t, err, proposal.BasicCheckError{ + Reason: "invalid round", + }) }) t.Run("No signature", func(t *testing.T) { @@ -87,10 +96,15 @@ func TestBasicCheck(t *testing.T) { "48bbbc616c005573d8ad4d5c6997996d6f488946cdd78410f0a400c4a7f9bdb41506bdf717a892fa0004f6") p := &proposal.Proposal{} err := cbor.Unmarshal(d, &p) - assert.NoError(t, err) - assert.Error(t, p.BasicCheck()) - assert.Error(t, p.Verify(pub)) + + err = p.BasicCheck() + assert.ErrorIs(t, err, proposal.BasicCheckError{ + Reason: "no signature", + }) + + err = p.Verify(pub) + assert.Error(t, err, crypto.ErrInvalidSignature) }) t.Run("Ok", func(t *testing.T) { diff --git a/util/errors/errors.go b/util/errors/errors.go index 8efa069db..f67d198e2 100644 --- a/util/errors/errors.go +++ b/util/errors/errors.go @@ -7,16 +7,12 @@ import ( const ( ErrNone = iota ErrGeneric - ErrInvalidBlock - ErrInvalidAmount ErrInvalidAddress ErrInvalidPublicKey ErrInvalidPrivateKey ErrInvalidSignature - ErrInvalidTx ErrInvalidHeight ErrInvalidRound - ErrInvalidProposal ErrInvalidVote ErrInvalidMessage ErrDuplicateVote @@ -27,16 +23,12 @@ const ( var messages = map[int]string{ ErrNone: "no error", ErrGeneric: "generic error", - ErrInvalidBlock: "invalid block", - ErrInvalidAmount: "invalid amount", ErrInvalidAddress: "invalid address", ErrInvalidPublicKey: "invalid public key", ErrInvalidPrivateKey: "invalid private key", ErrInvalidSignature: "invalid signature", - ErrInvalidTx: "invalid transaction", ErrInvalidHeight: "invalid height", ErrInvalidRound: "invalid round", - ErrInvalidProposal: "invalid proposal", ErrInvalidVote: "invalid vote", ErrInvalidMessage: "invalid message", ErrDuplicateVote: "duplicate vote", diff --git a/util/errors/errors_test.go b/util/errors/errors_test.go index 10318c9dc..6e5cca99c 100644 --- a/util/errors/errors_test.go +++ b/util/errors/errors_test.go @@ -8,9 +8,6 @@ import ( ) func TestCode(t *testing.T) { - err1 := Error(ErrInvalidAmount) - assert.Equal(t, ErrInvalidAmount, Code(err1)) - err2 := fmt.Errorf("Nope") assert.Equal(t, ErrGeneric, Code(err2)) @@ -22,15 +19,3 @@ func TestMessages(t *testing.T) { assert.NotEmpty(t, messages[i], "Error code %v", i) } } - -func TestErrorCode(t *testing.T) { - err1 := Error(ErrInvalidAmount) - err2 := Errorf(ErrInvalidTx, "%s", err1.Error()) - err3 := Errorf(ErrInvalidBlock, "%s", err1.Error()) - - assert.Equal(t, ErrInvalidTx, Code(err2)) - assert.Equal(t, ErrInvalidBlock, Code(err3)) - assert.Equal(t, "invalid amount", err1.Error()) - assert.Equal(t, "invalid transaction: invalid amount", err2.Error()) - assert.Equal(t, "invalid block: invalid amount", err3.Error()) -}