diff --git a/CHANGELOG.md b/CHANGELOG.md index 792386e5..eb22e8b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,31 @@ # Changelog +## v0.26.1 + +*November 11, 2018* + +Special thanks to external contributors on this release: @katakonst + +Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermint). + +### IMPROVEMENTS: + +- [consensus] [\#2704](https://github.com/tendermint/tendermint/issues/2704) Simplify valid POL round logic +- [docs] [\#2749](https://github.com/tendermint/tendermint/issues/2749) Deduplicate some ABCI docs +- [mempool] More detailed log messages + - [\#2724](https://github.com/tendermint/tendermint/issues/2724) + - [\#2762](https://github.com/tendermint/tendermint/issues/2762) + +### BUG FIXES: + +- [autofile] [\#2703](https://github.com/tendermint/tendermint/issues/2703) Do not panic when checking Head size +- [crypto/merkle] [\#2756](https://github.com/tendermint/tendermint/issues/2756) Fix crypto/merkle ProofOperators.Verify to check bounds on keypath parts. +- [mempool] fix a bug where we create a WAL despite `wal_dir` being empty +- [p2p] [\#2771](https://github.com/tendermint/tendermint/issues/2771) Fix `peer-id` label name to `peer_id` in prometheus metrics +- [p2p] [\#2797](https://github.com/tendermint/tendermint/pull/2797) Fix IDs in peer NodeInfo and require them for addresses + in AddressBook +- [p2p] [\#2797](https://github.com/tendermint/tendermint/pull/2797) Do not close conn immediately after sending pex addrs in seed mode. Partial fix for [\#2092](https://github.com/tendermint/tendermint/issues/2092). + ## v0.26.0 *November 2, 2018* @@ -139,9 +165,7 @@ increasing attention to backwards compatibility. Thanks for bearing with us! - [node] [\#2434](https://github.com/tendermint/tendermint/issues/2434) Make node respond to signal interrupts while sleeping for genesis time - [state] [\#2616](https://github.com/tendermint/tendermint/issues/2616) Pass nil to NewValidatorSet() when genesis file's Validators field is nil - [p2p] [\#2555](https://github.com/tendermint/tendermint/issues/2555) Fix p2p switch FlushThrottle value (@goolAdapter) -- [p2p] [\#2668](https://github.com/tendermint/tendermint/issues/2668) Reconnect to originally dialed address (not self-reported - address) for persistent peers - +- [p2p] [\#2668](https://github.com/tendermint/tendermint/issues/2668) Reconnect to originally dialed address (not self-reported address) for persistent peers ## v0.25.0 @@ -307,8 +331,8 @@ BUG FIXES: *August 22nd, 2018* BUG FIXES: -- [libs/autofile] \#2261 Fix log rotation so it actually happens. - - Fixes issues with consensus WAL growing unbounded ala \#2259 +- [libs/autofile] [\#2261](https://github.com/tendermint/tendermint/issues/2261) Fix log rotation so it actually happens. + - Fixes issues with consensus WAL growing unbounded ala [\#2259](https://github.com/tendermint/tendermint/issues/2259) ## 0.23.0 diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f5e56a12..ea3b9759 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,6 +1,6 @@ # Pending -## v0.26.1 +## v0.26.2 *TBA* @@ -25,3 +25,4 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi ### IMPROVEMENTS: ### BUG FIXES: + diff --git a/abci/README.md b/abci/README.md index 63b43e54..110ad40e 100644 --- a/abci/README.md +++ b/abci/README.md @@ -1,7 +1,5 @@ # Application BlockChain Interface (ABCI) -[![CircleCI](https://circleci.com/gh/tendermint/abci.svg?style=svg)](https://circleci.com/gh/tendermint/abci) - Blockchains are systems for multi-master state machine replication. **ABCI** is an interface that defines the boundary between the replication engine (the blockchain), and the state machine (the application). @@ -12,160 +10,28 @@ Previously, the ABCI was referred to as TMSP. The community has provided a number of addtional implementations, see the [Tendermint Ecosystem](https://tendermint.com/ecosystem) + +## Installation & Usage + +To get up and running quickly, see the [getting started guide](../docs/app-dev/getting-started.md) along with the [abci-cli documentation](../docs/app-dev/abci-cli.md) which will go through the examples found in the [examples](./example/) directory. + ## Specification A detailed description of the ABCI methods and message types is contained in: -- [A prose specification](specification.md) -- [A protobuf file](https://github.com/tendermint/tendermint/blob/master/abci/types/types.proto) -- [A Go interface](https://github.com/tendermint/tendermint/blob/master/abci/types/application.go). - -For more background information on ABCI, motivations, and tendermint, please visit [the documentation](https://tendermint.com/docs/). -The two guides to focus on are the `Application Development Guide` and `Using ABCI-CLI`. - +- [The main spec](../docs/spec/abci/abci.md) +- [A protobuf file](./types/types.proto) +- [A Go interface](./types/application.go) ## Protocol Buffers -To compile the protobuf file, run: +To compile the protobuf file, run (from the root of the repo): ``` -cd $GOPATH/src/github.com/tendermint/tendermint/; make protoc_abci +make protoc_abci ``` See `protoc --help` and [the Protocol Buffers site](https://developers.google.com/protocol-buffers) -for details on compiling for other languages. Note we also include a [GRPC](http://www.grpc.io/docs) +for details on compiling for other languages. Note we also include a [GRPC](https://www.grpc.io/docs) service definition. -## Install ABCI-CLI - -The `abci-cli` is a simple tool for debugging ABCI servers and running some -example apps. To install it: - -``` -mkdir -p $GOPATH/src/github.com/tendermint -cd $GOPATH/src/github.com/tendermint -git clone https://github.com/tendermint/tendermint.git -cd tendermint -make get_tools -make get_vendor_deps -make install_abci -``` - -## Implementation - -We provide three implementations of the ABCI in Go: - -- Golang in-process -- ABCI-socket -- GRPC - -Note the GRPC version is maintained primarily to simplify onboarding and prototyping and is not receiving the same -attention to security and performance as the others - -### In Process - -The simplest implementation just uses function calls within Go. -This means ABCI applications written in Golang can be compiled with TendermintCore and run as a single binary. - -See the [examples](#examples) below for more information. - -### Socket (TSP) - -ABCI is best implemented as a streaming protocol. -The socket implementation provides for asynchronous, ordered message passing over unix or tcp. -Messages are serialized using Protobuf3 and length-prefixed with a [signed Varint](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#signed-integers) - -For example, if the Protobuf3 encoded ABCI message is `0xDEADBEEF` (4 bytes), the length-prefixed message is `0x08DEADBEEF`, since `0x08` is the signed varint -encoding of `4`. If the Protobuf3 encoded ABCI message is 65535 bytes long, the length-prefixed message would be like `0xFEFF07...`. - -Note the benefit of using this `varint` encoding over the old version (where integers were encoded as `` is that -it is the standard way to encode integers in Protobuf. It is also generally shorter. - -### GRPC - -GRPC is an rpc framework native to Protocol Buffers with support in many languages. -Implementing the ABCI using GRPC can allow for faster prototyping, but is expected to be much slower than -the ordered, asynchronous socket protocol. The implementation has also not received as much testing or review. - -Note the length-prefixing used in the socket implementation does not apply for GRPC. - -## Usage - -The `abci-cli` tool wraps an ABCI client and can be used for probing/testing an ABCI server. -For instance, `abci-cli test` will run a test sequence against a listening server running the Counter application (see below). -It can also be used to run some example applications. -See [the documentation](https://tendermint.com/docs/) for more details. - -### Examples - -Check out the variety of example applications in the [example directory](example/). -It also contains the code refered to by the `counter` and `kvstore` apps; these apps come -built into the `abci-cli` binary. - -#### Counter - -The `abci-cli counter` application illustrates nonce checking in transactions. It's code looks like: - -```golang -func cmdCounter(cmd *cobra.Command, args []string) error { - - app := counter.NewCounterApplication(flagSerial) - - logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)) - - // Start the listener - srv, err := server.NewServer(flagAddrC, flagAbci, app) - if err != nil { - return err - } - srv.SetLogger(logger.With("module", "abci-server")) - if err := srv.Start(); err != nil { - return err - } - - // Wait forever - cmn.TrapSignal(func() { - // Cleanup - srv.Stop() - }) - return nil -} -``` - -and can be found in [this file](cmd/abci-cli/abci-cli.go). - -#### kvstore - -The `abci-cli kvstore` application, which illustrates a simple key-value Merkle tree - -```golang -func cmdKVStore(cmd *cobra.Command, args []string) error { - logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout)) - - // Create the application - in memory or persisted to disk - var app types.Application - if flagPersist == "" { - app = kvstore.NewKVStoreApplication() - } else { - app = kvstore.NewPersistentKVStoreApplication(flagPersist) - app.(*kvstore.PersistentKVStoreApplication).SetLogger(logger.With("module", "kvstore")) - } - - // Start the listener - srv, err := server.NewServer(flagAddrD, flagAbci, app) - if err != nil { - return err - } - srv.SetLogger(logger.With("module", "abci-server")) - if err := srv.Start(); err != nil { - return err - } - - // Wait forever - cmn.TrapSignal(func() { - // Cleanup - srv.Stop() - }) - return nil -} -``` diff --git a/config/config.go b/config/config.go index ede57207..fa618211 100644 --- a/config/config.go +++ b/config/config.go @@ -497,6 +497,11 @@ func (cfg *MempoolConfig) WalDir() string { return rootify(cfg.WalPath, cfg.RootDir) } +// WalEnabled returns true if the WAL is enabled. +func (cfg *MempoolConfig) WalEnabled() bool { + return cfg.WalPath != "" +} + // ValidateBasic performs basic validation (checking param bounds, etc.) and // returns an error if any check fails. func (cfg *MempoolConfig) ValidateBasic() error { diff --git a/consensus/reactor.go b/consensus/reactor.go index fc41e573..12e8e0f1 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1017,7 +1017,11 @@ func (ps *PeerState) PickSendVote(votes types.VoteSetReader) bool { if vote, ok := ps.PickVoteToSend(votes); ok { msg := &VoteMessage{vote} ps.logger.Debug("Sending vote message", "ps", ps, "vote", vote) - return ps.peer.Send(VoteChannel, cdc.MustMarshalBinaryBare(msg)) + if ps.peer.Send(VoteChannel, cdc.MustMarshalBinaryBare(msg)) { + ps.SetHasVote(vote) + return true + } + return false } return false } @@ -1046,7 +1050,6 @@ func (ps *PeerState) PickVoteToSend(votes types.VoteSetReader) (vote *types.Vote return nil, false // Not something worth sending } if index, ok := votes.BitArray().Sub(psVotes).PickRandom(); ok { - ps.setHasVote(height, round, type_, index) return votes.GetByIndex(index), true } return nil, false diff --git a/consensus/state.go b/consensus/state.go index 5b144898..8c2e292c 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1408,9 +1408,9 @@ func (cs *ConsensusState) defaultSetProposal(proposal *types.Proposal) error { return nil } - // Verify POLRound, which must be -1 or between 0 and proposal.Round exclusive. - if proposal.POLRound != -1 && - (proposal.POLRound < 0 || proposal.Round <= proposal.POLRound) { + // Verify POLRound, which must be -1 or in range [0, proposal.Round). + if proposal.POLRound < -1 || + (proposal.POLRound >= 0 && proposal.POLRound >= proposal.Round) { return ErrInvalidProposalPOLRound } diff --git a/consensus/wal.go b/consensus/wal.go index 6472c257..bbc9908f 100644 --- a/consensus/wal.go +++ b/consensus/wal.go @@ -13,6 +13,7 @@ import ( amino "github.com/tendermint/go-amino" auto "github.com/tendermint/tendermint/libs/autofile" cmn "github.com/tendermint/tendermint/libs/common" + "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/types" tmtime "github.com/tendermint/tendermint/types/time" ) @@ -95,6 +96,11 @@ func (wal *baseWAL) Group() *auto.Group { return wal.group } +func (wal *baseWAL) SetLogger(l log.Logger) { + wal.BaseService.Logger = l + wal.group.SetLogger(l) +} + func (wal *baseWAL) OnStart() error { size, err := wal.group.Head.Size() if err != nil { diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index 5705c96b..8f8b460c 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -43,6 +43,9 @@ func (poz ProofOperators) Verify(root []byte, keypath string, args [][]byte) (er for i, op := range poz { key := op.GetKey() if len(key) != 0 { + if len(keys) == 0 { + return cmn.NewError("Key path has insufficient # of parts: expected no more keys but got %+v", string(key)) + } lastKey := keys[len(keys)-1] if !bytes.Equal(lastKey, key) { return cmn.NewError("Key mismatch on operation #%d: expected %+v but got %+v", i, string(lastKey), string(key)) diff --git a/crypto/merkle/proof_test.go b/crypto/merkle/proof_test.go index cc208e9a..320b9188 100644 --- a/crypto/merkle/proof_test.go +++ b/crypto/merkle/proof_test.go @@ -107,6 +107,10 @@ func TestProofOperators(t *testing.T) { err = popz.Verify(bz("OUTPUT4"), "//KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")}) assert.NotNil(t, err) + // BAD KEY 5 + err = popz.Verify(bz("OUTPUT4"), "/KEY2/KEY1", [][]byte{bz("INPUT1")}) + assert.NotNil(t, err) + // BAD OUTPUT 1 err = popz.Verify(bz("OUTPUT4_WRONG"), "/KEY4/KEY2/KEY1", [][]byte{bz("INPUT1")}) assert.NotNil(t, err) diff --git a/docs/app-dev/app-development.md b/docs/app-dev/app-development.md index 2618bb1e..d157ce37 100644 --- a/docs/app-dev/app-development.md +++ b/docs/app-dev/app-development.md @@ -47,90 +47,6 @@ The mempool and consensus logic act as clients, and each maintains an open ABCI connection with the application, which hosts an ABCI server. Shown are the request and response types sent on each connection. -## Message Protocol - -The message protocol consists of pairs of requests and responses. Some -messages have no fields, while others may include byte-arrays, strings, -or integers. See the `message Request` and `message Response` -definitions in [the protobuf definition -file](https://github.com/tendermint/tendermint/blob/develop/abci/types/types.proto), -and the [protobuf -documentation](https://developers.google.com/protocol-buffers/docs/overview) -for more details. - -For each request, a server should respond with the corresponding -response, where order of requests is preserved in the order of -responses. - -## Server - -To use ABCI in your programming language of choice, there must be a ABCI -server in that language. Tendermint supports two kinds of implementation -of the server: - -- Asynchronous, raw socket server (Tendermint Socket Protocol, also - known as TSP or Teaspoon) -- GRPC - -Both can be tested using the `abci-cli` by setting the `--abci` flag -appropriately (ie. to `socket` or `grpc`). - -See examples, in various stages of maintenance, in -[Go](https://github.com/tendermint/tendermint/tree/develop/abci/server), -[JavaScript](https://github.com/tendermint/js-abci), -[Python](https://github.com/tendermint/tendermint/tree/develop/abci/example/python3/abci), -[C++](https://github.com/mdyring/cpp-tmsp), and -[Java](https://github.com/jTendermint/jabci). - -### GRPC - -If GRPC is available in your language, this is the easiest approach, -though it will have significant performance overhead. - -To get started with GRPC, copy in the [protobuf -file](https://github.com/tendermint/tendermint/blob/develop/abci/types/types.proto) -and compile it using the GRPC plugin for your language. For instance, -for golang, the command is `protoc --go_out=plugins=grpc:. types.proto`. -See the [grpc documentation for more details](http://www.grpc.io/docs/). -`protoc` will autogenerate all the necessary code for ABCI client and -server in your language, including whatever interface your application -must satisfy to be used by the ABCI server for handling requests. - -### TSP - -If GRPC is not available in your language, or you require higher -performance, or otherwise enjoy programming, you may implement your own -ABCI server using the Tendermint Socket Protocol, known affectionately -as Teaspoon. The first step is still to auto-generate the relevant data -types and codec in your language using `protoc`. Messages coming over -the socket are proto3 encoded, but additionally length-prefixed to -facilitate use as a streaming protocol. proto3 doesn't have an -official length-prefix standard, so we use our own. The first byte in -the prefix represents the length of the Big Endian encoded length. The -remaining bytes in the prefix are the Big Endian encoded length. - -For example, if the proto3 encoded ABCI message is 0xDEADBEEF (4 -bytes), the length-prefixed message is 0x0104DEADBEEF. If the proto3 -encoded ABCI message is 65535 bytes long, the length-prefixed message -would be like 0x02FFFF.... - -Note this prefixing does not apply for grpc. - -An ABCI server must also be able to support multiple connections, as -Tendermint uses three connections. - -## Client - -There are currently two use-cases for an ABCI client. One is a testing -tool, as in the `abci-cli`, which allows ABCI requests to be sent via -command line. The other is a consensus engine, such as Tendermint Core, -which makes requests to the application every time a new transaction is -received or a block is committed. - -It is unlikely that you will need to implement a client. For details of -our client, see -[here](https://github.com/tendermint/tendermint/tree/develop/abci/client). - Most of the examples below are from [kvstore application](https://github.com/tendermint/tendermint/blob/develop/abci/example/kvstore/kvstore.go), which is a part of the abci repo. [persistent_kvstore diff --git a/docs/spec/abci/client-server.md b/docs/spec/abci/client-server.md index 822bfd1f..5ac7b3eb 100644 --- a/docs/spec/abci/client-server.md +++ b/docs/spec/abci/client-server.md @@ -3,12 +3,8 @@ This section is for those looking to implement their own ABCI Server, perhaps in a new programming language. -You are expected to have read [ABCI Methods and Types](abci.md) and [ABCI -Applications](apps.md). - -See additional details in the [ABCI -readme](https://github.com/tendermint/tendermint/blob/develop/abci/README.md)(TODO: deduplicate -those details). +You are expected to have read [ABCI Methods and Types](./abci.md) and [ABCI +Applications](./apps.md). ## Message Protocol @@ -24,17 +20,16 @@ For each request, a server should respond with the corresponding response, where the order of requests is preserved in the order of responses. -## Server +## Server Implementations To use ABCI in your programming language of choice, there must be a ABCI -server in that language. Tendermint supports two kinds of implementation -of the server: +server in that language. Tendermint supports three implementations of the ABCI, written in Go: -- Asynchronous, raw socket server (Tendermint Socket Protocol, also - known as TSP or Teaspoon) +- In-process (Golang only) +- ABCI-socket - GRPC -Both can be tested using the `abci-cli` by setting the `--abci` flag +The latter two can be tested using the `abci-cli` by setting the `--abci` flag appropriately (ie. to `socket` or `grpc`). See examples, in various stages of maintenance, in @@ -44,6 +39,12 @@ See examples, in various stages of maintenance, in [C++](https://github.com/mdyring/cpp-tmsp), and [Java](https://github.com/jTendermint/jabci). +### In Process + +The simplest implementation uses function calls within Golang. +This means ABCI applications written in Golang can be compiled with TendermintCore and run as a single binary. + + ### GRPC If GRPC is available in your language, this is the easiest approach, @@ -58,15 +59,18 @@ See the [grpc documentation for more details](http://www.grpc.io/docs/). server in your language, including whatever interface your application must satisfy to be used by the ABCI server for handling requests. +Note the length-prefixing used in the socket implementation (TSP) does not apply for GRPC. + ### TSP +Tendermint Socket Protocol is an asynchronous, raw socket server which provides ordered message passing over unix or tcp. +Messages are serialized using Protobuf3 and length-prefixed with a [signed Varint](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#signed-integers) + If GRPC is not available in your language, or you require higher performance, or otherwise enjoy programming, you may implement your own -ABCI server using the Tendermint Socket Protocol, known affectionately -as Teaspoon. The first step is still to auto-generate the relevant data -types and codec in your language using `protoc`. Messages coming over -the socket are proto3 encoded, but additionally length-prefixed to -facilitate use as a streaming protocol. proto3 doesn't have an +ABCI server using the Tendermint Socket Protocol. The first step is still to auto-generate the relevant data +types and codec in your language using `protoc`. In addition to being proto3 encoded, messages coming over +the socket are length-prefixed to facilitate use as a streaming protocol. proto3 doesn't have an official length-prefix standard, so we use our own. The first byte in the prefix represents the length of the Big Endian encoded length. The remaining bytes in the prefix are the Big Endian encoded length. @@ -76,12 +80,14 @@ bytes), the length-prefixed message is 0x0104DEADBEEF. If the proto3 encoded ABCI message is 65535 bytes long, the length-prefixed message would be like 0x02FFFF.... -Note this prefixing does not apply for grpc. +The benefit of using this `varint` encoding over the old version (where integers were encoded as `` is that +it is the standard way to encode integers in Protobuf. It is also generally shorter. + +As noted above, this prefixing does not apply for GRPC. An ABCI server must also be able to support multiple connections, as Tendermint uses three connections. - ### Async vs Sync The main ABCI server (ie. non-GRPC) provides ordered asynchronous messages. diff --git a/evidence/reactor_test.go b/evidence/reactor_test.go index ea9657d2..69dcdec5 100644 --- a/evidence/reactor_test.go +++ b/evidence/reactor_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" cfg "github.com/tendermint/tendermint/config" + "github.com/tendermint/tendermint/crypto/secp256k1" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/p2p" @@ -178,3 +179,30 @@ func TestReactorSelectiveBroadcast(t *testing.T) { peers := reactors[1].Switch.Peers().List() assert.Equal(t, 1, len(peers)) } +func TestEvidenceListMessageValidationBasic(t *testing.T) { + + testCases := []struct { + testName string + malleateEvListMsg func(*EvidenceListMessage) + expectErr bool + }{ + {"Good EvidenceListMessage", func(evList *EvidenceListMessage) {}, false}, + {"Invalid EvidenceListMessage", func(evList *EvidenceListMessage) { + evList.Evidence = append(evList.Evidence, + &types.DuplicateVoteEvidence{PubKey: secp256k1.GenPrivKey().PubKey()}) + }, true}, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + evListMsg := &EvidenceListMessage{} + n := 3 + valAddr := []byte("myval") + evListMsg.Evidence = make([]types.Evidence, n) + for i := 0; i < n; i++ { + evListMsg.Evidence[i] = types.NewMockGoodEvidence(int64(i+1), 0, valAddr) + } + tc.malleateEvListMsg(evListMsg) + assert.Equal(t, tc.expectErr, evListMsg.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/libs/autofile/autofile.go b/libs/autofile/autofile.go index 6822545e..a1e2f49e 100644 --- a/libs/autofile/autofile.go +++ b/libs/autofile/autofile.go @@ -83,6 +83,8 @@ func OpenAutoFile(path string) (*AutoFile, error) { return af, nil } +// Close shuts down the closing goroutine, SIGHUP handler and closes the +// AutoFile. func (af *AutoFile) Close() error { af.closeTicker.Stop() close(af.closeTickerStopc) @@ -116,6 +118,10 @@ func (af *AutoFile) closeFile() (err error) { return file.Close() } +// Write writes len(b) bytes to the AutoFile. It returns the number of bytes +// written and an error, if any. Write returns a non-nil error when n != +// len(b). +// Opens AutoFile if needed. func (af *AutoFile) Write(b []byte) (n int, err error) { af.mtx.Lock() defer af.mtx.Unlock() @@ -130,6 +136,10 @@ func (af *AutoFile) Write(b []byte) (n int, err error) { return } +// Sync commits the current contents of the file to stable storage. Typically, +// this means flushing the file system's in-memory copy of recently written +// data to disk. +// Opens AutoFile if needed. func (af *AutoFile) Sync() error { af.mtx.Lock() defer af.mtx.Unlock() @@ -158,23 +168,22 @@ func (af *AutoFile) openFile() error { return nil } +// Size returns the size of the AutoFile. It returns -1 and an error if fails +// get stats or open file. +// Opens AutoFile if needed. func (af *AutoFile) Size() (int64, error) { af.mtx.Lock() defer af.mtx.Unlock() if af.file == nil { - err := af.openFile() - if err != nil { - if err == os.ErrNotExist { - return 0, nil - } + if err := af.openFile(); err != nil { return -1, err } } + stat, err := af.file.Stat() if err != nil { return -1, err } return stat.Size(), nil - } diff --git a/libs/autofile/autofile_test.go b/libs/autofile/autofile_test.go index 5408d820..0b3521c2 100644 --- a/libs/autofile/autofile_test.go +++ b/libs/autofile/autofile_test.go @@ -84,3 +84,40 @@ func TestOpenAutoFilePerms(t *testing.T) { t.Errorf("unexpected error %v", e) } } + +func TestAutoFileSize(t *testing.T) { + // First, create an AutoFile writing to a tempfile dir + f, err := ioutil.TempFile("", "sighup_test") + require.NoError(t, err) + err = f.Close() + require.NoError(t, err) + + // Here is the actual AutoFile. + af, err := OpenAutoFile(f.Name()) + require.NoError(t, err) + + // 1. Empty file + size, err := af.Size() + require.Zero(t, size) + require.NoError(t, err) + + // 2. Not empty file + data := []byte("Maniac\n") + _, err = af.Write(data) + require.NoError(t, err) + size, err = af.Size() + require.EqualValues(t, len(data), size) + require.NoError(t, err) + + // 3. Not existing file + err = af.Close() + require.NoError(t, err) + err = os.Remove(f.Name()) + require.NoError(t, err) + size, err = af.Size() + require.EqualValues(t, 0, size, "Expected a new file to be empty") + require.NoError(t, err) + + // Cleanup + _ = os.Remove(f.Name()) +} diff --git a/libs/autofile/group.go b/libs/autofile/group.go index ea272b61..1ec6b240 100644 --- a/libs/autofile/group.go +++ b/libs/autofile/group.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "log" "os" "path" "path/filepath" @@ -231,7 +230,8 @@ func (g *Group) checkHeadSizeLimit() { } size, err := g.Head.Size() if err != nil { - panic(err) + g.Logger.Error("Group's head may grow without bound", "head", g.Head.Path, "err", err) + return } if size >= limit { g.RotateFile() @@ -253,21 +253,21 @@ func (g *Group) checkTotalSizeLimit() { } if index == gInfo.MaxIndex { // Special degenerate case, just do nothing. - log.Println("WARNING: Group's head " + g.Head.Path + "may grow without bound") + g.Logger.Error("Group's head may grow without bound", "head", g.Head.Path) return } pathToRemove := filePathForIndex(g.Head.Path, index, gInfo.MaxIndex) - fileInfo, err := os.Stat(pathToRemove) + fInfo, err := os.Stat(pathToRemove) if err != nil { - log.Println("WARNING: Failed to fetch info for file @" + pathToRemove) + g.Logger.Error("Failed to fetch info for file", "file", pathToRemove) continue } err = os.Remove(pathToRemove) if err != nil { - log.Println(err) + g.Logger.Error("Failed to remove path", "path", pathToRemove) return } - totalSize -= fileInfo.Size() + totalSize -= fInfo.Size() } } diff --git a/mempool/mempool.go b/mempool/mempool.go index 65cd5535..b84eb4a6 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -11,7 +11,6 @@ import ( "github.com/pkg/errors" - amino "github.com/tendermint/go-amino" abci "github.com/tendermint/tendermint/abci/types" cfg "github.com/tendermint/tendermint/config" auto "github.com/tendermint/tendermint/libs/autofile" @@ -25,12 +24,12 @@ import ( // PreCheckFunc is an optional filter executed before CheckTx and rejects // transaction if false is returned. An example would be to ensure that a // transaction doesn't exceeded the block size. -type PreCheckFunc func(types.Tx) bool +type PreCheckFunc func(types.Tx) error // PostCheckFunc is an optional filter executed after CheckTx and rejects // transaction if false is returned. An example would be to ensure a // transaction doesn't require more gas than available for the block. -type PostCheckFunc func(types.Tx, *abci.ResponseCheckTx) bool +type PostCheckFunc func(types.Tx, *abci.ResponseCheckTx) error /* @@ -68,24 +67,52 @@ var ( ErrMempoolIsFull = errors.New("Mempool is full") ) +// ErrPreCheck is returned when tx is too big +type ErrPreCheck struct { + Reason error +} + +func (e ErrPreCheck) Error() string { + return e.Reason.Error() +} + +// IsPreCheckError returns true if err is due to pre check failure. +func IsPreCheckError(err error) bool { + _, ok := err.(ErrPreCheck) + return ok +} + // PreCheckAminoMaxBytes checks that the size of the transaction plus the amino // overhead is smaller or equal to the expected maxBytes. func PreCheckAminoMaxBytes(maxBytes int64) PreCheckFunc { - return func(tx types.Tx) bool { + return func(tx types.Tx) error { // We have to account for the amino overhead in the tx size as well - aminoOverhead := amino.UvarintSize(uint64(len(tx))) - return int64(len(tx)+aminoOverhead) <= maxBytes + // NOTE: fieldNum = 1 as types.Block.Data contains Txs []Tx as first field. + // If this field order ever changes this needs to updated here accordingly. + // NOTE: if some []Tx are encoded without a parenting struct, the + // fieldNum is also equal to 1. + aminoOverhead := types.ComputeAminoOverhead(tx, 1) + txSize := int64(len(tx)) + aminoOverhead + if txSize > maxBytes { + return fmt.Errorf("Tx size (including amino overhead) is too big: %d, max: %d", + txSize, maxBytes) + } + return nil } } // PostCheckMaxGas checks that the wanted gas is smaller or equal to the passed -// maxGas. Returns true if maxGas is -1. +// maxGas. Returns nil if maxGas is -1. func PostCheckMaxGas(maxGas int64) PostCheckFunc { - return func(tx types.Tx, res *abci.ResponseCheckTx) bool { + return func(tx types.Tx, res *abci.ResponseCheckTx) error { if maxGas == -1 { - return true + return nil } - return res.GasWanted <= maxGas + if res.GasWanted > maxGas { + return fmt.Errorf("gas wanted %d is greater than max gas %d", + res.GasWanted, maxGas) + } + return nil } } @@ -189,39 +216,33 @@ func WithMetrics(metrics *Metrics) MempoolOption { return func(mem *Mempool) { mem.metrics = metrics } } +// InitWAL creates a directory for the WAL file and opens a file itself. +// +// *panics* if can't create directory or open file. +// *not thread safe* +func (mem *Mempool) InitWAL() { + walDir := mem.config.WalDir() + err := cmn.EnsureDir(walDir, 0700) + if err != nil { + panic(errors.Wrap(err, "Error ensuring Mempool WAL dir")) + } + af, err := auto.OpenAutoFile(walDir + "/wal") + if err != nil { + panic(errors.Wrap(err, "Error opening Mempool WAL file")) + } + mem.wal = af +} + // CloseWAL closes and discards the underlying WAL file. // Any further writes will not be relayed to disk. -func (mem *Mempool) CloseWAL() bool { - if mem == nil { - return false - } - +func (mem *Mempool) CloseWAL() { mem.proxyMtx.Lock() defer mem.proxyMtx.Unlock() - if mem.wal == nil { - return false - } - if err := mem.wal.Close(); err != nil && mem.logger != nil { - mem.logger.Error("Mempool.CloseWAL", "err", err) + if err := mem.wal.Close(); err != nil { + mem.logger.Error("Error closing WAL", "err", err) } mem.wal = nil - return true -} - -func (mem *Mempool) InitWAL() { - walDir := mem.config.WalDir() - if walDir != "" { - err := cmn.EnsureDir(walDir, 0700) - if err != nil { - cmn.PanicSanity(errors.Wrap(err, "Error ensuring Mempool wal dir")) - } - af, err := auto.OpenAutoFile(walDir + "/wal") - if err != nil { - cmn.PanicSanity(errors.Wrap(err, "Error opening Mempool wal file")) - } - mem.wal = af - } } // Lock locks the mempool. The consensus must be able to hold lock to safely update. @@ -285,8 +306,10 @@ func (mem *Mempool) CheckTx(tx types.Tx, cb func(*abci.Response)) (err error) { return ErrMempoolIsFull } - if mem.preCheck != nil && !mem.preCheck(tx) { - return + if mem.preCheck != nil { + if err := mem.preCheck(tx); err != nil { + return ErrPreCheck{err} + } } // CACHE @@ -336,8 +359,11 @@ func (mem *Mempool) resCbNormal(req *abci.Request, res *abci.Response) { switch r := res.Value.(type) { case *abci.Response_CheckTx: tx := req.GetCheckTx().Tx - if (r.CheckTx.Code == abci.CodeTypeOK) && - mem.isPostCheckPass(tx, r.CheckTx) { + var postCheckErr error + if mem.postCheck != nil { + postCheckErr = mem.postCheck(tx, r.CheckTx) + } + if (r.CheckTx.Code == abci.CodeTypeOK) && postCheckErr == nil { mem.counter++ memTx := &mempoolTx{ counter: mem.counter, @@ -346,12 +372,18 @@ func (mem *Mempool) resCbNormal(req *abci.Request, res *abci.Response) { tx: tx, } mem.txs.PushBack(memTx) - mem.logger.Info("Added good transaction", "tx", TxID(tx), "res", r, "total", mem.Size()) + mem.logger.Info("Added good transaction", + "tx", TxID(tx), + "res", r, + "height", memTx.height, + "total", mem.Size(), + "counter", memTx.counter, + ) mem.metrics.TxSizeBytes.Observe(float64(len(tx))) mem.notifyTxsAvailable() } else { // ignore bad transaction - mem.logger.Info("Rejected bad transaction", "tx", TxID(tx), "res", r) + mem.logger.Info("Rejected bad transaction", "tx", TxID(tx), "res", r, "err", postCheckErr) mem.metrics.FailedTxs.Add(1) // remove from cache (it might be good later) mem.cache.Remove(tx) @@ -364,6 +396,7 @@ func (mem *Mempool) resCbNormal(req *abci.Request, res *abci.Response) { func (mem *Mempool) resCbRecheck(req *abci.Request, res *abci.Response) { switch r := res.Value.(type) { case *abci.Response_CheckTx: + tx := req.GetCheckTx().Tx memTx := mem.recheckCursor.Value.(*mempoolTx) if !bytes.Equal(req.GetCheckTx().Tx, memTx.tx) { cmn.PanicSanity( @@ -374,15 +407,20 @@ func (mem *Mempool) resCbRecheck(req *abci.Request, res *abci.Response) { ), ) } - if (r.CheckTx.Code == abci.CodeTypeOK) && mem.isPostCheckPass(memTx.tx, r.CheckTx) { + var postCheckErr error + if mem.postCheck != nil { + postCheckErr = mem.postCheck(tx, r.CheckTx) + } + if (r.CheckTx.Code == abci.CodeTypeOK) && postCheckErr == nil { // Good, nothing to do. } else { // Tx became invalidated due to newly committed block. + mem.logger.Info("Tx is no longer valid", "tx", TxID(tx), "res", r, "err", postCheckErr) mem.txs.Remove(mem.recheckCursor) mem.recheckCursor.DetachPrev() // remove from cache (it might be good later) - mem.cache.Remove(req.GetCheckTx().Tx) + mem.cache.Remove(tx) } if mem.recheckCursor == mem.recheckEnd { mem.recheckCursor = nil @@ -447,7 +485,7 @@ func (mem *Mempool) ReapMaxBytesMaxGas(maxBytes, maxGas int64) types.Txs { for e := mem.txs.Front(); e != nil; e = e.Next() { memTx := e.Value.(*mempoolTx) // Check total size requirement - aminoOverhead := int64(amino.UvarintSize(uint64(len(memTx.tx)))) + aminoOverhead := types.ComputeAminoOverhead(memTx.tx, 1) if maxBytes > -1 && totalBytes+int64(len(memTx.tx))+aminoOverhead > maxBytes { return txs } @@ -565,10 +603,6 @@ func (mem *Mempool) recheckTxs(goodTxs []types.Tx) { mem.proxyAppConn.FlushAsync() } -func (mem *Mempool) isPostCheckPass(tx types.Tx, r *abci.ResponseCheckTx) bool { - return mem.postCheck == nil || mem.postCheck(tx, r) -} - //-------------------------------------------------------------------------------- // mempoolTx is a transaction that successfully ran diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 5aabd00e..d7ab8273 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -14,7 +14,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - amino "github.com/tendermint/go-amino" "github.com/tendermint/tendermint/abci/example/counter" "github.com/tendermint/tendermint/abci/example/kvstore" abci "github.com/tendermint/tendermint/abci/types" @@ -66,7 +65,13 @@ func checkTxs(t *testing.T, mempool *Mempool, count int) types.Txs { t.Error(err) } if err := mempool.CheckTx(txBytes, nil); err != nil { - t.Fatalf("Error after CheckTx: %v", err) + // Skip invalid txs. + // TestMempoolFilters will fail otherwise. It asserts a number of txs + // returned. + if IsPreCheckError(err) { + continue + } + t.Fatalf("CheckTx failed: %v while checking #%d tx", err, i) } } return txs @@ -102,11 +107,11 @@ func TestReapMaxBytesMaxGas(t *testing.T) { {20, 0, -1, 0}, {20, 0, 10, 0}, {20, 10, 10, 0}, - {20, 21, 10, 1}, - {20, 210, -1, 10}, - {20, 210, 5, 5}, - {20, 210, 10, 10}, - {20, 210, 15, 10}, + {20, 22, 10, 1}, + {20, 220, -1, 10}, + {20, 220, 5, 5}, + {20, 220, 10, 10}, + {20, 220, 15, 10}, {20, 20000, -1, 20}, {20, 20000, 5, 5}, {20, 20000, 30, 20}, @@ -126,47 +131,29 @@ func TestMempoolFilters(t *testing.T) { mempool := newMempoolWithApp(cc) emptyTxArr := []types.Tx{[]byte{}} - nopPreFilter := func(tx types.Tx) bool { return true } - nopPostFilter := func(tx types.Tx, res *abci.ResponseCheckTx) bool { return true } - - // This is the same filter we expect to be used within node/node.go and state/execution.go - nBytePreFilter := func(n int) func(tx types.Tx) bool { - return func(tx types.Tx) bool { - // We have to account for the amino overhead in the tx size as well - aminoOverhead := amino.UvarintSize(uint64(len(tx))) - return (len(tx) + aminoOverhead) <= n - } - } - - nGasPostFilter := func(n int64) func(tx types.Tx, res *abci.ResponseCheckTx) bool { - return func(tx types.Tx, res *abci.ResponseCheckTx) bool { - if n == -1 { - return true - } - return res.GasWanted <= n - } - } + nopPreFilter := func(tx types.Tx) error { return nil } + nopPostFilter := func(tx types.Tx, res *abci.ResponseCheckTx) error { return nil } // each table driven test creates numTxsToCreate txs with checkTx, and at the end clears all remaining txs. // each tx has 20 bytes + amino overhead = 21 bytes, 1 gas tests := []struct { numTxsToCreate int - preFilter func(tx types.Tx) bool - postFilter func(tx types.Tx, res *abci.ResponseCheckTx) bool + preFilter PreCheckFunc + postFilter PostCheckFunc expectedNumTxs int }{ {10, nopPreFilter, nopPostFilter, 10}, - {10, nBytePreFilter(10), nopPostFilter, 0}, - {10, nBytePreFilter(20), nopPostFilter, 0}, - {10, nBytePreFilter(21), nopPostFilter, 10}, - {10, nopPreFilter, nGasPostFilter(-1), 10}, - {10, nopPreFilter, nGasPostFilter(0), 0}, - {10, nopPreFilter, nGasPostFilter(1), 10}, - {10, nopPreFilter, nGasPostFilter(3000), 10}, - {10, nBytePreFilter(10), nGasPostFilter(20), 0}, - {10, nBytePreFilter(30), nGasPostFilter(20), 10}, - {10, nBytePreFilter(21), nGasPostFilter(1), 10}, - {10, nBytePreFilter(21), nGasPostFilter(0), 0}, + {10, PreCheckAminoMaxBytes(10), nopPostFilter, 0}, + {10, PreCheckAminoMaxBytes(20), nopPostFilter, 0}, + {10, PreCheckAminoMaxBytes(22), nopPostFilter, 10}, + {10, nopPreFilter, PostCheckMaxGas(-1), 10}, + {10, nopPreFilter, PostCheckMaxGas(0), 0}, + {10, nopPreFilter, PostCheckMaxGas(1), 10}, + {10, nopPreFilter, PostCheckMaxGas(3000), 10}, + {10, PreCheckAminoMaxBytes(10), PostCheckMaxGas(20), 0}, + {10, PreCheckAminoMaxBytes(30), PostCheckMaxGas(20), 10}, + {10, PreCheckAminoMaxBytes(22), PostCheckMaxGas(1), 10}, + {10, PreCheckAminoMaxBytes(22), PostCheckMaxGas(0), 0}, } for tcIndex, tt := range tests { mempool.Update(1, emptyTxArr, tt.preFilter, tt.postFilter) @@ -385,15 +372,12 @@ func TestMempoolCloseWAL(t *testing.T) { // 7. Invoke CloseWAL() and ensure it discards the // WAL thus any other write won't go through. - require.True(t, mempool.CloseWAL(), "CloseWAL should CloseWAL") + mempool.CloseWAL() mempool.CheckTx(types.Tx([]byte("bar")), nil) sum2 := checksumFile(walFilepath, t) require.Equal(t, sum1, sum2, "expected no change to the WAL after invoking CloseWAL() since it was discarded") - // 8. Second CloseWAL should do nothing - require.False(t, mempool.CloseWAL(), "CloseWAL should CloseWAL") - - // 9. Sanity check to ensure that the WAL file still exists + // 8. Sanity check to ensure that the WAL file still exists m3, err := filepath.Glob(filepath.Join(rootDir, "*")) require.Nil(t, err, "successful globbing expected") require.Equal(t, 1, len(m3), "expecting the wal match in") diff --git a/node/node.go b/node/node.go index f62a8b47..0652a392 100644 --- a/node/node.go +++ b/node/node.go @@ -13,8 +13,8 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/tendermint/go-amino" + amino "github.com/tendermint/go-amino" abci "github.com/tendermint/tendermint/abci/types" bc "github.com/tendermint/tendermint/blockchain" cfg "github.com/tendermint/tendermint/config" @@ -265,21 +265,14 @@ func NewNode(config *cfg.Config, proxyApp.Mempool(), state.LastBlockHeight, mempl.WithMetrics(memplMetrics), - mempl.WithPreCheck( - mempl.PreCheckAminoMaxBytes( - types.MaxDataBytesUnknownEvidence( - state.ConsensusParams.BlockSize.MaxBytes, - state.Validators.Size(), - ), - ), - ), - mempl.WithPostCheck( - mempl.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas), - ), + mempl.WithPreCheck(sm.TxPreCheck(state)), + mempl.WithPostCheck(sm.TxPostCheck(state)), ) mempoolLogger := logger.With("module", "mempool") mempool.SetLogger(mempoolLogger) - mempool.InitWAL() // no need to have the mempool wal during tests + if config.Mempool.WalEnabled() { + mempool.InitWAL() // no need to have the mempool wal during tests + } mempoolReactor := mempl.NewMempoolReactor(config.Mempool, mempool) mempoolReactor.SetLogger(mempoolLogger) @@ -586,6 +579,11 @@ func (n *Node) OnStop() { // TODO: gracefully disconnect from peers. n.sw.Stop() + // stop mempool WAL + if n.config.Mempool.WalEnabled() { + n.mempoolReactor.Mempool.CloseWAL() + } + if err := n.transport.Close(); err != nil { n.Logger.Error("Error closing transport", "err", err) } diff --git a/p2p/netaddress.go b/p2p/netaddress.go index ec9a0ea7..f60271bc 100644 --- a/p2p/netaddress.go +++ b/p2p/netaddress.go @@ -32,8 +32,10 @@ type NetAddress struct { str string } -// IDAddressString returns id@hostPort. -func IDAddressString(id ID, hostPort string) string { +// IDAddressString returns id@hostPort. It strips the leading +// protocol from protocolHostPort if it exists. +func IDAddressString(id ID, protocolHostPort string) string { + hostPort := removeProtocolIfDefined(protocolHostPort) return fmt.Sprintf("%s@%s", id, hostPort) } @@ -218,10 +220,22 @@ func (na *NetAddress) Routable() bool { // For IPv4 these are either a 0 or all bits set address. For IPv6 a zero // address or one that matches the RFC3849 documentation address format. func (na *NetAddress) Valid() bool { + if string(na.ID) != "" { + data, err := hex.DecodeString(string(na.ID)) + if err != nil || len(data) != IDByteLength { + return false + } + } return na.IP != nil && !(na.IP.IsUnspecified() || na.RFC3849() || na.IP.Equal(net.IPv4bcast)) } +// HasID returns true if the address has an ID. +// NOTE: It does not check whether the ID is valid or not. +func (na *NetAddress) HasID() bool { + return string(na.ID) != "" +} + // Local returns true if it is a local address. func (na *NetAddress) Local() bool { return na.IP.IsLoopback() || zero4.Contains(na.IP) diff --git a/p2p/node_info.go b/p2p/node_info.go index e46174e0..c36d98d9 100644 --- a/p2p/node_info.go +++ b/p2p/node_info.go @@ -216,7 +216,8 @@ OUTER_LOOP: // ListenAddr. Note that the ListenAddr is not authenticated and // may not match that address actually dialed if its an outbound peer. func (info DefaultNodeInfo) NetAddress() *NetAddress { - netAddr, err := NewNetAddressString(IDAddressString(info.ID(), info.ListenAddr)) + idAddr := IDAddressString(info.ID(), info.ListenAddr) + netAddr, err := NewNetAddressString(idAddr) if err != nil { switch err.(type) { case ErrNetAddressLookup: diff --git a/p2p/peer.go b/p2p/peer.go index 944174b0..e98c16d2 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -240,7 +240,7 @@ func (p *peer) Send(chID byte, msgBytes []byte) bool { } res := p.mconn.Send(chID, msgBytes) if res { - p.metrics.PeerSendBytesTotal.With("peer-id", string(p.ID())).Add(float64(len(msgBytes))) + p.metrics.PeerSendBytesTotal.With("peer_id", string(p.ID())).Add(float64(len(msgBytes))) } return res } @@ -255,7 +255,7 @@ func (p *peer) TrySend(chID byte, msgBytes []byte) bool { } res := p.mconn.TrySend(chID, msgBytes) if res { - p.metrics.PeerSendBytesTotal.With("peer-id", string(p.ID())).Add(float64(len(msgBytes))) + p.metrics.PeerSendBytesTotal.With("peer_id", string(p.ID())).Add(float64(len(msgBytes))) } return res } @@ -330,7 +330,7 @@ func (p *peer) metricsReporter() { sendQueueSize += float64(chStatus.SendQueueSize) } - p.metrics.PeerPendingSendBytes.With("peer-id", string(p.ID())).Set(sendQueueSize) + p.metrics.PeerPendingSendBytes.With("peer_id", string(p.ID())).Set(sendQueueSize) case <-p.Quit(): return } diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index 61710bbf..405a4628 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -652,6 +652,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { return ErrAddrBookInvalidAddr{addr} } + if !addr.HasID() { + return ErrAddrBookInvalidAddrNoID{addr} + } + // TODO: we should track ourAddrs by ID and by IP:PORT and refuse both. if _, ok := a.ourAddrs[addr.String()]; ok { return ErrAddrBookSelf{addr} diff --git a/p2p/pex/errors.go b/p2p/pex/errors.go index fbee748a..1f44ceee 100644 --- a/p2p/pex/errors.go +++ b/p2p/pex/errors.go @@ -54,3 +54,11 @@ type ErrAddrBookInvalidAddr struct { func (err ErrAddrBookInvalidAddr) Error() string { return fmt.Sprintf("Cannot add invalid address %v", err.Addr) } + +type ErrAddrBookInvalidAddrNoID struct { + Addr *p2p.NetAddress +} + +func (err ErrAddrBookInvalidAddrNoID) Error() string { + return fmt.Sprintf("Cannot add address with no ID %v", err.Addr) +} diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index 46a12c48..85d292b0 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -221,7 +221,11 @@ func (r *PEXReactor) Receive(chID byte, src Peer, msgBytes []byte) { // 2) limit the output size if r.config.SeedMode { r.SendAddrs(src, r.book.GetSelectionWithBias(biasToSelectNewPeers)) - r.Switch.StopPeerGracefully(src) + go func() { + // TODO Fix properly #2092 + time.Sleep(time.Second * 5) + r.Switch.StopPeerGracefully(src) + }() } else { r.SendAddrs(src, r.book.GetSelection()) } diff --git a/p2p/switch.go b/p2p/switch.go index b1406b9b..b70900ea 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -328,6 +328,11 @@ func (sw *Switch) reconnectToPeer(addr *NetAddress) { return } + if sw.IsDialingOrExistingAddress(addr) { + sw.Logger.Debug("Peer connection has been established or dialed while we waiting next try", "addr", addr) + return + } + err := sw.DialPeerWithAddress(addr, true) if err == nil { return // success @@ -415,12 +420,15 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b if addr.Same(ourAddr) { sw.Logger.Debug("Ignore attempt to connect to ourselves", "addr", addr, "ourAddr", ourAddr) return - } else if sw.IsDialingOrExistingAddress(addr) { + } + + sw.randomSleep(0) + + if sw.IsDialingOrExistingAddress(addr) { sw.Logger.Debug("Ignore attempt to connect to an existing peer", "addr", addr) return } - sw.randomSleep(0) err := sw.DialPeerWithAddress(addr, persistent) if err != nil { switch err.(type) { @@ -616,7 +624,7 @@ func (sw *Switch) addPeer(p Peer) error { return err } - p.SetLogger(sw.Logger.With("peer", p.NodeInfo().NetAddress().String)) + p.SetLogger(sw.Logger.With("peer", p.NodeInfo().NetAddress())) // All good. Start peer if sw.IsRunning() { diff --git a/state/execution.go b/state/execution.go index cc8e7e75..4f5a1545 100644 --- a/state/execution.go +++ b/state/execution.go @@ -8,7 +8,6 @@ import ( dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/fail" "github.com/tendermint/tendermint/libs/log" - "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/proxy" "github.com/tendermint/tendermint/types" ) @@ -180,13 +179,8 @@ func (blockExec *BlockExecutor) Commit( err = blockExec.mempool.Update( block.Height, block.Txs, - mempool.PreCheckAminoMaxBytes( - types.MaxDataBytesUnknownEvidence( - state.ConsensusParams.BlockSize.MaxBytes, - state.Validators.Size(), - ), - ), - mempool.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas), + TxPreCheck(state), + TxPostCheck(state), ) return res.Data, err diff --git a/state/tx_filter.go b/state/tx_filter.go index b8882d8e..518eb187 100644 --- a/state/tx_filter.go +++ b/state/tx_filter.go @@ -1,15 +1,22 @@ package state import ( + mempl "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/types" ) -// TxFilter returns a function to filter transactions. The function limits the -// size of a transaction to the maximum block's data size. -func TxFilter(state State) func(tx types.Tx) bool { +// TxPreCheck returns a function to filter transactions before processing. +// The function limits the size of a transaction to the block's maximum data size. +func TxPreCheck(state State) mempl.PreCheckFunc { maxDataBytes := types.MaxDataBytesUnknownEvidence( state.ConsensusParams.BlockSize.MaxBytes, state.Validators.Size(), ) - return func(tx types.Tx) bool { return int64(len(tx)) <= maxDataBytes } + return mempl.PreCheckAminoMaxBytes(maxDataBytes) +} + +// TxPostCheck returns a function to filter transactions after processing. +// The function limits the gas wanted by a transaction to the block's maximum total gas. +func TxPostCheck(state State) mempl.PostCheckFunc { + return mempl.PostCheckMaxGas(state.ConsensusParams.BlockSize.MaxGas) } diff --git a/state/tx_filter_test.go b/state/tx_filter_test.go index e6b8999f..52ae396b 100644 --- a/state/tx_filter_test.go +++ b/state/tx_filter_test.go @@ -18,12 +18,18 @@ func TestTxFilter(t *testing.T) { genDoc := randomGenesisDoc() genDoc.ConsensusParams.BlockSize.MaxBytes = 3000 + // Max size of Txs is much smaller than size of block, + // since we need to account for commits and evidence. testCases := []struct { - tx types.Tx - isTxValid bool + tx types.Tx + isErr bool }{ - {types.Tx(cmn.RandBytes(250)), true}, - {types.Tx(cmn.RandBytes(3001)), false}, + {types.Tx(cmn.RandBytes(250)), false}, + {types.Tx(cmn.RandBytes(1809)), false}, + {types.Tx(cmn.RandBytes(1810)), false}, + {types.Tx(cmn.RandBytes(1811)), true}, + {types.Tx(cmn.RandBytes(1812)), true}, + {types.Tx(cmn.RandBytes(3000)), true}, } for i, tc := range testCases { @@ -31,8 +37,12 @@ func TestTxFilter(t *testing.T) { state, err := LoadStateFromDBOrGenesisDoc(stateDB, genDoc) require.NoError(t, err) - f := TxFilter(state) - assert.Equal(t, tc.isTxValid, f(tc.tx), "#%v", i) + f := TxPreCheck(state) + if tc.isErr { + assert.NotNil(t, f(tc.tx), "#%v", i) + } else { + assert.Nil(t, f(tc.tx), "#%v", i) + } } } diff --git a/types/block.go b/types/block.go index 4ae51d4d..b2cddb5e 100644 --- a/types/block.go +++ b/types/block.go @@ -21,6 +21,8 @@ const ( // MaxAminoOverheadForBlock - maximum amino overhead to encode a block (up to // MaxBlockSizeBytes in size) not including it's parts except Data. + // This means it also excludes the overhead for individual transactions. + // To compute individual transactions' overhead use types.ComputeAminoOverhead(tx types.Tx, fieldNum int). // // Uvarint length of MaxBlockSizeBytes: 4 bytes // 2 fields (2 embedded): 2 bytes diff --git a/types/block_test.go b/types/block_test.go index cdea293f..bedd8c8d 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -250,7 +250,7 @@ func TestMaxHeaderBytes(t *testing.T) { timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) h := Header{ - Version: version.Consensus{math.MaxInt64, math.MaxInt64}, + Version: version.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, ChainID: maxChainID, Height: math.MaxInt64, Time: timestamp, diff --git a/types/evidence_test.go b/types/evidence_test.go index 033b51e5..a96b63a9 100644 --- a/types/evidence_test.go +++ b/types/evidence_test.go @@ -61,7 +61,7 @@ func TestEvidence(t *testing.T) { {vote1, makeVote(val, chainID, 0, 10, 3, 1, blockID2), false}, // wrong round {vote1, makeVote(val, chainID, 0, 10, 2, 2, blockID2), false}, // wrong step {vote1, makeVote(val2, chainID, 0, 10, 2, 1, blockID), false}, // wrong validator - {vote1, badVote, false}, // signed by wrong key + {vote1, badVote, false}, // signed by wrong key } pubKey := val.GetPubKey() @@ -121,3 +121,38 @@ func randomDuplicatedVoteEvidence() *DuplicateVoteEvidence { VoteB: makeVote(val, chainID, 0, 10, 2, 1, blockID2), } } + +func TestDuplicateVoteEvidenceValidation(t *testing.T) { + val := NewMockPV() + blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) + blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) + const chainID = "mychain" + + testCases := []struct { + testName string + malleateEvidence func(*DuplicateVoteEvidence) + expectErr bool + }{ + {"Good DuplicateVoteEvidence", func(ev *DuplicateVoteEvidence) {}, false}, + {"Nil vote A", func(ev *DuplicateVoteEvidence) { ev.VoteA = nil }, true}, + {"Nil vote B", func(ev *DuplicateVoteEvidence) { ev.VoteB = nil }, true}, + {"Nil votes", func(ev *DuplicateVoteEvidence) { + ev.VoteA = nil + ev.VoteB = nil + }, true}, + {"Invalid vote type", func(ev *DuplicateVoteEvidence) { + ev.VoteA = makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0, blockID2) + }, true}, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + ev := &DuplicateVoteEvidence{ + PubKey: secp256k1.GenPrivKey().PubKey(), + VoteA: makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0x02, blockID), + VoteB: makeVote(val, chainID, math.MaxInt64, math.MaxInt64, math.MaxInt64, 0x02, blockID2), + } + tc.malleateEvidence(ev) + assert.Equal(t, tc.expectErr, ev.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/types/heartbeat_test.go b/types/heartbeat_test.go index e1ffdd6f..0951c7b9 100644 --- a/types/heartbeat_test.go +++ b/types/heartbeat_test.go @@ -3,8 +3,10 @@ package types import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/secp256k1" ) func TestHeartbeatCopy(t *testing.T) { @@ -58,3 +60,45 @@ func TestHeartbeatWriteSignBytes(t *testing.T) { require.Equal(t, string(signBytes), "null") }) } + +func TestHeartbeatValidateBasic(t *testing.T) { + testCases := []struct { + testName string + malleateHeartBeat func(*Heartbeat) + expectErr bool + }{ + {"Good HeartBeat", func(hb *Heartbeat) {}, false}, + {"Invalid address size", func(hb *Heartbeat) { + hb.ValidatorAddress = nil + }, true}, + {"Negative validator index", func(hb *Heartbeat) { + hb.ValidatorIndex = -1 + }, true}, + {"Negative height", func(hb *Heartbeat) { + hb.Height = -1 + }, true}, + {"Negative round", func(hb *Heartbeat) { + hb.Round = -1 + }, true}, + {"Negative sequence", func(hb *Heartbeat) { + hb.Sequence = -1 + }, true}, + {"Missing signature", func(hb *Heartbeat) { + hb.Signature = nil + }, true}, + {"Signature too big", func(hb *Heartbeat) { + hb.Signature = make([]byte, MaxSignatureSize+1) + }, true}, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + hb := &Heartbeat{ + ValidatorAddress: secp256k1.GenPrivKey().PubKey().Address(), + Signature: make([]byte, 4), + ValidatorIndex: 1, Height: 10, Round: 1} + + tc.malleateHeartBeat(hb) + assert.Equal(t, tc.expectErr, hb.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/types/part_set_test.go b/types/part_set_test.go index 3576e747..e597088c 100644 --- a/types/part_set_test.go +++ b/types/part_set_test.go @@ -83,3 +83,48 @@ func TestWrongProof(t *testing.T) { t.Errorf("Expected to fail adding a part with bad bytes.") } } + +func TestPartSetHeaderSetValidateBasic(t *testing.T) { + + testCases := []struct { + testName string + malleatePartSetHeader func(*PartSetHeader) + expectErr bool + }{ + {"Good PartSet", func(psHeader *PartSetHeader) {}, false}, + {"Negative Total", func(psHeader *PartSetHeader) { psHeader.Total = -2 }, true}, + {"Invalid Hash", func(psHeader *PartSetHeader) { psHeader.Hash = make([]byte, 1) }, true}, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + data := cmn.RandBytes(testPartSize * 100) + ps := NewPartSetFromData(data, testPartSize) + psHeader := ps.Header() + tc.malleatePartSetHeader(&psHeader) + assert.Equal(t, tc.expectErr, psHeader.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} + +func TestPartValidateBasic(t *testing.T) { + + testCases := []struct { + testName string + malleatePart func(*Part) + expectErr bool + }{ + {"Good Part", func(pt *Part) {}, false}, + {"Negative index", func(pt *Part) { pt.Index = -1 }, true}, + {"Too big part", func(pt *Part) { pt.Bytes = make([]byte, BlockPartSizeBytes+1) }, true}, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + data := cmn.RandBytes(testPartSize * 100) + ps := NewPartSetFromData(data, testPartSize) + part := ps.GetPart(0) + tc.malleatePart(part) + assert.Equal(t, tc.expectErr, part.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/types/proposal_test.go b/types/proposal_test.go index 9738db2d..f1c048e1 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -1,10 +1,13 @@ package types import ( + "math" "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/tmhash" ) var testProposal *Proposal @@ -97,3 +100,41 @@ func BenchmarkProposalVerifySignature(b *testing.B) { pubKey.VerifyBytes(testProposal.SignBytes("test_chain_id"), testProposal.Signature) } } + +func TestProposalValidateBasic(t *testing.T) { + + privVal := NewMockPV() + testCases := []struct { + testName string + malleateProposal func(*Proposal) + expectErr bool + }{ + {"Good Proposal", func(p *Proposal) {}, false}, + {"Invalid Type", func(p *Proposal) { p.Type = PrecommitType }, true}, + {"Invalid Height", func(p *Proposal) { p.Height = -1 }, true}, + {"Invalid Round", func(p *Proposal) { p.Round = -1 }, true}, + {"Invalid POLRound", func(p *Proposal) { p.POLRound = -2 }, true}, + {"Invalid BlockId", func(p *Proposal) { + p.BlockID = BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}} + }, true}, + {"Invalid Signature", func(p *Proposal) { + p.Signature = make([]byte, 0) + }, true}, + {"Too big Signature", func(p *Proposal) { + p.Signature = make([]byte, MaxSignatureSize+1) + }, true}, + } + blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), math.MaxInt64, tmhash.Sum([]byte("partshash"))) + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + prop := NewProposal( + 4, 2, 2, + blockID) + err := privVal.SignProposal("test_chain_id", prop) + require.NoError(t, err) + tc.malleateProposal(prop) + assert.Equal(t, tc.expectErr, prop.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/types/tx.go b/types/tx.go index 10c097e3..41be7794 100644 --- a/types/tx.go +++ b/types/tx.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "github.com/tendermint/go-amino" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/crypto/tmhash" @@ -118,3 +120,18 @@ type TxResult struct { Tx Tx `json:"tx"` Result abci.ResponseDeliverTx `json:"result"` } + +// ComputeAminoOverhead calculates the overhead for amino encoding a transaction. +// The overhead consists of varint encoding the field number and the wire type +// (= length-delimited = 2), and another varint encoding the length of the +// transaction. +// The field number can be the field number of the particular transaction, or +// the field number of the parenting struct that contains the transactions []Tx +// as a field (this field number is repeated for each contained Tx). +// If some []Tx are encoded directly (without a parenting struct), the default +// fieldNum is also 1 (see BinFieldNum in amino.MarshalBinaryBare). +func ComputeAminoOverhead(tx Tx, fieldNum int) int64 { + fnum := uint64(fieldNum) + typ3AndFieldNum := (uint64(fnum) << 3) | uint64(amino.Typ3_ByteLength) + return int64(amino.UvarintSize(typ3AndFieldNum)) + int64(amino.UvarintSize(uint64(len(tx)))) +} diff --git a/types/tx_test.go b/types/tx_test.go index 6ce23d6f..3afaaccc 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -96,6 +96,63 @@ func TestTxProofUnchangable(t *testing.T) { } } +func TestComputeTxsOverhead(t *testing.T) { + cases := []struct { + txs Txs + wantOverhead int + }{ + {Txs{[]byte{6, 6, 6, 6, 6, 6}}, 2}, + // one 21 Mb transaction: + {Txs{make([]byte, 22020096, 22020096)}, 5}, + // two 21Mb/2 sized transactions: + {Txs{make([]byte, 11010048, 11010048), make([]byte, 11010048, 11010048)}, 10}, + {Txs{[]byte{1, 2, 3}, []byte{1, 2, 3}, []byte{4, 5, 6}}, 6}, + {Txs{[]byte{100, 5, 64}, []byte{42, 116, 118}, []byte{6, 6, 6}, []byte{6, 6, 6}}, 8}, + } + + for _, tc := range cases { + totalBytes := int64(0) + totalOverhead := int64(0) + for _, tx := range tc.txs { + aminoOverhead := ComputeAminoOverhead(tx, 1) + totalOverhead += aminoOverhead + totalBytes += aminoOverhead + int64(len(tx)) + } + bz, err := cdc.MarshalBinaryBare(tc.txs) + assert.EqualValues(t, tc.wantOverhead, totalOverhead) + assert.NoError(t, err) + assert.EqualValues(t, len(bz), totalBytes) + } +} + +func TestComputeAminoOverhead(t *testing.T) { + cases := []struct { + tx Tx + fieldNum int + want int + }{ + {[]byte{6, 6, 6}, 1, 2}, + {[]byte{6, 6, 6}, 16, 3}, + {[]byte{6, 6, 6}, 32, 3}, + {[]byte{6, 6, 6}, 64, 3}, + {[]byte{6, 6, 6}, 512, 3}, + {[]byte{6, 6, 6}, 1024, 3}, + {[]byte{6, 6, 6}, 2048, 4}, + {make([]byte, 64), 1, 2}, + {make([]byte, 65), 1, 2}, + {make([]byte, 127), 1, 2}, + {make([]byte, 128), 1, 3}, + {make([]byte, 256), 1, 3}, + {make([]byte, 512), 1, 3}, + {make([]byte, 1024), 1, 3}, + {make([]byte, 128), 16, 4}, + } + for _, tc := range cases { + got := ComputeAminoOverhead(tc.tx, tc.fieldNum) + assert.EqualValues(t, tc.want, got) + } +} + func testTxProofUnchangable(t *testing.T) { // make some proof txs := makeTxs(randInt(2, 100), randInt(16, 128)) diff --git a/types/vote_set.go b/types/vote_set.go index cdfa3d40..0cf6cbb7 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -158,7 +158,7 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) { if (vote.Height != voteSet.height) || (vote.Round != voteSet.round) || (vote.Type != voteSet.type_) { - return false, errors.Wrapf(ErrVoteUnexpectedStep, "Got %d/%d/%d, expected %d/%d/%d", + return false, errors.Wrapf(ErrVoteUnexpectedStep, "Expected %d/%d/%d, but got %d/%d/%d", voteSet.height, voteSet.round, voteSet.type_, vote.Height, vote.Round, vote.Type) } diff --git a/types/vote_test.go b/types/vote_test.go index cda54f89..942f2d6b 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -250,3 +250,31 @@ func TestVoteString(t *testing.T) { t.Errorf("Got unexpected string for Vote. Expected:\n%v\nGot:\n%v", expected, str2) } } + +func TestVoteValidateBasic(t *testing.T) { + privVal := NewMockPV() + + testCases := []struct { + testName string + malleateVote func(*Vote) + expectErr bool + }{ + {"Good Vote", func(v *Vote) {}, false}, + {"Negative Height", func(v *Vote) { v.Height = -1 }, true}, + {"Negative Round", func(v *Vote) { v.Round = -1 }, true}, + {"Invalid BlockID", func(v *Vote) { v.BlockID = BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}} }, true}, + {"Invalid Address", func(v *Vote) { v.ValidatorAddress = make([]byte, 1) }, true}, + {"Invalid ValidatorIndex", func(v *Vote) { v.ValidatorIndex = -1 }, true}, + {"Invalid Signature", func(v *Vote) { v.Signature = nil }, true}, + {"Too big Signature", func(v *Vote) { v.Signature = make([]byte, MaxSignatureSize+1) }, true}, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + vote := examplePrecommit() + err := privVal.SignVote("test_chain_id", vote) + require.NoError(t, err) + tc.malleateVote(vote) + assert.Equal(t, tc.expectErr, vote.ValidateBasic() != nil, "Validate Basic had an unexpected result") + }) + } +} diff --git a/version/version.go b/version/version.go index b4664fd7..b7a72a7f 100644 --- a/version/version.go +++ b/version/version.go @@ -18,7 +18,7 @@ const ( // TMCoreSemVer is the current version of Tendermint Core. // It's the Semantic Version of the software. // Must be a string because scripts like dist.sh read this file. - TMCoreSemVer = "0.26.0" + TMCoreSemVer = "0.26.1" // ABCISemVer is the semantic version of the ABCI library ABCISemVer = "0.15.0"