Merge pull request #1711 from tendermint/bucky/adrs

Bucky/adrs

docs/app-development.md

@@ -99,14 +99,14 @@ 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 Protobuf3 encoded, but additionally length-prefixed to
+the socket are proto3 encoded, but additionally length-prefixed to
-facilitate use as a streaming protocol. Protobuf3 doesn't have an
+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 Protobuf3 encoded ABCI message is 0xDEADBEEF (4
+For example, if the proto3 encoded ABCI message is 0xDEADBEEF (4
-bytes), the length-prefixed message is 0x0104DEADBEEF. If the Protobuf3
+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....
 
@@ -188,9 +188,9 @@ In Java:
 
     ResponseCheckTx requestCheckTx(RequestCheckTx req) {
         byte[] transaction = req.getTx().toByteArray();
-    
+
         // validate transaction
-    
+
         if (notValid) {
             return ResponseCheckTx.newBuilder().setCode(CodeType.BadNonce).setLog("invalid tx").build();
         } else {
@@ -260,15 +260,15 @@ In Java:
      */
     ResponseDeliverTx deliverTx(RequestDeliverTx request) {
         byte[] transaction  = request.getTx().toByteArray();
-    
+
         // validate your transaction
-    
+
         if (notValid) {
             return ResponseDeliverTx.newBuilder().setCode(CodeType.BadNonce).setLog("transaction was invalid").build();
         } else {
             ResponseDeliverTx.newBuilder().setCode(CodeType.OK).build();
         }
-    
+
     }
 
 ### Commit
@@ -302,10 +302,10 @@ In go:
 In Java:
 
     ResponseCommit requestCommit(RequestCommit requestCommit) {
-    
+
         // update the internal app-state
         byte[] newAppState = calculateAppState();
-    
+
         // and return it to the node
         return ResponseCommit.newBuilder().setCode(CodeType.OK).setData(ByteString.copyFrom(newAppState)).build();
     }
@@ -326,7 +326,7 @@ In go:
     func (app *PersistentKVStoreApplication) BeginBlock(params types.RequestBeginBlock) {
       // update latest block info
       app.blockHeader = params.Header
-    
+
       // reset valset changes
       app.changes = make([]*types.Validator, 0)
     }
@@ -337,14 +337,14 @@ In Java:
      * all types come from protobuf definition
      */
     ResponseBeginBlock requestBeginBlock(RequestBeginBlock req) {
-    
+
         Header header = req.getHeader();
         byte[] prevAppHash = header.getAppHash().toByteArray();
         long prevHeight = header.getHeight();
         long numTxs = header.getNumTxs();
-    
+
         // run your pre-block logic. Maybe prepare a state snapshot, message components, etc
-    
+
         return ResponseBeginBlock.newBuilder().build();
     }
 
@@ -377,10 +377,10 @@ In Java:
     ResponseEndBlock requestEndBlock(RequestEndBlock req) {
         final long currentHeight = req.getHeight();
         final byte[] validatorPubKey = getValPubKey();
-    
+
         ResponseEndBlock.Builder builder = ResponseEndBlock.newBuilder();
         builder.addDiffs(1, Types.Validator.newBuilder().setPower(10L).setPubKey(ByteString.copyFrom(validatorPubKey)).build());
-    
+
         return builder.build();
     }
 
@@ -437,25 +437,25 @@ In Java:
     ResponseQuery requestQuery(RequestQuery req) {
         final boolean isProveQuery = req.getProve();
         final ResponseQuery.Builder responseBuilder = ResponseQuery.newBuilder();
-    
+
         if (isProveQuery) {
             com.app.example.ProofResult proofResult = generateProof(req.getData().toByteArray());
             final byte[] proofAsByteArray = proofResult.getAsByteArray();
-    
+
             responseBuilder.setProof(ByteString.copyFrom(proofAsByteArray));
             responseBuilder.setKey(req.getData());
             responseBuilder.setValue(ByteString.copyFrom(proofResult.getData()));
             responseBuilder.setLog(result.getLogValue());
         } else {
             byte[] queryData = req.getData().toByteArray();
-    
+
             final com.app.example.QueryResult result = generateQueryResult(queryData);
-    
+
             responseBuilder.setIndex(result.getIndex());
             responseBuilder.setValue(ByteString.copyFrom(result.getValue()));
             responseBuilder.setLog(result.getLogValue());
         }
-    
+
         return responseBuilder.build();
     }
 
@@ -515,13 +515,13 @@ In Java:
     ResponseInitChain requestInitChain(RequestInitChain req) {
         final int validatorsCount = req.getValidatorsCount();
         final List<Types.Validator> validatorsList = req.getValidatorsList();
-    
+
         validatorsList.forEach((validator) -> {
             long power = validator.getPower();
             byte[] validatorPubKey = validator.getPubKey().toByteArray();
-    
+
             // do somehing for validator setup in app
         });
-    
+
         return ResponseInitChain.newBuilder().build();
     }

docs/architecture/README.md

@@ -1,5 +1,22 @@
-# Architecture Decision Records
+# Architecture Decision Records (ADR)
 
-This is a location to record all high-level architecture decisions in the tendermint project.  Not the implementation details, but the reasoning that happened.  This should be refered to for guidance of the "right way" to extend the application.  And if we notice that the original decisions were lacking, we should have another open discussion, record the new decisions here, and then modify the code to match.
+This is a location to record all high-level architecture decisions in the tendermint project.
 
-Read up on the concept in this [blog post](https://product.reverb.com/documenting-architecture-decisions-the-reverb-way-a3563bb24bd0#.78xhdix6t).
+You can read more about the ADR concept in this [blog post](https://product.reverb.com/documenting-architecture-decisions-the-reverb-way-a3563bb24bd0#.78xhdix6t).
+
+An ADR should provide:
+
+- Context on the relevant goals and the current state
+- Proposed changes to achieve the goals
+- Summary of pros and cons
+- References
+- Changelog
+
+Note the distinction between an ADR and a spec. The ADR provides the context, intuition, reasoning, and
+justification for a change in architecture, or for the architecture of something
+new. The spec is much more compressed and streamlined summary of everything as
+it stands today.
+
+If recorded decisions turned out to be lacking, convene a discussion, record the new decisions here, and then modify the code to match.
+
+Note the context/background should be written in the present tense.

docs/architecture/adr-009-ABCI-design.md

@@ -0,0 +1,273 @@
+# ADR 009: ABCI UX Improvements
+
+## Changelog
+
+23-06-2018: Some minor fixes from review
+07-06-2018: Some updates based on discussion with Jae
+07-06-2018: Initial draft to match what was released in ABCI v0.11
+
+## Context
+
+The ABCI was first introduced in late 2015.  It's purpose is to be:
+
+- a generic interface between state machines and their replication engines
+- agnostic to the language the state machine is written in
+- agnostic to the replication engine that drives it
+
+This means ABCI should provide an interface for both pluggable applications and
+pluggable consensus engines.
+
+To achieve this, it uses Protocol Buffers (proto3) for message types. The dominant
+implementation is in Go.
+
+After some recent discussions with the community on github, the following were
+identified as pain points:
+
+- Amino encoded types
+- Managing validator sets
+- Imports in the protobuf file
+
+See the [references](#references) for more.
+
+### Imports
+
+The native proto library in Go generates inflexible and verbose code.
+Many in the Go community have adopted a fork called
+[gogoproto](https://github.com/gogo/protobuf) that provides a
+variety of features aimed to improve the developer experience.
+While `gogoproto` is nice, it creates an additional dependency, and compiling
+the protobuf types for other languages has been reported to fail when `gogoproto` is used.
+
+### Amino
+
+Amino is an encoding protocol designed to improve over insufficiencies of protobuf.
+It's goal is to be proto4.
+
+Many people are frustrated by incompatibility with protobuf,
+and with the requirement for Amino to be used at all within ABCI.
+
+We intend to make Amino successful enough that we can eventually use it for ABCI
+message types directly. By then it should be called proto4. In the meantime,
+we want it to be easy to use.
+
+### PubKey
+
+PubKeys are encoded using Amino (and before that, go-wire).
+Ideally, PubKeys are an interface type where we don't know all the
+implementation types, so its unfitting to use `oneof` or `enum`.
+
+### Addresses
+
+The address for ED25519 pubkey is the RIPEMD160 of the Amino
+encoded pubkey. This introduces an Amino dependency in the address generation,
+a functionality that is widely required and should be easy to compute as
+possible.
+
+### Validators
+
+To change the validator set, applications can return a list of validator updates
+with ResponseEndBlock. In these updates, the public key *must* be included,
+because Tendermint requires the public key to verify validator signatures.  This
+means ABCI developers have to work with PubKeys. That said, it would also be
+convenient to work with address information, and for it to be simple to do so.
+
+### AbsentValidators
+
+Tendermint also provides a list of validators in BeginBlock who did not sign the
+last block. This allows applications to reflect availability behaviour in the
+application, for instance by punishing validators for not having votes included
+in commits.
+
+### InitChain
+
+Tendermint passes in a list of validators here, and nothing else.  It would
+benefit the application to be able to control the initial validator set. For
+instance the genesis file could include application-based information about the
+initial validator set that the application could process to determine the
+initial validator set. Additionally, InitChain would benefit from getting all
+the genesis information.
+
+### Header
+
+ABCI provides the Header in RequestBeginBlock so the application can have
+important information about the latest state of the blockchain.
+
+## Decision
+
+### Imports
+
+Move away from gogoproto. In the short term, we will just maintain a second
+protobuf file without the gogoproto annotations. In the medium term, we will
+make copies of all the structs in Golang and shuttle back and forth. In the long
+term, we will use Amino.
+
+### Amino
+
+To simplify ABCI application development in the short term,
+Amino will be completely removed from the ABCI:
+
+- It will not be required for PubKey encoding
+- It will not be required for computing PubKey addresses
+
+That said, we are working to make Amino a huge success, and to become proto4.
+To facilitate adoption and cross-language compatibility in the near-term, Amino
+v1 will:
+
+- be fully compatible with the subset of proto3 that excludes `oneof`
+- use the Amino prefix system to provide interface types, as opposed to `oneof`
+  style union types.
+
+That said, an Amino v2 will be worked on to improve the performance of the
+format and its useability in cryptographic applications.
+
+
+### PubKey
+
+Encoding schemes infect software. As a generic middleware, ABCI aims to have
+some cross scheme compatibility. For this it has no choice but to include opaque
+bytes from time to time. While we will not enforce Amino encoding for these
+bytes yet, we need to provide a type system. The simplest way to do this is to
+use a type string.
+
+PubKey will now look like:
+
+```
+message PubKey {
+    string type
+    bytes data
+}
+```
+
+where `type` can be:
+
+- "ed225519", with `data = <raw 32-byte pubkey>`
+- "secp256k1", with `data = <33-byte OpenSSL compressed pubkey>`
+
+
+As we want to retain flexibility here, and since ideally, PubKey would be an
+interface type, we do not use `enum` or `oneof`.
+
+### Addresses
+
+To simplify and improve computing addresses, we change it to the first 20-bytes of the SHA256
+of the raw 32-byte public key.
+
+We continue to use the Bitcoin address scheme for secp256k1 keys.
+
+### Validators
+
+Add a `bytes address` field:
+
+```
+message Validator {
+    bytes address
+    PubKey pub_key
+    int64 power
+}
+```
+
+### RequestBeginBlock and AbsentValidators
+
+To simplify this, RequestBeginBlock will include the complete validator set,
+including the address, and voting power of each validator, along
+with a boolean for whether or not they voted:
+
+```
+message RequestBeginBlock {
+  bytes hash
+  Header header
+  LastCommitInfo last_commit_info
+  repeated Evidence byzantine_validators
+}
+
+message LastCommitInfo {
+  int32 CommitRound
+  repeated SigningValidator validators
+}
+
+message SigningValidator {
+    Validator validator
+    bool signed_last_block
+}
+```
+
+Note that in Validators in RequestBeginBlock, we DO NOT include public keys. Public keys are
+larger than addresses and in the future, with quantum computers, will be much
+larger. The overhead of passing them, especially during fast-sync, is
+significant.
+
+Additional, addresses are changing to be simpler to compute, further removing
+the need to include pubkeys here.
+
+In short, ABCI developers must be aware of both addresses and public keys.
+
+### ResponseEndBlock
+
+Since ResponseEndBlock includes Validator, it must now include their address.
+
+### InitChain
+
+Change RequestInitChain to give the app all the information from the genesis file:
+
+```
+message RequestInitChain {
+    int64 time
+    string chain_id
+    ConsensusParams consensus_params
+    repeated Validator validators
+    bytes app_state_bytes
+}
+```
+
+Change ResponseInitChain to allow the app to specify the initial validator set
+and consensus parameters.
+
+```
+message ResponseInitChain {
+    ConsensusParams consensus_params
+    repeated Validator validators
+}
+```
+
+### Header
+
+Now that Tendermint Amino will be compatible with proto3, the Header in ABCI
+should exactly match the Tendermint header - they will then be encoded
+identically in ABCI and in Tendermint Core.
+
+## Status
+
+Accepted.
+
+## Consequences
+
+### Positive
+
+- Easier for developers to build on the ABCI
+- ABCI and Tendermint headers are identically serialized
+
+### Negative
+
+- Maintenance overhead of alternative type encoding scheme
+- Performance overhead of passing all validator info every block (at least its
+  only addresses, and not also pubkeys)
+- Maintenance overhead of duplicate types
+
+### Neutral
+
+- ABCI developers must know about validator addresses
+
+## References
+
+- [ABCI v0.10.3 Specification (before this
+  proposal)](https://github.com/tendermint/abci/blob/v0.10.3/specification.rst)
+- [ABCI v0.11.0 Specification (implementing first draft of this
+  proposal)](https://github.com/tendermint/abci/blob/v0.11.0/specification.md)
+- [Ed25519 addresses](https://github.com/tendermint/go-crypto/issues/103)
+- [InitChain contains the
+  Genesis](https://github.com/tendermint/abci/issues/216)
+- [PubKeys](https://github.com/tendermint/tendermint/issues/1524)
+- [Notes on
+  Header](https://github.com/tendermint/tendermint/issues/1605)
+- [Gogoproto issues](https://github.com/tendermint/abci/issues/256)
+- [Absent Validators](https://github.com/tendermint/abci/issues/231)

docs/architecture/adr-010-crypto-changes.md

@@ -0,0 +1,78 @@
+# ADR 010: Crypto Changes
+
+## Context
+
+Tendermint is a cryptographic protocol that uses and composes a variety of cryptographic primitives.
+
+After nearly 4 years of development, Tendermint has recently undergone multiple security reviews to search for vulnerabilities and to assess the the use and composition of cryptographic primitives.
+
+### Hash Functions
+
+Tendermint uses RIPEMD160 universally as a hash function, most notably in its Merkle tree implementation.
+
+RIPEMD160 was chosen because it provides the shortest fingerprint that is long enough to be considered secure (ie. birthday bound of 80-bits).
+It was also developed in the open academic community, unlike NSA-designed algorithms like SHA256.
+
+That said, the cryptographic community appears to unanimously agree on the security of SHA256. It has become a universal standard, especially now that SHA1 is broken, being required in TLS connections and having optimized support in hardware.
+
+### Merkle Trees
+
+Tendermint uses a simple Merkle tree to compute digests of large structures like transaction batches
+and even blockchain headers. The Merkle tree length prefixes byte arrays before concatenating and hashing them.
+It uses RIPEMD160.
+
+### Addresses
+
+ED25519 addresses are computed using the RIPEMD160 of the Amino encoding of the public key.
+RIPEMD160 is generally considered an outdated hash function, and is much slower
+than more modern functions like SHA256 or Blake2.
+
+### Authenticated Encryption
+
+Tendermint P2P connections use authenticated encryption to provide privacy and authentication in the communications.
+This is done using the simple Station-to-Station protocol with the NaCL Ed25519 library.
+
+While there have been no vulnerabilities found in the implementation, there are some concerns:
+
+- NaCL uses Salsa20, a not-widely used and relatively out-dated stream cipher that has been obsoleted by ChaCha20
+- Connections use RIPEMD160 to compute a value that is used for the encryption nonce with subtle requirements on how it's used
+
+## Decision
+
+### Hash Functions
+
+Use the first 20-bytes of the SHA256 hash instead of RIPEMD160 for everything
+
+### Merkle Trees
+
+TODO
+
+### Addresses
+
+Compute ED25519 addresses as the first 20-bytes of the SHA256 of the raw 32-byte public key
+
+### Authenticated Encryption
+
+Make the following changes:
+
+- Use xChaCha20 instead of xSalsa20 - https://github.com/tendermint/tendermint/issues/1124
+- Use an HKDF instead of RIPEMD160 to compute nonces - https://github.com/tendermint/tendermint/issues/1165
+
+## Status
+
+## Consequences
+
+### Positive
+
+- More modern and standard cryptographic functions with wider adoption and hardware acceleration
+
+
+### Negative
+
+- Exact authenticated encryption construction isn't already provided in a well-used library
+
+
+### Neutral
+
+## References
+

docs/spec/blockchain/encoding.md

@@ -2,8 +2,8 @@
 
 ## Amino
 
-Tendermint uses the Protobuf3 derivative [Amino](https://github.com/tendermint/go-amino) for all data structures.
+Tendermint uses the proto3 derivative [Amino](https://github.com/tendermint/go-amino) for all data structures.
-Think of Amino as an object-oriented Protobuf3 with native JSON support.
+Think of Amino as an object-oriented proto3 with native JSON support.
 The goal of the Amino encoding protocol is to bring parity between application
 logic objects and persistence objects.
 
@@ -21,7 +21,7 @@ arbitrary object and return the Amino encoded bytes.
 ## Byte Arrays
 
 The encoding of a byte array is simply the raw-bytes prefixed with the length of
-the array as a `UVarint` (what Protobuf calls a `Varint`).
+the array as a `UVarint` (what proto calls a `Varint`).
 
 For details on varints, see the [protobuf
 spec](https://developers.google.com/protocol-buffers/docs/encoding#varints).