From eee457ae49bb9e50404337bafb546ed7edd3e96b Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 16:28:41 +0300 Subject: [PATCH 01/10] drop support for Go 1.12 It's old and not maintained. Use latest Go for builds and test only with 1.13 and 1.14. --- .circleci/config.yml | 23 ++--------------------- .travis.yml | 2 +- README.md | 2 +- go.mod | 2 +- 4 files changed, 5 insertions(+), 24 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a465fd97a..13efe8051 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -3,11 +3,6 @@ orbs: codecov: codecov/codecov@1.0.5 executors: - go1_12: - docker: - - image: circleci/golang:1.12 - environment: - GO111MODULE: "on" go1_13: docker: - image: circleci/golang:1.13 @@ -54,16 +49,6 @@ jobs: name: go-vet command: go vet ./... - test_1_12: - working_directory: /go/src/github.com/nspcc-dev/neo-go - executor: go1_12 - steps: - - checkout - - run: git submodule sync - - run: git submodule update --init - - gomod - - run: go test -v -race ./... - test_1_13: working_directory: /go/src/github.com/nspcc-dev/neo-go executor: go1_13 @@ -100,7 +85,7 @@ jobs: build_cli: working_directory: /go/src/github.com/nspcc-dev/neo-go - executor: go1_12 + executor: go1_14 steps: - checkout - gomod @@ -111,7 +96,7 @@ jobs: build_image: working_directory: /go/src/github.com/nspcc-dev/neo-go - executor: go1_12 + executor: go1_14 docker: - image: golang:1-alpine steps: @@ -142,10 +127,6 @@ workflows: filters: tags: only: v/[0-9]+\.[0-9]+\.[0-9]+/ - - test_1_12: - filters: - tags: - only: v/[0-9]+\.[0-9]+\.[0-9]+/ - test_1_13: filters: tags: diff --git a/.travis.yml b/.travis.yml index 1efe14318..77e2b66aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: go go: - - 1.12.x + - 1.14.x env: - GO111MODULE=on install: diff --git a/README.md b/README.md index 10dcac6ca..563fdd1cf 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Neo 3 code (0.90.0 being compatible with Neo 3 preview2). ## Installation -Go: 1.12+ +Go: 1.13+ Install dependencies. diff --git a/go.mod b/go.mod index 9d982537e..f4a2e11af 100644 --- a/go.mod +++ b/go.mod @@ -28,4 +28,4 @@ require ( gopkg.in/abiosoft/ishell.v2 v2.0.0 ) -go 1.12 +go 1.13 From 6e5920cc09a10f532ccbadc83f074b9491e14ef5 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 16:32:36 +0300 Subject: [PATCH 02/10] use -trimpath build flag for more reproducible builds Fix #349. --- Dockerfile | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 04b9b1cda..7dd749887 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,7 @@ RUN set -x \ && export GO111MODULE=on \ && export CGO_ENABLED=0 \ && export LDFLAGS="-X ${REPO}/pkg/config.Version=${VERSION}" \ - && go build -v -mod=vendor -ldflags "${LDFLAGS}" -o /go/bin/neo-go ./cli + && go build -trimpath -v -mod=vendor -ldflags "${LDFLAGS}" -o /go/bin/neo-go ./cli # Executable image FROM alpine diff --git a/Makefile b/Makefile index 50c944628..e6bcce52f 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ build: deps @set -x \ && export GOGC=off \ && export CGO_ENABLED=0 \ - && go build -v -mod=vendor -ldflags $(BUILD_FLAGS) -o ${BINARY} ./cli/main.go + && go build -trimpath -v -mod=vendor -ldflags $(BUILD_FLAGS) -o ${BINARY} ./cli/main.go neo-go.service: neo-go.service.template @sed -r -e 's_BINDIR_$(BINDIR)_' -e 's_UNITWORKDIR_$(UNITWORKDIR)_' -e 's_SYSCONFIGDIR_$(SYSCONFIGDIR)_' $< >$@ From 5ef08f60ae746a41825c23300448a97d405480ae Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 17:44:08 +0300 Subject: [PATCH 03/10] remove github.com/pkg/errors from dependencies It's not needed any more with Go 1.13 as we have wrapping/unwrapping in base packages. All errors.Wrap calls are replaced with fmt.Errorf, some strings are improved along the way. --- cli/server/server.go | 5 ++-- cli/smartcontract/smart_contract.go | 14 ++++----- go.mod | 1 - pkg/compiler/compiler.go | 5 ++-- pkg/config/config.go | 7 ++--- pkg/consensus/payload.go | 6 ++-- pkg/consensus/recovery_message.go | 3 +- pkg/core/blockchain.go | 45 ++++++++++++++--------------- pkg/core/helper_test.go | 3 +- pkg/core/interop/runtime/util.go | 3 +- pkg/core/interop/runtime/witness.go | 5 ++-- pkg/core/native/native_neo.go | 2 +- pkg/core/native/policy.go | 2 +- pkg/crypto/keys/publickey.go | 4 +-- pkg/encoding/base58/base58.go | 2 +- pkg/network/payload/headers.go | 5 ++-- pkg/rpc/client/client.go | 6 ++-- pkg/rpc/client/policy.go | 2 +- pkg/rpc/client/rpc.go | 16 +++++----- pkg/rpc/client/rpc_test.go | 3 +- pkg/rpc/request/param.go | 2 +- pkg/rpc/request/types.go | 9 +++--- pkg/rpc/response/events.go | 3 +- pkg/rpc/server/server.go | 4 +-- pkg/smartcontract/param_type.go | 5 ++-- pkg/smartcontract/parameter.go | 14 ++++----- pkg/vm/json_test.go | 2 +- pkg/vm/state.go | 3 +- pkg/vm/vm.go | 2 +- pkg/vm/vm_test.go | 2 +- 30 files changed, 90 insertions(+), 95 deletions(-) diff --git a/cli/server/server.go b/cli/server/server.go index 63b494655..ef58f135a 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -15,7 +15,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/network" "github.com/nspcc-dev/neo-go/pkg/network/metrics" "github.com/nspcc-dev/neo-go/pkg/rpc/server" - "github.com/pkg/errors" "github.com/urfave/cli" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -367,13 +366,13 @@ Main: for { select { case err := <-errChan: - shutdownErr = errors.Wrap(err, "Error encountered by server") + shutdownErr = fmt.Errorf("server error: %w", err) cancel() case <-grace.Done(): serv.Shutdown() if serverErr := rpcServer.Shutdown(); serverErr != nil { - shutdownErr = errors.Wrap(serverErr, "Error encountered whilst shutting down server") + shutdownErr = fmt.Errorf("error on shutdown: %w", serverErr) } prometheus.ShutDown() pprof.ShutDown() diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index 56a748ecd..ad590b6f9 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/hex" "encoding/json" + "errors" "fmt" "io/ioutil" "os" @@ -25,7 +26,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/wallet" - "github.com/pkg/errors" "github.com/urfave/cli" "golang.org/x/crypto/ssh/terminal" ) @@ -499,7 +499,7 @@ func testInvokeScript(ctx *cli.Context) error { } nefFile, err := nef.FileFromBytes(b) if err != nil { - return cli.NewExitError(errors.Wrapf(err, "failed to restore .nef file"), 1) + return cli.NewExitError(fmt.Errorf("failed to restore .nef file: %w", err), 1) } args := ctx.Args() @@ -570,12 +570,12 @@ func inspect(ctx *cli.Context) error { if compile { b, err = compiler.Compile(bytes.NewReader(b)) if err != nil { - return cli.NewExitError(errors.Wrap(err, "failed to compile"), 1) + return cli.NewExitError(fmt.Errorf("failed to compile: %w", err), 1) } } else { nefFile, err := nef.FileFromBytes(b) if err != nil { - return cli.NewExitError(errors.Wrapf(err, "failed to restore .nef file"), 1) + return cli.NewExitError(fmt.Errorf("failed to restore .nef file: %w", err), 1) } b = nefFile.Script } @@ -645,17 +645,17 @@ func contractDeploy(ctx *cli.Context) error { } nefFile, err := nef.FileFromBytes(f) if err != nil { - return cli.NewExitError(errors.Wrapf(err, "failed to restore .nef file"), 1) + return cli.NewExitError(fmt.Errorf("failed to restore .nef file: %w", err), 1) } manifestBytes, err := ioutil.ReadFile(manifestFile) if err != nil { - return cli.NewExitError(errors.Wrapf(err, "failed to read manifest file"), 1) + return cli.NewExitError(fmt.Errorf("failed to read manifest file: %w", err), 1) } m := &manifest.Manifest{} err = json.Unmarshal(manifestBytes, m) if err != nil { - return cli.NewExitError(errors.Wrapf(err, "failed to restore manifest file"), 1) + return cli.NewExitError(fmt.Errorf("failed to restore manifest file: %w", err), 1) } gctx, cancel := options.GetTimeoutContext(ctx) diff --git a/go.mod b/go.mod index f4a2e11af..b16c559c9 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,6 @@ require ( github.com/nspcc-dev/dbft v0.0.0-20200711144034-c526ccc6f570 github.com/nspcc-dev/rfc6979 v0.2.0 github.com/pierrec/lz4 v2.5.2+incompatible - github.com/pkg/errors v0.8.1 github.com/prometheus/client_golang v1.2.1 github.com/stretchr/testify v1.4.0 github.com/syndtr/goleveldb v0.0.0-20180307113352-169b1b37be73 diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index aaefbc039..19a00b056 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -15,7 +15,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" - "github.com/pkg/errors" "golang.org/x/tools/go/loader" ) @@ -179,11 +178,11 @@ func CompileAndSave(src string, o *Options) ([]byte, error) { if o.ManifestFile != "" { m, err := di.ConvertToManifest(o.ContractFeatures, o.ContractSupportedStandards...) if err != nil { - return b, errors.Wrap(err, "failed to convert debug info to manifest") + return b, fmt.Errorf("failed to convert debug info to manifest: %w", err) } mData, err := json.Marshal(m) if err != nil { - return b, errors.Wrap(err, "failed to marshal manifest") + return b, fmt.Errorf("failed to marshal manifest to JSON: %w", err) } return b, ioutil.WriteFile(o.ManifestFile, mData, os.ModePerm) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 390704893..7315e0e82 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,7 +7,6 @@ import ( "github.com/go-yaml/yaml" "github.com/nspcc-dev/neo-go/pkg/config/netmode" - "github.com/pkg/errors" ) const userAgentFormat = "/NEO-GO:%s/" @@ -37,12 +36,12 @@ func Load(path string, netMode netmode.Magic) (Config, error) { // LoadFile loads config from the provided path. func LoadFile(configPath string) (Config, error) { if _, err := os.Stat(configPath); os.IsNotExist(err) { - return Config{}, errors.Wrap(err, "Unable to load config") + return Config{}, fmt.Errorf("config '%s' doesn't exist", configPath) } configData, err := ioutil.ReadFile(configPath) if err != nil { - return Config{}, errors.Wrap(err, "Unable to read config") + return Config{}, fmt.Errorf("unable to read config: %w", err) } config := Config{ @@ -54,7 +53,7 @@ func LoadFile(configPath string) (Config, error) { err = yaml.Unmarshal(configData, &config) if err != nil { - return Config{}, errors.Wrap(err, "Problem unmarshaling config json data") + return Config{}, fmt.Errorf("failed to unmarshal config YAML: %w", err) } return config, nil diff --git a/pkg/consensus/payload.go b/pkg/consensus/payload.go index c61ac2799..60af7b20e 100644 --- a/pkg/consensus/payload.go +++ b/pkg/consensus/payload.go @@ -1,6 +1,7 @@ package consensus import ( + "errors" "fmt" "github.com/nspcc-dev/dbft/payload" @@ -14,7 +15,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/emit" - "github.com/pkg/errors" ) type ( @@ -306,7 +306,7 @@ func (m *message) DecodeBinary(r *io.BinReader) { case recoveryMessageType: m.payload = new(recoveryMessage) default: - r.Err = errors.Errorf("invalid type: 0x%02x", byte(m.Type)) + r.Err = fmt.Errorf("invalid type: 0x%02x", byte(m.Type)) return } m.payload.DecodeBinary(r) @@ -338,7 +338,7 @@ func (p *Payload) decodeData() error { br := io.NewBinReaderFromBuf(p.data) m.DecodeBinary(br) if br.Err != nil { - return errors.Wrap(br.Err, "cannot decode data into message") + return fmt.Errorf("can't decode message: %w", br.Err) } p.message = m return nil diff --git a/pkg/consensus/recovery_message.go b/pkg/consensus/recovery_message.go index 817e241e8..1bfc8449a 100644 --- a/pkg/consensus/recovery_message.go +++ b/pkg/consensus/recovery_message.go @@ -1,11 +1,12 @@ package consensus import ( + "errors" + "github.com/nspcc-dev/dbft/crypto" "github.com/nspcc-dev/dbft/payload" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/pkg/errors" ) type ( diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index f95c5448e..6d5347f49 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1,6 +1,7 @@ package core import ( + "errors" "fmt" "math/big" "sort" @@ -27,7 +28,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" "go.uber.org/zap" ) @@ -223,7 +223,7 @@ func (bc *Blockchain) init() error { bc.blockHeight = bHeight bc.persistedHeight = bHeight if err = bc.dao.InitMPT(bHeight); err != nil { - return errors.Wrapf(err, "can't init MPT at height %d", bHeight) + return fmt.Errorf("can't init MPT at height %d: %w", bHeight, err) } hashes, err := bc.dao.GetHeaderHashes() @@ -579,9 +579,9 @@ func (bc *Blockchain) storeBlock(block *block.Block) error { v.LoadScriptWithFlags(bc.contracts.GetPersistScript(), smartcontract.AllowModifyStates|smartcontract.AllowCall) v.SetPriceGetter(getPrice) if err := v.Run(); err != nil { - return errors.Wrap(err, "can't persist native contracts") + return fmt.Errorf("onPersist run failed: %w", err) } else if _, err := systemInterop.DAO.Persist(); err != nil { - return errors.Wrap(err, "can't persist `onPersist` changes") + return fmt.Errorf("can't save onPersist changes: %w", err) } for i := range systemInterop.Notifications { bc.handleNotification(&systemInterop.Notifications[i], cache, block, block.Hash()) @@ -597,7 +597,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error { appExecResults = append(appExecResults, aer) err := cache.PutAppExecResult(aer) if err != nil { - return errors.Wrap(err, "failed to Store notifications") + return fmt.Errorf("failed to store onPersist exec result: %w", err) } } @@ -616,7 +616,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error { if !v.HasFailed() { _, err := systemInterop.DAO.Persist() if err != nil { - return errors.Wrap(err, "failed to persist invocation results") + return fmt.Errorf("failed to persist invocation results: %w", err) } for i := range systemInterop.Notifications { bc.handleNotification(&systemInterop.Notifications[i], cache, block, tx.Hash()) @@ -638,7 +638,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error { appExecResults = append(appExecResults, aer) err = cache.PutAppExecResult(aer) if err != nil { - return errors.Wrap(err, "failed to Store notifications") + return fmt.Errorf("failed to store tx exec result: %w", err) } } @@ -647,7 +647,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error { if block.Index > 0 { prev, err := bc.dao.GetStateRoot(block.Index - 1) if err != nil { - return errors.WithMessagef(err, "can't get previous state root") + return fmt.Errorf("can't get previous state root: %w", err) } prevHash = hash.DoubleSha256(prev.GetSignedPart()) } @@ -1204,7 +1204,7 @@ func (bc *Blockchain) verifyHeader(currHeader, prevHeader *block.Header) error { func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) error { height := bc.BlockHeight() if t.ValidUntilBlock <= height || t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement { - return errors.Errorf("transaction has expired. ValidUntilBlock = %d, current height = %d", t.ValidUntilBlock, height) + return fmt.Errorf("transaction has expired. ValidUntilBlock = %d, current height = %d", t.ValidUntilBlock, height) } hashes, err := bc.GetScriptHashesForVerifying(t) if err != nil { @@ -1219,26 +1219,26 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e return !blockedAccounts[i].Less(h) }) if i != len(blockedAccounts) && blockedAccounts[i].Equals(h) { - return errors.Errorf("policy check failed: account %s is blocked", h.StringLE()) + return fmt.Errorf("policy check failed: account %s is blocked", h.StringLE()) } } maxBlockSystemFee := bc.contracts.Policy.GetMaxBlockSystemFeeInternal(bc.dao) if maxBlockSystemFee < t.SystemFee { - return errors.Errorf("policy check failed: transaction's fee shouldn't exceed maximum block system fee %d", maxBlockSystemFee) + return fmt.Errorf("policy check failed: transaction's fee shouldn't exceed maximum block system fee %d", maxBlockSystemFee) } balance := bc.GetUtilityTokenBalance(t.Sender()) need := t.SystemFee + t.NetworkFee if balance.Cmp(big.NewInt(need)) < 0 { - return errors.Errorf("insufficient funds: balance is %v, need: %v", balance, need) + return fmt.Errorf("insufficient funds: balance is %v, need: %v", balance, need) } size := io.GetVarSize(t) if size > transaction.MaxTransactionSize { - return errors.Errorf("invalid transaction size = %d. It shoud be less then MaxTransactionSize = %d", io.GetVarSize(t), transaction.MaxTransactionSize) + return fmt.Errorf("too big transaction (%d > MaxTransactionSize %d)", size, transaction.MaxTransactionSize) } needNetworkFee := int64(size) * bc.FeePerByte() netFee := t.NetworkFee - needNetworkFee if netFee < 0 { - return errors.Errorf("insufficient funds: net fee is %v, need %v", t.NetworkFee, needNetworkFee) + return fmt.Errorf("insufficient funds: net fee is %v, need %v", t.NetworkFee, needNetworkFee) } if block == nil { if ok := bc.memPool.Verify(t, bc); !ok { @@ -1286,7 +1286,7 @@ func (bc *Blockchain) AddStateRoot(r *state.MPTRoot) error { } } if err := bc.verifyStateRoot(r); err != nil { - return errors.WithMessage(err, "invalid state root") + return fmt.Errorf("invalid state root: %w", err) } if r.Index > bc.BlockHeight() { // just put it into the store for future checks return bc.dao.PutStateRoot(&state.MPTRootState{ @@ -1298,7 +1298,7 @@ func (bc *Blockchain) AddStateRoot(r *state.MPTRoot) error { flag := state.Unverified if r.Witness != nil { if err := bc.verifyStateRootWitness(r); err != nil { - return errors.WithMessage(err, "can't verify signature") + return fmt.Errorf("can't verify signature: %w", err) } flag = state.Verified } @@ -1315,7 +1315,7 @@ func (bc *Blockchain) AddStateRoot(r *state.MPTRoot) error { func (bc *Blockchain) updateStateHeight(newHeight uint32) error { h, err := bc.dao.GetCurrentStateRootHeight() if err != nil { - return errors.WithMessage(err, "can't get current state root height") + return fmt.Errorf("can't get current state root height: %w", err) } else if newHeight == h+1 { updateStateHeightMetric(newHeight) return bc.dao.PutCurrentStateRootHeight(h + 1) @@ -1474,12 +1474,12 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa } err = vm.Run() if vm.HasFailed() { - return errors.Errorf("vm failed to execute the script with error: %s", err) + return fmt.Errorf("vm failed to execute the script with error: %s", err) } resEl := vm.Estack().Pop() if resEl != nil { if !resEl.Bool() { - return errors.Errorf("signature check failed") + return fmt.Errorf("signature check failed") } if useKeys { bc.keyCacheLock.RLock() @@ -1492,7 +1492,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa } } } else { - return errors.Errorf("no result returned from the script") + return fmt.Errorf("no result returned from the script") } return nil } @@ -1511,7 +1511,7 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block witnesses := t.Scripts if len(hashes) != len(witnesses) { - return errors.Errorf("expected len(hashes) == len(witnesses). got: %d != %d", len(hashes), len(witnesses)) + return fmt.Errorf("expected len(hashes) == len(witnesses). got: %d != %d", len(hashes), len(witnesses)) } sort.Slice(hashes, func(i, j int) bool { return hashes[i].Less(hashes[j]) }) sort.Slice(witnesses, func(i, j int) bool { return witnesses[i].ScriptHash().Less(witnesses[j].ScriptHash()) }) @@ -1519,8 +1519,7 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block for i := 0; i < len(hashes); i++ { err := bc.verifyHashAgainstScript(hashes[i], &witnesses[i], interopCtx, false, t.NetworkFee) if err != nil { - numStr := fmt.Sprintf("witness #%d", i) - return errors.Wrap(err, numStr) + return fmt.Errorf("witness #%d: %w", i, err) } } diff --git a/pkg/core/helper_test.go b/pkg/core/helper_test.go index 51c2a8550..08a619565 100644 --- a/pkg/core/helper_test.go +++ b/pkg/core/helper_test.go @@ -26,7 +26,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/wallet" - "github.com/pkg/errors" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -396,7 +395,7 @@ func signTx(bc *Blockchain, txs ...*transaction.Transaction) error { validators := bc.GetStandByValidators() rawScript, err := smartcontract.CreateMultiSigRedeemScript(len(bc.config.StandbyValidators)/2+1, validators) if err != nil { - return errors.Wrap(err, "fail to sign tx") + return fmt.Errorf("failed to sign tx: %w", err) } for _, tx := range txs { size := io.GetVarSize(tx) diff --git a/pkg/core/interop/runtime/util.go b/pkg/core/interop/runtime/util.go index aef0f8ce8..ba806761f 100644 --- a/pkg/core/interop/runtime/util.go +++ b/pkg/core/interop/runtime/util.go @@ -1,12 +1,13 @@ package runtime import ( + "errors" + "github.com/nspcc-dev/neo-go/pkg/core/interop" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" ) // GasLeft returns remaining amount of GAS. diff --git a/pkg/core/interop/runtime/witness.go b/pkg/core/interop/runtime/witness.go index 948aa3a86..413980e0c 100644 --- a/pkg/core/interop/runtime/witness.go +++ b/pkg/core/interop/runtime/witness.go @@ -2,6 +2,8 @@ package runtime import ( "crypto/elliptic" + "errors" + "fmt" "github.com/nspcc-dev/neo-go/pkg/core/dao" "github.com/nspcc-dev/neo-go/pkg/core/interop" @@ -9,7 +11,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" - "github.com/pkg/errors" ) // CheckHashedWitness checks given hash against current list of script hashes @@ -91,7 +92,7 @@ func CheckWitness(ic *interop.Context, v *vm.VM) error { res, err = CheckHashedWitness(ic, hash) } if err != nil { - return errors.Wrap(err, "failed to check") + return fmt.Errorf("failed to check witness: %w", err) } v.Estack().PushVal(res) return nil diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 04beb261e..e02b7005b 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -2,6 +2,7 @@ package native import ( "crypto/elliptic" + "errors" "math/big" "sort" "strings" @@ -18,7 +19,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" ) // NEO represents NEO native contract. diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 2cd090bbe..5e24fc067 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -2,6 +2,7 @@ package native import ( "encoding/binary" + "errors" "math/big" "sort" "sync" @@ -15,7 +16,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" ) const ( diff --git a/pkg/crypto/keys/publickey.go b/pkg/crypto/keys/publickey.go index 003a41d62..358193b61 100644 --- a/pkg/crypto/keys/publickey.go +++ b/pkg/crypto/keys/publickey.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "encoding/hex" "encoding/json" + "errors" "fmt" "math/big" @@ -16,7 +17,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" - "github.com/pkg/errors" ) // coordLen is the number of bytes in serialized X or Y coordinate. @@ -267,7 +267,7 @@ func (p *PublicKey) DecodeBinary(r *io.BinReader) { return } default: - r.Err = errors.Errorf("invalid prefix %d", prefix) + r.Err = fmt.Errorf("invalid prefix %d", prefix) return } if x.Cmp(curveParams.P) >= 0 || y.Cmp(curveParams.P) >= 0 { diff --git a/pkg/encoding/base58/base58.go b/pkg/encoding/base58/base58.go index d14503e9c..3598478ac 100644 --- a/pkg/encoding/base58/base58.go +++ b/pkg/encoding/base58/base58.go @@ -2,10 +2,10 @@ package base58 import ( "bytes" + "errors" "github.com/mr-tron/base58" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" - "github.com/pkg/errors" ) // CheckDecode implements a base58-encoded string decoding with hash-based diff --git a/pkg/network/payload/headers.go b/pkg/network/payload/headers.go index 31550a3df..ba68bbc78 100644 --- a/pkg/network/payload/headers.go +++ b/pkg/network/payload/headers.go @@ -1,10 +1,11 @@ package payload import ( + "fmt" + "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/core/block" "github.com/nspcc-dev/neo-go/pkg/io" - "github.com/pkg/errors" ) // Headers payload. @@ -19,7 +20,7 @@ const ( ) // ErrTooManyHeaders is an error returned when too many headers were received. -var ErrTooManyHeaders = errors.Errorf("too many headers were received (max: %d)", MaxHeadersAllowed) +var ErrTooManyHeaders = fmt.Errorf("too many headers were received (max: %d)", MaxHeadersAllowed) // DecodeBinary implements Serializable interface. func (p *Headers) DecodeBinary(br *io.BinReader) { diff --git a/pkg/rpc/client/client.go b/pkg/rpc/client/client.go index a5110b29c..b7a9a8747 100644 --- a/pkg/rpc/client/client.go +++ b/pkg/rpc/client/client.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "net" "net/http" @@ -15,7 +16,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/rpc/request" "github.com/nspcc-dev/neo-go/pkg/rpc/response" - "github.com/pkg/errors" ) const ( @@ -123,7 +123,7 @@ func (c *Client) SetWIF(wif string) error { defer c.wifMu.Unlock() decodedWif, err := keys.WIFDecode(wif, 0x00) if err != nil { - return errors.Wrap(err, "Failed to decode WIF; failed to add WIF to client ") + return fmt.Errorf("failed to decode WIF: %w", err) } c.wif = decodedWif return nil @@ -176,7 +176,7 @@ func (c *Client) makeHTTPRequest(r *request.Raw) (*response.Raw, error) { if resp.StatusCode != http.StatusOK { err = fmt.Errorf("HTTP %d/%s", resp.StatusCode, http.StatusText(resp.StatusCode)) } else { - err = errors.Wrap(err, "JSON decoding") + err = fmt.Errorf("JSON decoding: %w", err) } } if err != nil { diff --git a/pkg/rpc/client/policy.go b/pkg/rpc/client/policy.go index 558491df5..cb27263a6 100644 --- a/pkg/rpc/client/policy.go +++ b/pkg/rpc/client/policy.go @@ -1,13 +1,13 @@ package client import ( + "errors" "fmt" "github.com/nspcc-dev/neo-go/pkg/core/native" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" ) // PolicyContractHash represents a hash of native Policy contract. diff --git a/pkg/rpc/client/rpc.go b/pkg/rpc/client/rpc.go index d9972d0cc..70161143a 100644 --- a/pkg/rpc/client/rpc.go +++ b/pkg/rpc/client/rpc.go @@ -2,6 +2,7 @@ package client import ( "encoding/hex" + "errors" "fmt" "github.com/nspcc-dev/neo-go/pkg/core" @@ -16,7 +17,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/wallet" - "github.com/pkg/errors" ) // GetApplicationLog returns the contract log based on the specified txid. @@ -428,28 +428,28 @@ func (c *Client) SignAndPushInvocationTx(script []byte, acc *wallet.Account, sys addr, err := address.StringToUint160(acc.Address) if err != nil { - return txHash, errors.Wrap(err, "failed to get address") + return txHash, fmt.Errorf("failed to get address: %w", err) } tx.Signers = getSigners(addr, cosigners) validUntilBlock, err := c.CalculateValidUntilBlock() if err != nil { - return txHash, errors.Wrap(err, "failed to add validUntilBlock to transaction") + return txHash, fmt.Errorf("failed to add validUntilBlock to transaction: %w", err) } tx.ValidUntilBlock = validUntilBlock err = c.AddNetworkFee(tx, int64(netfee), acc) if err != nil { - return txHash, errors.Wrapf(err, "failed to add network fee") + return txHash, fmt.Errorf("failed to add network fee: %w", err) } if err = acc.SignTx(tx); err != nil { - return txHash, errors.Wrap(err, "failed to sign tx") + return txHash, fmt.Errorf("failed to sign tx: %w", err) } txHash = tx.Hash() actualHash, err := c.SendRawTransaction(tx) if err != nil { - return txHash, errors.Wrap(err, "failed sendning tx") + return txHash, fmt.Errorf("failed to send tx: %w", err) } if !actualHash.Equals(txHash) { return actualHash, fmt.Errorf("sent and actual tx hashes mismatch:\n\tsent: %v\n\tactual: %v", txHash.StringLE(), actualHash.StringLE()) @@ -505,7 +505,7 @@ func (c *Client) CalculateValidUntilBlock() (uint32, error) { ) blockCount, err := c.GetBlockCount() if err != nil { - return result, errors.Wrapf(err, "cannot get block count") + return result, fmt.Errorf("can't get block count: %w", err) } if c.cache.calculateValidUntilBlock.expiresAt > blockCount { @@ -513,7 +513,7 @@ func (c *Client) CalculateValidUntilBlock() (uint32, error) { } else { validators, err := c.GetValidators() if err != nil { - return result, errors.Wrapf(err, "cannot get validators") + return result, fmt.Errorf("can't get validators: %w", err) } validatorsCount = uint32(len(validators)) c.cache.calculateValidUntilBlock = calculateValidUntilBlockCache{ diff --git a/pkg/rpc/client/rpc_test.go b/pkg/rpc/client/rpc_test.go index 5da0f90ed..4f83d7922 100644 --- a/pkg/rpc/client/rpc_test.go +++ b/pkg/rpc/client/rpc_test.go @@ -28,7 +28,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -577,7 +576,7 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{ result: func(c *Client) interface{} { addr, err := address.StringToUint160("NMipL5VsNoLUBUJKPKLhxaEbPQVCZnyJyB") if err != nil { - panic(errors.Wrap(err, "failed to parse UnclaimedGas address")) + panic(fmt.Errorf("failed to parse UnclaimedGas address: %w", err)) } return result.UnclaimedGas{ Address: addr, diff --git a/pkg/rpc/request/param.go b/pkg/rpc/request/param.go index 378f79073..2ef95676e 100644 --- a/pkg/rpc/request/param.go +++ b/pkg/rpc/request/param.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" + "errors" "fmt" "strconv" @@ -12,7 +13,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/pkg/errors" ) type ( diff --git a/pkg/rpc/request/types.go b/pkg/rpc/request/types.go index 9020d508e..4da21d45b 100644 --- a/pkg/rpc/request/types.go +++ b/pkg/rpc/request/types.go @@ -2,9 +2,8 @@ package request import ( "encoding/json" + "fmt" "io" - - "github.com/pkg/errors" ) const ( @@ -60,11 +59,11 @@ func (r *In) DecodeData(data io.ReadCloser) error { err := json.NewDecoder(data).Decode(r) if err != nil { - return errors.Errorf("error parsing JSON payload: %s", err) + return fmt.Errorf("error parsing JSON payload: %s", err) } if r.JSONRPC != JSONRPCVersion { - return errors.Errorf("invalid version, expected 2.0 got: '%s'", r.JSONRPC) + return fmt.Errorf("invalid version, expected 2.0 got: '%s'", r.JSONRPC) } return nil @@ -77,7 +76,7 @@ func (r *In) Params() (*Params, error) { err := json.Unmarshal(r.RawParams, ¶ms) if err != nil { - return nil, errors.Errorf("error parsing params field in payload: %s", err) + return nil, fmt.Errorf("error parsing params field in payload: %s", err) } return ¶ms, nil diff --git a/pkg/rpc/response/events.go b/pkg/rpc/response/events.go index 3941fcfb8..32d1e11ee 100644 --- a/pkg/rpc/response/events.go +++ b/pkg/rpc/response/events.go @@ -2,8 +2,7 @@ package response import ( "encoding/json" - - "github.com/pkg/errors" + "errors" ) type ( diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index add8073e5..22f60c3bc 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -4,6 +4,7 @@ import ( "context" "encoding/hex" "encoding/json" + "errors" "fmt" "math" "math/big" @@ -30,7 +31,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/rpc/response/result" "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/pkg/errors" "go.uber.org/zap" ) @@ -692,7 +692,7 @@ func (s *Server) getrawtransaction(reqParams request.Params) (interface{}, *resp if txHash, err := reqParams.Value(0).GetUint256(); err != nil { resultsErr = response.ErrInvalidParams } else if tx, height, err := s.chain.GetTransaction(txHash); err != nil { - err = errors.Wrapf(err, "Invalid transaction hash: %s", txHash) + err = fmt.Errorf("invalid transaction %s: %w", txHash, err) return nil, response.NewRPCError("Unknown transaction", err.Error(), err) } else if reqParams.Value(1).GetBoolean() { _header := s.chain.GetHeaderHash(int(height)) diff --git a/pkg/smartcontract/param_type.go b/pkg/smartcontract/param_type.go index 4d0b45666..e702c8b89 100644 --- a/pkg/smartcontract/param_type.go +++ b/pkg/smartcontract/param_type.go @@ -3,6 +3,8 @@ package smartcontract import ( "encoding/hex" "encoding/json" + "errors" + "fmt" "strconv" "strings" @@ -10,7 +12,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/encoding/address" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/pkg/errors" ) // ParamType represents the Type of the smart contract parameter. @@ -160,7 +161,7 @@ func ParseParamType(typ string) (ParamType, error) { case "any": return AnyType, nil default: - return UnknownType, errors.Errorf("Unknown contract parameter type: %s", typ) + return UnknownType, fmt.Errorf("bad parameter type: %s", typ) } } diff --git a/pkg/smartcontract/parameter.go b/pkg/smartcontract/parameter.go index 56072d16a..116d98eaf 100644 --- a/pkg/smartcontract/parameter.go +++ b/pkg/smartcontract/parameter.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "encoding/hex" "encoding/json" + "errors" "fmt" "math" "math/bits" @@ -14,7 +15,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" - "github.com/pkg/errors" ) // PropertyState represents contract properties (flags). @@ -94,7 +94,7 @@ func (p Parameter) MarshalJSON() ([]byte, error) { case InteropInterfaceType, AnyType: resultRawValue = nil default: - resultErr = errors.Errorf("Marshaller for type %s not implemented", p.Type) + resultErr = fmt.Errorf("can't marshal %s", p.Type) } if resultErr != nil { return nil, resultErr @@ -183,7 +183,7 @@ func (p *Parameter) UnmarshalJSON(data []byte) (err error) { // stub, ignore value, it can only be null p.Value = nil default: - return errors.Errorf("Unmarshaller for type %s not implemented", p.Type) + return fmt.Errorf("can't unmarshal %s", p.Type) } return } @@ -300,7 +300,7 @@ func (p Parameter) TryParse(dest interface{}) error { switch p.Type { case ByteArrayType: if data, ok = p.Value.([]byte); !ok { - return errors.Errorf("failed to cast %s to []byte", p.Value) + return fmt.Errorf("failed to cast %s to []byte", p.Value) } switch dest := dest.(type) { case *util.Uint160: @@ -362,7 +362,7 @@ func (p Parameter) TryParse(dest interface{}) error { *dest = string(data) return nil default: - return errors.Errorf("cannot cast param of type %s to type %s", p.Type, dest) + return fmt.Errorf("cannot cast param of type %s to type %s", p.Type, dest) } default: return errors.New("cannot define param type") @@ -373,7 +373,7 @@ func (p Parameter) TryParse(dest interface{}) error { func bytesToUint64(b []byte, size int) (uint64, error) { var length = size / 8 if len(b) > length { - return 0, errors.Errorf("input doesn't fit into %d bits", size) + return 0, fmt.Errorf("input doesn't fit into %d bits", size) } if len(b) < length { data := make([]byte, length) @@ -412,7 +412,7 @@ func NewParameterFromString(in string) (*Parameter, error) { } // We currently do not support following types: if res.Type == ArrayType || res.Type == MapType || res.Type == InteropInterfaceType || res.Type == VoidType { - return nil, errors.Errorf("Unsupported contract parameter type: %s", res.Type) + return nil, fmt.Errorf("unsupported parameter type %s", res.Type) } buf.Reset() hadType = true diff --git a/pkg/vm/json_test.go b/pkg/vm/json_test.go index 7a86f3a70..0f0057f41 100644 --- a/pkg/vm/json_test.go +++ b/pkg/vm/json_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" + "errors" "fmt" "io/ioutil" "math/big" @@ -19,7 +20,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/smartcontract" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" "github.com/stretchr/testify/require" ) diff --git a/pkg/vm/state.go b/pkg/vm/state.go index 5fe6ee00f..c94de5a22 100644 --- a/pkg/vm/state.go +++ b/pkg/vm/state.go @@ -1,9 +1,8 @@ package vm import ( + "errors" "strings" - - "github.com/pkg/errors" ) // State of the VM. diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index a1cd02358..7d8f10fca 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -4,6 +4,7 @@ import ( "crypto/elliptic" "encoding/binary" "encoding/json" + "errors" "fmt" "io/ioutil" "math" @@ -20,7 +21,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" ) type errorAtInstruct struct { diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 2c872cd94..5cd5e0d18 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -5,6 +5,7 @@ import ( "crypto/elliptic" "encoding/binary" "encoding/hex" + "errors" "fmt" "math/big" "math/rand" @@ -17,7 +18,6 @@ import ( "github.com/nspcc-dev/neo-go/pkg/vm/emit" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) From 205f52c5632c84f2cfe757115eb8c00ed07cf067 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 18:34:44 +0300 Subject: [PATCH 04/10] core: use error wrapping to provide more details --- pkg/core/blockchain.go | 13 ++++++------- pkg/core/native/policy.go | 11 ++++++----- pkg/network/server.go | 8 ++++---- pkg/rpc/server/server.go | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 6d5347f49..3cd33e991 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -420,7 +420,7 @@ func (bc *Blockchain) AddBlock(block *block.Block) error { expectedHeight := bc.BlockHeight() + 1 if expectedHeight != block.Index { - return ErrInvalidBlockIndex + return fmt.Errorf("expected %d, got %d: %w", expectedHeight, block.Index, ErrInvalidBlockIndex) } headerLen := bc.headerListLen() @@ -1367,23 +1367,22 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { defer bc.lock.RUnlock() if bc.HasTransaction(t.Hash()) { - return ErrAlreadyExists + return fmt.Errorf("blockchain: %w", ErrAlreadyExists) } if err := bc.verifyTx(t, nil); err != nil { return err } // Policying. - if ok, err := bc.contracts.Policy.CheckPolicy(bc.newInteropContext(trigger.Application, bc.dao, nil, t), t); err != nil { - return err - } else if !ok { - return ErrPolicy + if err := bc.contracts.Policy.CheckPolicy(bc.newInteropContext(trigger.Application, bc.dao, nil, t), t); err != nil { + // Only one %w can be used. + return fmt.Errorf("%w: %v", ErrPolicy, err) } if err := bc.memPool.Add(t, bc); err != nil { switch err { case mempool.ErrOOM: return ErrOOM case mempool.ErrConflict: - return ErrAlreadyExists + return fmt.Errorf("mempool: %w", ErrAlreadyExists) default: return err } diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 5e24fc067..46abf3b40 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -3,6 +3,7 @@ package native import ( "encoding/binary" "errors" + "fmt" "math/big" "sort" "sync" @@ -510,21 +511,21 @@ func (p *Policy) checkValidators(ic *interop.Context) (bool, error) { // CheckPolicy checks whether transaction's script hashes for verifying are // included into blocked accounts list. -func (p *Policy) CheckPolicy(ic *interop.Context, tx *transaction.Transaction) (bool, error) { +func (p *Policy) CheckPolicy(ic *interop.Context, tx *transaction.Transaction) error { ba, err := p.GetBlockedAccountsInternal(ic.DAO) if err != nil { - return false, err + return fmt.Errorf("unable to get blocked accounts list: %w", err) } scriptHashes, err := ic.Chain.GetScriptHashesForVerifying(tx) if err != nil { - return false, err + return fmt.Errorf("unable to get tx script hashes: %w", err) } for _, acc := range ba { for _, hash := range scriptHashes { if acc.Equals(hash) { - return false, nil + return fmt.Errorf("account %s is blocked", hash.StringLE()) } } } - return true, nil + return nil } diff --git a/pkg/network/server.go b/pkg/network/server.go index 562908eaa..3a926f19c 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -843,12 +843,12 @@ func (s *Server) relayBlocksLoop() { // verifyAndPoolTX verifies the TX and adds it to the local mempool. func (s *Server) verifyAndPoolTX(t *transaction.Transaction) RelayReason { if err := s.chain.PoolTx(t); err != nil { - switch err { - case core.ErrAlreadyExists: + switch { + case errors.Is(err, core.ErrAlreadyExists): return RelayAlreadyExists - case core.ErrOOM: + case errors.Is(err, core.ErrOOM): return RelayOutOfMemory - case core.ErrPolicy: + case errors.Is(err, core.ErrPolicy): return RelayPolicyFail default: return RelayInvalid diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index 22f60c3bc..c64e48e54 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -917,8 +917,8 @@ func (s *Server) submitBlock(reqParams request.Params) (interface{}, *response.E } err = s.chain.AddBlock(b) if err != nil { - switch err { - case core.ErrInvalidBlockIndex, core.ErrAlreadyExists: + switch { + case errors.Is(err, core.ErrInvalidBlockIndex) || errors.Is(err, core.ErrAlreadyExists): return nil, response.ErrAlreadyExists default: return nil, response.ErrValidationFailed From 0e2784cd2cd329dfb6de12c086292cbf03a10a8d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 19:09:57 +0300 Subject: [PATCH 05/10] always wrap errors when creating new ones with fmt.Errorf() It doesn't really change anything in most of the cases, but it's a useful habit anyway. Fix #350. --- cli/server/server.go | 12 ++++++------ cli/smartcontract/smart_contract.go | 20 ++++++++++---------- cli/wallet/multisig.go | 14 +++++++------- cli/wallet/nep5.go | 20 ++++++++++---------- cli/wallet/wallet.go | 12 ++++++------ docs/conventions.md | 23 ++++++++++++++++++++++- pkg/compiler/compiler.go | 6 +++--- pkg/core/blockchain.go | 10 +++++----- pkg/core/interop/crypto/ecdsa.go | 4 ++-- pkg/core/interop_neo.go | 14 +++++++------- pkg/core/native/interop.go | 2 +- pkg/core/native/native_gas.go | 2 +- pkg/core/storage/boltdb_store.go | 2 +- pkg/io/fileWriter.go | 2 +- pkg/network/tcp_peer.go | 2 +- pkg/rpc/client/nep5.go | 12 ++++++------ pkg/rpc/client/rpc_test.go | 4 ++-- pkg/rpc/request/types.go | 4 ++-- pkg/rpc/server/server.go | 2 +- pkg/smartcontract/nef/nef.go | 8 ++++---- pkg/vm/cli/cli.go | 4 ++-- pkg/vm/interop.go | 2 +- pkg/vm/json_test.go | 2 +- scripts/compare-dumps.go | 6 +++--- 24 files changed, 105 insertions(+), 84 deletions(-) diff --git a/cli/server/server.go b/cli/server/server.go index ef58f135a..cc83178f3 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -197,7 +197,7 @@ func dumpDB(ctx *cli.Context) error { bh := chain.GetHeaderHash(int(i)) b, err := chain.GetBlock(bh) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to get block %d: %s", i, err), 1) + return cli.NewExitError(fmt.Errorf("failed to get block %d: %w", i, err), 1) } buf := io.NewBufBinWriter() b.EncodeBinary(buf.BinWriter) @@ -295,7 +295,7 @@ func restoreDB(ctx *cli.Context) error { } else { err = chain.AddBlock(block) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to add block %d: %s", i, err), 1) + return cli.NewExitError(fmt.Errorf("failed to add block %d: %w", i, err), 1) } } if dumpDir != "" { @@ -308,7 +308,7 @@ func restoreDB(ctx *cli.Context) error { lastIndex = block.Index if block.Index%1000 == 0 { if err := dump.tryPersist(dumpDir, block.Index); err != nil { - return cli.NewExitError(fmt.Errorf("can't dump storage to file: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't dump storage to file: %w", err), 1) } } } @@ -349,7 +349,7 @@ func startServer(ctx *cli.Context) error { serv, err := network.NewServer(serverConfig, chain, log) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to create network server: %v", err), 1) + return cli.NewExitError(fmt.Errorf("failed to create network server: %w", err), 1) } rpcServer := server.New(chain, cfg.ApplicationConfiguration.RPC, serv, log) errChan := make(chan error) @@ -410,12 +410,12 @@ func configureAddresses(cfg config.ApplicationConfiguration) { func initBlockChain(cfg config.Config, log *zap.Logger) (*core.Blockchain, error) { store, err := storage.NewStore(cfg.ApplicationConfiguration.DBConfiguration) if err != nil { - return nil, cli.NewExitError(fmt.Errorf("could not initialize storage: %s", err), 1) + return nil, cli.NewExitError(fmt.Errorf("could not initialize storage: %w", err), 1) } chain, err := core.NewBlockchain(store, cfg.ProtocolConfiguration, log) if err != nil { - return nil, cli.NewExitError(fmt.Errorf("could not initialize blockchain: %s", err), 1) + return nil, cli.NewExitError(fmt.Errorf("could not initialize blockchain: %w", err), 1) } return chain, nil } diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index ad590b6f9..a6f62960e 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -411,7 +411,7 @@ func invokeInternal(ctx *cli.Context, signAndPush bool) error { } script, err := util.Uint160DecodeStringLE(args[0]) if err != nil { - return cli.NewExitError(fmt.Errorf("incorrect script hash: %v", err), 1) + return cli.NewExitError(fmt.Errorf("incorrect script hash: %w", err), 1) } if len(args) <= 1 { return cli.NewExitError(errNoMethod, 1) @@ -427,7 +427,7 @@ func invokeInternal(ctx *cli.Context, signAndPush bool) error { } param, err := smartcontract.NewParameterFromString(s) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to parse argument #%d: %v", k+paramsStart+1, err), 1) + return cli.NewExitError(fmt.Errorf("failed to parse argument #%d: %w", k+paramsStart+1, err), 1) } params = append(params, *param) } @@ -437,7 +437,7 @@ func invokeInternal(ctx *cli.Context, signAndPush bool) error { for i, c := range args[cosignersStart:] { cosigner, err := parseCosigner(c) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to parse cosigner #%d: %v", i+1, err), 1) + return cli.NewExitError(fmt.Errorf("failed to parse cosigner #%d: %w", i+1, err), 1) } cosigners = append(cosigners, cosigner) } @@ -468,11 +468,11 @@ func invokeInternal(ctx *cli.Context, signAndPush bool) error { } script, err := hex.DecodeString(resp.Script) if err != nil { - return cli.NewExitError(fmt.Errorf("bad script returned from the RPC node: %v", err), 1) + return cli.NewExitError(fmt.Errorf("bad script returned from the RPC node: %w", err), 1) } txHash, err := c.SignAndPushInvocationTx(script, acc, resp.GasConsumed, gas, cosigners) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to push invocation tx: %v", err), 1) + return cli.NewExitError(fmt.Errorf("failed to push invocation tx: %w", err), 1) } fmt.Printf("Sent invocation transaction %s\n", txHash.StringLE()) } else { @@ -508,7 +508,7 @@ func testInvokeScript(ctx *cli.Context) error { for i, c := range args[:] { cosigner, err := parseCosigner(c) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to parse signer #%d: %v", i+1, err), 1) + return cli.NewExitError(fmt.Errorf("failed to parse signer #%d: %w", i+1, err), 1) } signers = append(signers, cosigner) } @@ -668,17 +668,17 @@ func contractDeploy(ctx *cli.Context) error { txScript, err := request.CreateDeploymentScript(nefFile.Script, m) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to create deployment script: %v", err), 1) + return cli.NewExitError(fmt.Errorf("failed to create deployment script: %w", err), 1) } // It doesn't require any signers. invRes, err := c.InvokeScript(txScript, nil) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to test-invoke deployment script: %v", err), 1) + return cli.NewExitError(fmt.Errorf("failed to test-invoke deployment script: %w", err), 1) } txHash, err := c.SignAndPushInvocationTx(txScript, acc, invRes.GasConsumed, gas, nil) if err != nil { - return cli.NewExitError(fmt.Errorf("failed to push invocation tx: %v", err), 1) + return cli.NewExitError(fmt.Errorf("failed to push invocation tx: %w", err), 1) } fmt.Printf("Sent deployment transaction %s for contract %s\n", txHash.StringLE(), nefFile.Header.ScriptHash.StringLE()) return nil @@ -693,7 +693,7 @@ func parseContractConfig(confFile string) (ProjectConfig, error) { err = yaml.Unmarshal(confBytes, &conf) if err != nil { - return conf, cli.NewExitError(fmt.Errorf("bad config: %v", err), 1) + return conf, cli.NewExitError(fmt.Errorf("bad config: %w", err), 1) } return conf, nil } diff --git a/cli/wallet/multisig.go b/cli/wallet/multisig.go index 3165de97f..0fde124aa 100644 --- a/cli/wallet/multisig.go +++ b/cli/wallet/multisig.go @@ -48,7 +48,7 @@ func signMultisig(ctx *cli.Context) error { addr := ctx.String("addr") sh, err := address.StringToUint160(addr) if err != nil { - return cli.NewExitError(fmt.Errorf("invalid address: %v", err), 1) + return cli.NewExitError(fmt.Errorf("invalid address: %w", err), 1) } acc := wall.GetAccount(sh) if acc == nil { @@ -65,13 +65,13 @@ func signMultisig(ctx *cli.Context) error { if err != nil { return cli.NewExitError(err, 1) } else if err := acc.Decrypt(pass); err != nil { - return cli.NewExitError(fmt.Errorf("can't unlock an account: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't unlock an account: %w", err), 1) } priv := acc.PrivateKey() sign := priv.Sign(tx.GetSignedPart()) if err := c.AddSignature(acc.Contract, priv.PublicKey(), sign); err != nil { - return cli.NewExitError(fmt.Errorf("can't add signature: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) } else if err := writeParameterContext(c, ctx.String("out")); err != nil { return cli.NewExitError(err, 1) } @@ -104,21 +104,21 @@ func signMultisig(ctx *cli.Context) error { func readParameterContext(filename string) (*context.ParameterContext, error) { data, err := ioutil.ReadFile(filename) if err != nil { - return nil, fmt.Errorf("can't read input file: %v", err) + return nil, fmt.Errorf("can't read input file: %w", err) } c := new(context.ParameterContext) if err := json.Unmarshal(data, c); err != nil { - return nil, fmt.Errorf("can't parse transaction: %v", err) + return nil, fmt.Errorf("can't parse transaction: %w", err) } return c, nil } func writeParameterContext(c *context.ParameterContext, filename string) error { if data, err := json.Marshal(c); err != nil { - return fmt.Errorf("can't marshal transaction: %v", err) + return fmt.Errorf("can't marshal transaction: %w", err) } else if err := ioutil.WriteFile(filename, data, 0644); err != nil { - return fmt.Errorf("can't write transaction to file: %v", err) + return fmt.Errorf("can't write transaction to file: %w", err) } return nil } diff --git a/cli/wallet/nep5.go b/cli/wallet/nep5.go index 51fa47600..f41a4ad4a 100644 --- a/cli/wallet/nep5.go +++ b/cli/wallet/nep5.go @@ -144,7 +144,7 @@ func getNEP5Balance(ctx *cli.Context) error { addr := ctx.String("addr") addrHash, err := address.StringToUint160(addr) if err != nil { - return cli.NewExitError(fmt.Errorf("invalid address: %v", err), 1) + return cli.NewExitError(fmt.Errorf("invalid address: %w", err), 1) } acc := wall.GetAccount(addrHash) if acc == nil { @@ -242,7 +242,7 @@ func importNEP5Token(ctx *cli.Context) error { tokenHash, err := util.Uint160DecodeStringLE(ctx.String("token")) if err != nil { - return cli.NewExitError(fmt.Errorf("invalid token contract hash: %v", err), 1) + return cli.NewExitError(fmt.Errorf("invalid token contract hash: %w", err), 1) } for _, t := range wall.Extra.Tokens { @@ -262,7 +262,7 @@ func importNEP5Token(ctx *cli.Context) error { tok, err := c.NEP5TokenInfo(tokenHash) if err != nil { - return cli.NewExitError(fmt.Errorf("can't receive token info: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't receive token info: %w", err), 1) } wall.AddToken(tok) @@ -327,9 +327,9 @@ func removeNEP5Token(ctx *cli.Context) error { } } if err := wall.RemoveToken(token.Hash); err != nil { - return cli.NewExitError(fmt.Errorf("can't remove token: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't remove token: %w", err), 1) } else if err := wall.Save(); err != nil { - return cli.NewExitError(fmt.Errorf("error while saving wallet: %v", err), 1) + return cli.NewExitError(fmt.Errorf("error while saving wallet: %w", err), 1) } return nil } @@ -385,7 +385,7 @@ func multiTransferNEP5(ctx *cli.Context) error { } amount, err := util.FixedNFromString(ss[2], int(token.Decimals)) if err != nil { - return cli.NewExitError(fmt.Errorf("invalid amount: %v", err), 1) + return cli.NewExitError(fmt.Errorf("invalid amount: %w", err), 1) } recepients = append(recepients, client.TransferTarget{ Token: token.Hash, @@ -432,7 +432,7 @@ func transferNEP5(ctx *cli.Context) error { amount, err := util.FixedNFromString(ctx.String("amount"), int(token.Decimals)) if err != nil { - return cli.NewExitError(fmt.Errorf("invalid amount: %v", err), 1) + return cli.NewExitError(fmt.Errorf("invalid amount: %w", err), 1) } return signAndSendTransfer(ctx, c, acc, []client.TransferTarget{{ @@ -464,11 +464,11 @@ func signAndSendTransfer(ctx *cli.Context, c *client.Client, acc *wallet.Account sign := priv.Sign(tx.GetSignedPart()) scCtx := context.NewParameterContext("Neo.Core.ContractTransaction", tx) if err := scCtx.AddSignature(acc.Contract, pub, sign); err != nil { - return cli.NewExitError(fmt.Errorf("can't add signature: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't add signature: %w", err), 1) } else if data, err := json.Marshal(scCtx); err != nil { - return cli.NewExitError(fmt.Errorf("can't marshal tx to JSON: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't marshal tx to JSON: %w", err), 1) } else if err := ioutil.WriteFile(outFile, data, 0644); err != nil { - return cli.NewExitError(fmt.Errorf("can't write tx to file: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't write tx to file: %w", err), 1) } } else { _ = acc.SignTx(tx) diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index 91ddb6847..4c89bd43a 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -262,7 +262,7 @@ func convertWallet(ctx *cli.Context) error { address.Prefix = address.NEO3Prefix newAcc, err := wallet.NewAccountFromWIF(acc.PrivateKey().WIF()) if err != nil { - return cli.NewExitError(fmt.Errorf("can't convert account: %v", err), -1) + return cli.NewExitError(fmt.Errorf("can't convert account: %w", err), -1) } newAcc.Address = address.Uint160ToString(acc.Contract.ScriptHash()) newAcc.Contract = acc.Contract @@ -270,7 +270,7 @@ func convertWallet(ctx *cli.Context) error { newAcc.Label = acc.Label newAcc.Locked = acc.Locked if err := newAcc.Encrypt(pass); err != nil { - return cli.NewExitError(fmt.Errorf("can't encrypt converted account: %v", err), -1) + return cli.NewExitError(fmt.Errorf("can't encrypt converted account: %w", err), -1) } newWallet.AddAccount(newAcc) } @@ -311,7 +311,7 @@ func exportKeys(ctx *cli.Context) error { addr = ctx.Args().First() _, err := address.StringToUint160(addr) if err != nil { - return cli.NewExitError(fmt.Errorf("can't parse address: %v", err), 1) + return cli.NewExitError(fmt.Errorf("can't parse address: %w", err), 1) } } @@ -372,7 +372,7 @@ func importMultisig(ctx *cli.Context) error { for i := range args { pubs[i], err = keys.NewPublicKeyFromString(args[i]) if err != nil { - return cli.NewExitError(fmt.Errorf("can't decode public key %d: %v", i, err), 1) + return cli.NewExitError(fmt.Errorf("can't decode public key %d: %w", i, err), 1) } } @@ -446,9 +446,9 @@ func removeAccount(ctx *cli.Context) error { } if err := wall.RemoveAccount(acc.Address); err != nil { - return cli.NewExitError(fmt.Errorf("error on remove: %v", err), 1) + return cli.NewExitError(fmt.Errorf("error on remove: %w", err), 1) } else if err := wall.Save(); err != nil { - return cli.NewExitError(fmt.Errorf("error while saving wallet: %v", err), 1) + return cli.NewExitError(fmt.Errorf("error while saving wallet: %w", err), 1) } return nil } diff --git a/docs/conventions.md b/docs/conventions.md index e5a6f5eab..364c791d2 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -14,4 +14,25 @@ func example(test int) (num int) { return } -In the above function we have used a named return paramter, which allows you to include a simple return statement without the variables you are returning. This practice can cause confusion when functions become large or the logic becomes complex, so these should be avoided. \ No newline at end of file +In the above function we have used a named return paramter, which allows you to include a simple return statement without the variables you are returning. This practice can cause confusion when functions become large or the logic becomes complex, so these should be avoided. + +## Use error wrapping + +Bad: +``` +err = SomeAPI() +if err != nil { + return fmt.Errorf("something bad happened: %v", err) +} +``` + +Good: +``` +err = SomeAPI() +if err != nil { + return fmt.Errorf("something bad happened: %w", err) +} +``` + +Error wrapping allows `errors.Is` and `errors.As` usage in upper layer +functions which might be useful. diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index 19a00b056..b221b8d74 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -140,15 +140,15 @@ func CompileAndSave(src string, o *Options) ([]byte, error) { } b, di, err := CompileWithDebugInfo(bytes.NewReader(b)) if err != nil { - return nil, fmt.Errorf("error while trying to compile smart contract file: %v", err) + return nil, fmt.Errorf("error while trying to compile smart contract file: %w", err) } f, err := nef.NewFile(b) if err != nil { - return nil, fmt.Errorf("error while trying to create .nef file: %v", err) + return nil, fmt.Errorf("error while trying to create .nef file: %w", err) } bytes, err := f.Bytes() if err != nil { - return nil, fmt.Errorf("error while serializing .nef file: %v", err) + return nil, fmt.Errorf("error while serializing .nef file: %w", err) } out := fmt.Sprintf("%s.%s", o.Outfile, o.Ext) err = ioutil.WriteFile(out, bytes, os.ModePerm) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 3cd33e991..bfbdbba60 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -263,7 +263,7 @@ func (bc *Blockchain) init() error { for hash != targetHash { header, err := bc.GetHeader(hash) if err != nil { - return fmt.Errorf("could not get header %s: %s", hash, err) + return fmt.Errorf("could not get header %s: %w", hash, err) } headers = append(headers, header) hash = header.PrevHash @@ -433,13 +433,13 @@ func (bc *Blockchain) AddBlock(block *block.Block) error { if bc.config.VerifyBlocks { err := block.Verify() if err != nil { - return fmt.Errorf("block %s is invalid: %s", block.Hash().StringLE(), err) + return fmt.Errorf("block %s is invalid: %w", block.Hash().StringLE(), err) } if bc.config.VerifyTransactions { for _, tx := range block.Transactions { err := bc.VerifyTx(tx, block) if err != nil { - return fmt.Errorf("transaction %s failed to verify: %s", tx.Hash().StringLE(), err) + return fmt.Errorf("transaction %s failed to verify: %w", tx.Hash().StringLE(), err) } } } @@ -478,7 +478,7 @@ func (bc *Blockchain) addHeaders(verify bool, headers ...*block.Header) (err err // Verify that the chain of the headers is consistent. var lastHeader *block.Header if lastHeader, err = bc.GetHeader(headers[0].PrevHash); err != nil { - return fmt.Errorf("previous header was not found: %v", err) + return fmt.Errorf("previous header was not found: %w", err) } for _, h := range headers { if err = bc.verifyHeader(h, lastHeader); err != nil { @@ -1473,7 +1473,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa } err = vm.Run() if vm.HasFailed() { - return fmt.Errorf("vm failed to execute the script with error: %s", err) + return fmt.Errorf("vm execution has failed: %w", err) } resEl := vm.Estack().Pop() if resEl != nil { diff --git a/pkg/core/interop/crypto/ecdsa.go b/pkg/core/interop/crypto/ecdsa.go index 67676292c..333cc63ff 100644 --- a/pkg/core/interop/crypto/ecdsa.go +++ b/pkg/core/interop/crypto/ecdsa.go @@ -62,14 +62,14 @@ func ecdsaCheckMultisig(ic *interop.Context, v *vm.VM, curve elliptic.Curve) err hashToCheck := hash.Sha256(msg).BytesBE() pkeys, err := v.Estack().PopSigElements() if err != nil { - return fmt.Errorf("wrong parameters: %s", err.Error()) + return fmt.Errorf("wrong parameters: %w", err) } if !v.AddGas(ECDSAVerifyPrice * int64(len(pkeys))) { return errors.New("gas limit exceeded") } sigs, err := v.Estack().PopSigElements() if err != nil { - return fmt.Errorf("wrong parameters: %s", err.Error()) + return fmt.Errorf("wrong parameters: %w", err) } // It's ok to have more keys than there are signatures (it would // just mean that some keys didn't sign), but not the other way around. diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index 5fa5d3067..cb1588b9d 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -74,7 +74,7 @@ func createContractStateFromVM(ic *interop.Context, v *vm.VM) (*state.Contract, var m manifest.Manifest err := m.UnmarshalJSON(manifestBytes) if err != nil { - return nil, fmt.Errorf("unable to retrieve manifest from stack: %v", err) + return nil, fmt.Errorf("unable to retrieve manifest from stack: %w", err) } return &state.Contract{ Script: script, @@ -105,7 +105,7 @@ func contractCreate(ic *interop.Context, v *vm.VM) error { } cs, err := contractToStackItem(newcontract) if err != nil { - return fmt.Errorf("cannot convert contract to stack item: %v", err) + return fmt.Errorf("cannot convert contract to stack item: %w", err) } v.Estack().PushVal(cs) return nil @@ -149,10 +149,10 @@ func contractUpdate(ic *interop.Context, v *vm.VM) error { } contract.Manifest.ABI.Hash = newHash if err := ic.DAO.PutContractState(contract); err != nil { - return fmt.Errorf("failed to update script: %v", err) + return fmt.Errorf("failed to update script: %w", err) } if err := ic.DAO.DeleteContractState(oldHash); err != nil { - return fmt.Errorf("failed to update script: %v", err) + return fmt.Errorf("failed to update script: %w", err) } } // if manifest was provided, update the old contract manifest and check associated @@ -161,7 +161,7 @@ func contractUpdate(ic *interop.Context, v *vm.VM) error { var newManifest manifest.Manifest err := newManifest.UnmarshalJSON(manifestBytes) if err != nil { - return fmt.Errorf("unable to retrieve manifest from stack: %v", err) + return fmt.Errorf("unable to retrieve manifest from stack: %w", err) } // we don't have to perform `GetContractState` one more time as it's already up-to-date contract.Manifest = newManifest @@ -171,14 +171,14 @@ func contractUpdate(ic *interop.Context, v *vm.VM) error { if !contract.HasStorage() { siMap, err := ic.DAO.GetStorageItems(contract.ID) if err != nil { - return fmt.Errorf("failed to update manifest: %v", err) + return fmt.Errorf("failed to update manifest: %w", err) } if len(siMap) != 0 { return errors.New("old contract shouldn't have storage") } } if err := ic.DAO.PutContractState(contract); err != nil { - return fmt.Errorf("failed to update manifest: %v", err) + return fmt.Errorf("failed to update manifest: %w", err) } } diff --git a/pkg/core/native/interop.go b/pkg/core/native/interop.go index 5e1d23515..cdbed93cc 100644 --- a/pkg/core/native/interop.go +++ b/pkg/core/native/interop.go @@ -27,7 +27,7 @@ func Deploy(ic *interop.Context, _ *vm.VM) error { return err } if err := native.Initialize(ic); err != nil { - return fmt.Errorf("initializing %s native contract: %v", md.Name, err) + return fmt.Errorf("initializing %s native contract: %w", md.Name, err) } } return nil diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index cca7655bd..8c524b77c 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -92,7 +92,7 @@ func (g *GAS) OnPersist(ic *interop.Context) error { } validators, err := g.NEO.getNextBlockValidatorsInternal(ic.Chain, ic.DAO) if err != nil { - return fmt.Errorf("cannot get block validators: %v", err) + return fmt.Errorf("can't get block validators: %w", err) } primary := validators[ic.Block.ConsensusData.PrimaryIndex].GetScriptHash() var netFee int64 diff --git a/pkg/core/storage/boltdb_store.go b/pkg/core/storage/boltdb_store.go index fa906ce06..feb0718ea 100644 --- a/pkg/core/storage/boltdb_store.go +++ b/pkg/core/storage/boltdb_store.go @@ -39,7 +39,7 @@ func NewBoltDBStore(cfg BoltDBOptions) (*BoltDBStore, error) { err = db.Update(func(tx *bbolt.Tx) error { _, err = tx.CreateBucketIfNotExists(Bucket) if err != nil { - return fmt.Errorf("could not create root bucket: %v", err) + return fmt.Errorf("could not create root bucket: %w", err) } return nil }) diff --git a/pkg/io/fileWriter.go b/pkg/io/fileWriter.go index 35d87f34f..35f3e7c62 100644 --- a/pkg/io/fileWriter.go +++ b/pkg/io/fileWriter.go @@ -12,7 +12,7 @@ func MakeDirForFile(filePath string, creator string) error { dir := path.Dir(fileName) err := os.MkdirAll(dir, os.ModePerm) if err != nil { - return fmt.Errorf("could not create dir for %s: %v", creator, err) + return fmt.Errorf("could not create dir for %s: %w", creator, err) } return nil } diff --git a/pkg/network/tcp_peer.go b/pkg/network/tcp_peer.go index 664089bdf..aec25796e 100644 --- a/pkg/network/tcp_peer.go +++ b/pkg/network/tcp_peer.go @@ -161,7 +161,7 @@ func (p *TCPPeer) handleConn() { } if err = p.server.handleMessage(p, msg); err != nil { if p.Handshaked() { - err = fmt.Errorf("handling %s message: %v", msg.Command.String(), err) + err = fmt.Errorf("handling %s message: %w", msg.Command.String(), err) } break } diff --git a/pkg/rpc/client/nep5.go b/pkg/rpc/client/nep5.go index 5c3ea9304..384305fc8 100644 --- a/pkg/rpc/client/nep5.go +++ b/pkg/rpc/client/nep5.go @@ -123,7 +123,7 @@ func (c *Client) CreateNEP5TransferTx(acc *wallet.Account, to util.Uint160, toke func (c *Client) CreateNEP5MultiTransferTx(acc *wallet.Account, gas int64, recepients ...TransferTarget) (*transaction.Transaction, error) { from, err := address.StringToUint160(acc.Address) if err != nil { - return nil, fmt.Errorf("bad account address: %v", err) + return nil, fmt.Errorf("bad account address: %w", err) } w := io.NewBufBinWriter() for i := range recepients { @@ -140,7 +140,7 @@ func (c *Client) CreateNEP5MultiTransferTx(acc *wallet.Account, gas int64, recep }, }) if err != nil { - return nil, fmt.Errorf("can't add system fee to transaction: %v", err) + return nil, fmt.Errorf("can't add system fee to transaction: %w", err) } tx := transaction.New(c.opts.Network, script, result.GasConsumed) tx.Signers = []transaction.Signer{ @@ -151,12 +151,12 @@ func (c *Client) CreateNEP5MultiTransferTx(acc *wallet.Account, gas int64, recep } tx.ValidUntilBlock, err = c.CalculateValidUntilBlock() if err != nil { - return nil, fmt.Errorf("can't calculate validUntilBlock: %v", err) + return nil, fmt.Errorf("can't calculate validUntilBlock: %w", err) } err = c.AddNetworkFee(tx, gas, acc) if err != nil { - return nil, fmt.Errorf("can't add network fee to transaction: %v", err) + return nil, fmt.Errorf("can't add network fee to transaction: %w", err) } return tx, nil @@ -173,7 +173,7 @@ func (c *Client) TransferNEP5(acc *wallet.Account, to util.Uint160, token util.U } if err := acc.SignTx(tx); err != nil { - return util.Uint256{}, fmt.Errorf("can't sign tx: %v", err) + return util.Uint256{}, fmt.Errorf("can't sign tx: %w", err) } return c.SendRawTransaction(tx) @@ -187,7 +187,7 @@ func (c *Client) MultiTransferNEP5(acc *wallet.Account, gas int64, recepients .. } if err := acc.SignTx(tx); err != nil { - return util.Uint256{}, fmt.Errorf("can't sign tx: %v", err) + return util.Uint256{}, fmt.Errorf("can't sign tx: %w", err) } return c.SendRawTransaction(tx) diff --git a/pkg/rpc/client/rpc_test.go b/pkg/rpc/client/rpc_test.go index 4f83d7922..ca1265277 100644 --- a/pkg/rpc/client/rpc_test.go +++ b/pkg/rpc/client/rpc_test.go @@ -690,7 +690,7 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{ result: func(c *Client) interface{} { h, err := util.Uint256DecodeStringLE("72159b0cf1221110daad6e1df6ef4ff03012173b63c86910bd7134deb659c875") if err != nil { - panic(fmt.Errorf("can't decode `sendrawtransaction` result hash: %v", err)) + panic(fmt.Errorf("can't decode `sendrawtransaction` result hash: %w", err)) } return h }, @@ -710,7 +710,7 @@ var rpcClientTestCases = map[string][]rpcClientTestCase{ result: func(c *Client) interface{} { h, err := util.Uint256DecodeStringLE("1bdea8f80eb5bd97fade38d5e7fb93b02c9d3e01394e9f4324218132293f7ea6") if err != nil { - panic(fmt.Errorf("can't decode `submitblock` result hash: %v", err)) + panic(fmt.Errorf("can't decode `submitblock` result hash: %w", err)) } return h }, diff --git a/pkg/rpc/request/types.go b/pkg/rpc/request/types.go index 4da21d45b..6882385f1 100644 --- a/pkg/rpc/request/types.go +++ b/pkg/rpc/request/types.go @@ -59,7 +59,7 @@ func (r *In) DecodeData(data io.ReadCloser) error { err := json.NewDecoder(data).Decode(r) if err != nil { - return fmt.Errorf("error parsing JSON payload: %s", err) + return fmt.Errorf("error parsing JSON payload: %w", err) } if r.JSONRPC != JSONRPCVersion { @@ -76,7 +76,7 @@ func (r *In) Params() (*Params, error) { err := json.Unmarshal(r.RawParams, ¶ms) if err != nil { - return nil, fmt.Errorf("error parsing params field in payload: %s", err) + return nil, fmt.Errorf("error parsing params: %w", err) } return ¶ms, nil diff --git a/pkg/rpc/server/server.go b/pkg/rpc/server/server.go index c64e48e54..d7407aad7 100644 --- a/pkg/rpc/server/server.go +++ b/pkg/rpc/server/server.go @@ -614,7 +614,7 @@ func (s *Server) getDecimals(contractID int32, cache map[int32]decimals) (decima }, }) if err != nil { - return decimals{}, fmt.Errorf("can't create script: %v", err) + return decimals{}, fmt.Errorf("can't create script: %w", err) } res := s.runScriptInVM(script, nil) if res == nil || res.State != "HALT" || len(res.Stack) == 0 { diff --git a/pkg/smartcontract/nef/nef.go b/pkg/smartcontract/nef/nef.go index 7ab79aae9..9725e3df4 100644 --- a/pkg/smartcontract/nef/nef.go +++ b/pkg/smartcontract/nef/nef.go @@ -95,13 +95,13 @@ func GetVersion(version string) (Version, error) { } major, err := strconv.ParseInt(versions[0], 10, 32) if err != nil { - return result, fmt.Errorf("failed to parse major version: %v", err) + return result, fmt.Errorf("failed to parse major version: %w", err) } result.Major = int32(major) minor, err := strconv.ParseInt(versions[1], 10, 32) if err != nil { - return result, fmt.Errorf("failed to parse minor version: %v", err) + return result, fmt.Errorf("failed to parse minor version: %w", err) } result.Minor = int32(minor) @@ -112,7 +112,7 @@ func GetVersion(version string) (Version, error) { } build, err := strconv.ParseInt(b, 10, 32) if err != nil { - return result, fmt.Errorf("failed to parse build version: %v", err) + return result, fmt.Errorf("failed to parse build version: %w", err) } result.Build = int32(build) @@ -120,7 +120,7 @@ func GetVersion(version string) (Version, error) { r := strings.SplitN(versions[3], "-", 2)[0] revision, err := strconv.ParseInt(r, 10, 32) if err != nil { - return result, fmt.Errorf("failed to parse revision version: %v", err) + return result, fmt.Errorf("failed to parse revision version: %w", err) } result.Revision = int32(revision) } diff --git a/pkg/vm/cli/cli.go b/pkg/vm/cli/cli.go index 1ec35e2e0..c2109c6c6 100644 --- a/pkg/vm/cli/cli.go +++ b/pkg/vm/cli/cli.go @@ -241,7 +241,7 @@ func handleBreak(c *ishell.Context) { } n, err := strconv.Atoi(c.Args[0]) if err != nil { - c.Err(fmt.Errorf("argument conversion error: %s", err)) + c.Err(fmt.Errorf("argument conversion error: %w", err)) return } @@ -388,7 +388,7 @@ func handleStep(c *ishell.Context) { if len(c.Args) > 0 { n, err = strconv.Atoi(c.Args[0]) if err != nil { - c.Err(fmt.Errorf("argument conversion error: %s", err)) + c.Err(fmt.Errorf("argument conversion error: %w", err)) return } } diff --git a/pkg/vm/interop.go b/pkg/vm/interop.go index 1d68c46aa..ec1cf0ef3 100644 --- a/pkg/vm/interop.go +++ b/pkg/vm/interop.go @@ -128,7 +128,7 @@ func EnumeratorCreate(v *VM) error { default: data, err := t.TryBytes() if err != nil { - return fmt.Errorf("can not create enumerator from type %s: %v", t.Type(), err) + return fmt.Errorf("can not create enumerator from type %s: %w", t.Type(), err) } interop = &byteArrayWrapper{ index: -1, diff --git a/pkg/vm/json_test.go b/pkg/vm/json_test.go index 0f0057f41..36fb7660a 100644 --- a/pkg/vm/json_test.go +++ b/pkg/vm/json_test.go @@ -437,7 +437,7 @@ func (v *vmUTStackItem) UnmarshalJSON(data []byte) error { var it vmUTStackItem if err := d.Decode(&it); err != nil { - return fmt.Errorf("can't decode map value: %v", err) + return fmt.Errorf("can't decode map value: %w", err) } item := jsonStringToInteger(key) diff --git a/scripts/compare-dumps.go b/scripts/compare-dumps.go index 268806e6d..f3ca4e48c 100644 --- a/scripts/compare-dumps.go +++ b/scripts/compare-dumps.go @@ -55,11 +55,11 @@ func (d dump) normalize() { func compare(a, b string) error { dumpA, err := readFile(a) if err != nil { - return fmt.Errorf("reading file %s: %v", a, err) + return fmt.Errorf("reading file %s: %w", a, err) } dumpB, err := readFile(b) if err != nil { - return fmt.Errorf("reading file %s: %v", b, err) + return fmt.Errorf("reading file %s: %w", b, err) } dumpA.normalize() dumpB.normalize() @@ -143,7 +143,7 @@ func cliMain(c *cli.Context) error { bname := filepath.Join(b, fname) err := compare(aname, bname) if err != nil { - return fmt.Errorf("file %s: %v", fname, err) + return fmt.Errorf("file %s: %w", fname, err) } } } From 4c38fae54c644c3136beec06ec69f0659ff57a82 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 19:41:26 +0300 Subject: [PATCH 06/10] core: fix mempool error check in PoolTx ErrAlreadyExists should be returned for ErrDup, not ErrConflict. --- pkg/core/blockchain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index bfbdbba60..919606e79 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1381,7 +1381,7 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { switch err { case mempool.ErrOOM: return ErrOOM - case mempool.ErrConflict: + case mempool.ErrDup: return fmt.Errorf("mempool: %w", ErrAlreadyExists) default: return err From c19838ea670fa072caee7533cd14f8890a610e87 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 20:13:25 +0300 Subject: [PATCH 07/10] core: use errors.Is in PoolTx Just in case. --- pkg/core/blockchain.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 919606e79..62063c7ce 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1378,10 +1378,10 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { return fmt.Errorf("%w: %v", ErrPolicy, err) } if err := bc.memPool.Add(t, bc); err != nil { - switch err { - case mempool.ErrOOM: + switch { + case errors.Is(err, mempool.ErrOOM): return ErrOOM - case mempool.ErrDup: + case errors.Is(err, mempool.ErrDup): return fmt.Errorf("mempool: %w", ErrAlreadyExists) default: return err From 791c983304d8e66132a747ca5d35c50a6c7eed96 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 21:16:34 +0300 Subject: [PATCH 08/10] core: drop GetScriptHashesForVerifying It no longer depends on blockchain state and there can't ever be an error, in fact we can always iterate over signers, so copying these hashes doesn't make much sense at all as well as sorting arrays in verifyTxWitnesses (witnesses order must match signers order). --- pkg/core/blockchain.go | 41 ++++++--------------------- pkg/core/blockchainer/blockchainer.go | 1 - pkg/core/native/policy.go | 10 ++----- pkg/network/helper_test.go | 3 -- 4 files changed, 11 insertions(+), 44 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 62063c7ce..3b7e34dea 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1206,20 +1206,16 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e if t.ValidUntilBlock <= height || t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement { return fmt.Errorf("transaction has expired. ValidUntilBlock = %d, current height = %d", t.ValidUntilBlock, height) } - hashes, err := bc.GetScriptHashesForVerifying(t) - if err != nil { - return err - } blockedAccounts, err := bc.contracts.Policy.GetBlockedAccountsInternal(bc.dao) if err != nil { return err } - for _, h := range hashes { + for _, signer := range t.Signers { i := sort.Search(len(blockedAccounts), func(i int) bool { - return !blockedAccounts[i].Less(h) + return !blockedAccounts[i].Less(signer.Account) }) - if i != len(blockedAccounts) && blockedAccounts[i].Equals(h) { - return fmt.Errorf("policy check failed: account %s is blocked", h.StringLE()) + if i != len(blockedAccounts) && blockedAccounts[i].Equals(signer.Account) { + return fmt.Errorf("policy check failed: account %s is blocked", signer.Account.StringLE()) } } maxBlockSystemFee := bc.contracts.Policy.GetMaxBlockSystemFeeInternal(bc.dao) @@ -1410,19 +1406,6 @@ func (bc *Blockchain) GetEnrollments() ([]state.Validator, error) { return bc.contracts.NEO.GetRegisteredValidators(bc.dao) } -// GetScriptHashesForVerifying returns all the ScriptHashes of a transaction which will be use -// to verify whether the transaction is bonafide or not. -// Golang implementation of GetScriptHashesForVerifying method in C# (https://github.com/neo-project/neo/blob/master/neo/Network/P2P/Payloads/Transaction.cs#L190) -func (bc *Blockchain) GetScriptHashesForVerifying(t *transaction.Transaction) ([]util.Uint160, error) { - hashesResult := make([]util.Uint160, len(t.Signers)) - for i, s := range t.Signers { - hashesResult[i] = s.Account - } - - return hashesResult, nil - -} - // GetTestVM returns a VM and a Store setup for a test run of some sort of code. func (bc *Blockchain) GetTestVM(tx *transaction.Transaction) *vm.VM { systemInterop := bc.newInteropContext(trigger.Application, bc.dao, nil, tx) @@ -1503,20 +1486,12 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa // not yet added into any block. // Golang implementation of VerifyWitnesses method in C# (https://github.com/neo-project/neo/blob/master/neo/SmartContract/Helper.cs#L87). func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block.Block) error { - hashes, err := bc.GetScriptHashesForVerifying(t) - if err != nil { - return err + if len(t.Signers) != len(t.Scripts) { + return fmt.Errorf("number of signers doesn't match witnesses (%d vs %d)", len(t.Signers), len(t.Scripts)) } - - witnesses := t.Scripts - if len(hashes) != len(witnesses) { - return fmt.Errorf("expected len(hashes) == len(witnesses). got: %d != %d", len(hashes), len(witnesses)) - } - sort.Slice(hashes, func(i, j int) bool { return hashes[i].Less(hashes[j]) }) - sort.Slice(witnesses, func(i, j int) bool { return witnesses[i].ScriptHash().Less(witnesses[j].ScriptHash()) }) interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t) - for i := 0; i < len(hashes); i++ { - err := bc.verifyHashAgainstScript(hashes[i], &witnesses[i], interopCtx, false, t.NetworkFee) + for i := range t.Signers { + err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, false, t.NetworkFee) if err != nil { return fmt.Errorf("witness #%d: %w", i, err) } diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index b1ff94f12..038e7f3d1 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -43,7 +43,6 @@ type Blockchainer interface { GetNEP5Balances(util.Uint160) *state.NEP5Balances GetValidators() ([]*keys.PublicKey, error) GetStandByValidators() keys.PublicKeys - GetScriptHashesForVerifying(*transaction.Transaction) ([]util.Uint160, error) GetStateRoot(height uint32) (*state.MPTRootState, error) GetStorageItem(id int32, key []byte) *state.StorageItem GetStorageItems(id int32) (map[string]*state.StorageItem, error) diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 46abf3b40..254a776b7 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -516,14 +516,10 @@ func (p *Policy) CheckPolicy(ic *interop.Context, tx *transaction.Transaction) e if err != nil { return fmt.Errorf("unable to get blocked accounts list: %w", err) } - scriptHashes, err := ic.Chain.GetScriptHashesForVerifying(tx) - if err != nil { - return fmt.Errorf("unable to get tx script hashes: %w", err) - } for _, acc := range ba { - for _, hash := range scriptHashes { - if acc.Equals(hash) { - return fmt.Errorf("account %s is blocked", hash.StringLE()) + for _, signer := range tx.Signers { + if acc.Equals(signer.Account) { + return fmt.Errorf("account %s is blocked", acc.StringLE()) } } } diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go index 45ccc3615..d4f2669a3 100644 --- a/pkg/network/helper_test.go +++ b/pkg/network/helper_test.go @@ -109,9 +109,6 @@ func (chain testChain) GetStandByValidators() keys.PublicKeys { func (chain testChain) GetEnrollments() ([]state.Validator, error) { panic("TODO") } -func (chain testChain) GetScriptHashesForVerifying(*transaction.Transaction) ([]util.Uint160, error) { - panic("TODO") -} func (chain testChain) GetStateRoot(height uint32) (*state.MPTRootState, error) { panic("TODO") } From 90180c6fb6943357736ee8aeacae4034e1646b11 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 21:39:00 +0300 Subject: [PATCH 09/10] native: pass DAO to CheckPolicy(), it doesn't need interop context Simplify things a bit. --- pkg/core/blockchain.go | 2 +- pkg/core/native/policy.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 3b7e34dea..26d8207b6 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1369,7 +1369,7 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { return err } // Policying. - if err := bc.contracts.Policy.CheckPolicy(bc.newInteropContext(trigger.Application, bc.dao, nil, t), t); err != nil { + if err := bc.contracts.Policy.CheckPolicy(bc.dao, t); err != nil { // Only one %w can be used. return fmt.Errorf("%w: %v", ErrPolicy, err) } diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 254a776b7..874e24293 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -511,8 +511,8 @@ func (p *Policy) checkValidators(ic *interop.Context) (bool, error) { // CheckPolicy checks whether transaction's script hashes for verifying are // included into blocked accounts list. -func (p *Policy) CheckPolicy(ic *interop.Context, tx *transaction.Transaction) error { - ba, err := p.GetBlockedAccountsInternal(ic.DAO) +func (p *Policy) CheckPolicy(d dao.DAO, tx *transaction.Transaction) error { + ba, err := p.GetBlockedAccountsInternal(d) if err != nil { return fmt.Errorf("unable to get blocked accounts list: %w", err) } From d5a9d80c123f4d70ff81c1b2680ddc2fc54f08f2 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 6 Aug 2020 21:49:54 +0300 Subject: [PATCH 10/10] core: refactor out policy check for transaction We were checking blocked accounts twice which is obviously excessive. We also have our accounts sorted, so we can rely on that in CheckPolicy(). It also doesn't make much sense to check MaxBlockSystemFee in Blockchain code, policy contract can handle that. --- pkg/core/blockchain.go | 25 ++++--------------------- pkg/core/native/policy.go | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 26d8207b6..4c9d721c6 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "math/big" - "sort" "sync" "sync/atomic" "time" @@ -1206,21 +1205,10 @@ func (bc *Blockchain) verifyTx(t *transaction.Transaction, block *block.Block) e if t.ValidUntilBlock <= height || t.ValidUntilBlock > height+transaction.MaxValidUntilBlockIncrement { return fmt.Errorf("transaction has expired. ValidUntilBlock = %d, current height = %d", t.ValidUntilBlock, height) } - blockedAccounts, err := bc.contracts.Policy.GetBlockedAccountsInternal(bc.dao) - if err != nil { - return err - } - for _, signer := range t.Signers { - i := sort.Search(len(blockedAccounts), func(i int) bool { - return !blockedAccounts[i].Less(signer.Account) - }) - if i != len(blockedAccounts) && blockedAccounts[i].Equals(signer.Account) { - return fmt.Errorf("policy check failed: account %s is blocked", signer.Account.StringLE()) - } - } - maxBlockSystemFee := bc.contracts.Policy.GetMaxBlockSystemFeeInternal(bc.dao) - if maxBlockSystemFee < t.SystemFee { - return fmt.Errorf("policy check failed: transaction's fee shouldn't exceed maximum block system fee %d", maxBlockSystemFee) + // Policying. + if err := bc.contracts.Policy.CheckPolicy(bc.dao, t); err != nil { + // Only one %w can be used. + return fmt.Errorf("%w: %v", ErrPolicy, err) } balance := bc.GetUtilityTokenBalance(t.Sender()) need := t.SystemFee + t.NetworkFee @@ -1368,11 +1356,6 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error { if err := bc.verifyTx(t, nil); err != nil { return err } - // Policying. - if err := bc.contracts.Policy.CheckPolicy(bc.dao, t); err != nil { - // Only one %w can be used. - return fmt.Errorf("%w: %v", ErrPolicy, err) - } if err := bc.memPool.Add(t, bc); err != nil { switch { case errors.Is(err, mempool.ErrOOM): diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 874e24293..6b661e4c9 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -509,19 +509,27 @@ func (p *Policy) checkValidators(ic *interop.Context) (bool, error) { return runtime.CheckHashedWitness(ic, prevBlock.NextConsensus) } -// CheckPolicy checks whether transaction's script hashes for verifying are -// included into blocked accounts list. +// CheckPolicy checks whether transaction conforms to current policy restrictions +// like not being signed by blocked account or not exceeding block-level system +// fee limit. func (p *Policy) CheckPolicy(d dao.DAO, tx *transaction.Transaction) error { ba, err := p.GetBlockedAccountsInternal(d) if err != nil { return fmt.Errorf("unable to get blocked accounts list: %w", err) } - for _, acc := range ba { + if len(ba) > 0 { for _, signer := range tx.Signers { - if acc.Equals(signer.Account) { - return fmt.Errorf("account %s is blocked", acc.StringLE()) + i := sort.Search(len(ba), func(i int) bool { + return !ba[i].Less(signer.Account) + }) + if i != len(ba) && ba[i].Equals(signer.Account) { + return fmt.Errorf("account %s is blocked", signer.Account.StringLE()) } } } + maxBlockSystemFee := p.GetMaxBlockSystemFeeInternal(d) + if maxBlockSystemFee < tx.SystemFee { + return fmt.Errorf("transaction's fee can't exceed maximum block system fee %d", maxBlockSystemFee) + } return nil }