Fixes#5192.
@liamsi Can you verify that the test vectors match the Rust implementation? I updated `ProofsFromByteSlices()` as well, anything else that should be updated?
Reorganizes the Protobuf schemas. It is mostly bikeshedding, so if something is contentious or causes a lot of extra work then I'm fine with reverting. Some Protobuf and Go import paths will change.
* Move `abci/types/types.proto` to `abci/types.proto`.
* Move `crypto/keys/types.proto` and `crypto/merkle/types.proto` to `crypto/keys.proto` and `crypto/proof.proto`.
* Drop the use of `msgs` in filenames, as "message" is a very overloaded term (all Protobuf types are messages, and we also have `message Message`). Use `types.proto` as a catch-all, and otherwise name files by conceptual grouping instead of message kind.
## Description
This PR removes simple prefix from all types in the crypto/merkle directory.
The two proto types `Proof` & `ProofOp` have been moved to the `proto/crypto/merkle` directory.
proto messge `Proof` was renamed to `ProofOps` and `SimpleProof` message to `Proof`.
Closes: #2755
Closes#4603
Commands used (VIM):
```
:args `rg -l errors.Wrap`
:argdo normal @q | update
```
where q is a macros rewriting the `errors.Wrap` to `fmt.Errorf`.
Fixes#2593@alexanderbez This is used in a single place in the SDK, how upset are you about removing it?
______
For contributor use:
- [x] ~Wrote tests~
- [x] Updated CHANGELOG_PENDING.md
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [x] Updated relevant documentation (`docs/`) and code comments
- [x] Re-reviewed `Files changed` in the Github PR explorer
- [x] Applied Appropriate Labels
* format: add format cmd & goimport repo
- replaced format command
- added goimports to format command
- ran goimports
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* fix outliers & undo proto file changes
* proto: minor linting
minor linting after working with the proto files in the sdk.
there is no logic change just spacing fixes
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* hardcore linting
* proto: add buf and protogen script
- add buf with minimal changes
- add protogen script to easier generate proto files
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* add protoc needs
* add some needed shell cmds
* remove buf from tools as it is not needed everytime
* add proto lint and breakage to ci
* add section in changelog and upgrading files
* address pr comments
* remove space in circle config
* remove spaces in makefile comment
* add section on contributing on how to work with proto
* bump buf to 0.7
* test bufbuild image
* test install make in bufbuild image
* revert to tendermintdev image
* Update Makefile
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
* lint: golint issue fixes
- on my local machine golint is a lot stricter than the bot so slowly going through and fixing things.
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* more fixes from golint
* remove isPeerPersistentFn
* add changelog entry
* libs/common: refactor libs/common 2
- move random function to there own pkg
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* change imports and usage throughout repo
* fix goimports
* add changelog entry
* libs/common: Refactor libs/common 01
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* regenerate proto files, move intslice to where its used
* update kv.KVPair(s) to kv.Pair(s)
* add changelog entry
* make intInSlice private
Fixes#3986
This pull request is prefixing all the types in proto to avoid conflict.
When a go application is using Tendermint as a library and also define similar types in gogo proto some conflicts might occur (as types is a common package in go).
By prefixing the types with tendermint, this highly reduces the risk of conflicts.
BREAKING CHANGE.
This modification breaks the ABCI Application endpoint.
What was accessible before with `/types.ABCIApplication/Flush` is now accessible with `/tendermint.abci.types.ABCIApplication/Flush`.
* crypto: expose MaxAunts for documentation purposes
* types: update godoc for new maxes
* docs: make hard-coded limits more explicit
* wal: add todo to clarify max size
* shorten lines in test
* cs: panic only when WAL#WriteSync fails
- modify WAL#Write and WAL#WriteSync to return an error
* fix test
* types: validate Part#Proof
add ValidateBasic to crypto/merkle/SimpleProof
* cs: limit max bit array size and block parts count
* cs: test new limits
* cs: only assert important stuff
* update changelog and bump version to 0.32.7
* fixes after Ethan's review
* align max wal msg and max consensus msg sizes
* fix tests
* fix test
* add change log for 31.11
* Fix long line errors in abci, crypto, and libs packages
* Fix long lines in p2p and rpc packages
* Fix long lines in abci, state, and tools packages
* Fix long lines in behaviour and blockchain packages
* Fix long lines in cmd and config packages
* Begin fixing long lines in consensus package
* Finish fixing long lines in consensus package
* Add lll exclusion for lines containing URLs
* Fix long lines in crypto package
* Fix long lines in evidence package
* Fix long lines in mempool and node packages
* Fix long lines in libs package
* Fix long lines in lite package
* Fix new long line in node package
* Fix long lines in p2p package
* Ignore gocritic warning
* Fix long lines in privval package
* Fix long lines in rpc package
* Fix long lines in scripts package
* Fix long lines in state package
* Fix long lines in tools package
* Fix long lines in types package
* Enable lll linter
* Remove omitempty from *pb.go
- remove omitempty from *pb.go files
- added command to makefile for everytime `make protoc_all` is run
- open question:
- Do we want to further remove omitempty from other places
- https://github.com/tendermint/tendermint/blob/master/rpc/lib/types/types.go#L151
- and other places
ref #3882
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* bring back omitempty to *pb.go
* Update types/tx.go
* custom marshlers
* undo benchmark `omitepmty`
* golangci lint fix
* cleanup comments
* changelog_pending entry
* Pin range scope vars
* Don't disable scopelint
This PR repairs linter errors seen when running the following commands:
golangci-lint run --no-config --disable-all=true --enable=scopelint
Contributes to #3262
* Remove unnecessary type conversions
* Consolidate repeated strings into consts
* Clothe return statements
* Update blockchain/v1/reactor_fsm_test.go
Co-Authored-By: Anton Kaliaev <anton.kalyaev@gmail.com>
This PR repairs linter errors seen when running the following commands:
golangci-lint run --no-config --disable-all=true --enable=unconvert
golangci-lint run --no-config --disable-all=true --enable=goconst
golangci-lint run --no-config --disable-all=true --enable=nakedret
Contributes to #3262
* init of (2/2) common errors
* Remove instances of cmn.Error (2/2)
- Replace usage of cmnError and errorWrap
- ref #3862
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* comment wording
* simplify IsErrXXX functions
* log panic along with stopping the MConnection
* (1/2) of replace errors.go with github.com/pkg/errors
ref #3862
- step one in removing instances of errors.go in favor of github.com/pkg/errors
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* gofmt
* add in /store
* Renamed wire.go to codec.go
- Wire was the previous name of amino
- Codec describes the file better than `wire` & `amino`
Signed-off-by: Marko Baricevic <marbar3778@yahoo.com>
* ide error
* rename amino.go to codec.go
(#2611) had suggested that an iterative version of
SimpleHashFromByteSlice would be faster, presumably because
we can envision some overhead accumulating from stack
frames and function calls. Additionally, a recursive algorithm risks
hitting the stack limit and causing a stack overflow should the tree
be too large.
Provided here is an iterative alternative, a simple test to assert
correctness and a benchmark. On the performance side, there appears to
be no overall difference:
```
BenchmarkSimpleHashAlternatives/recursive-4 20000 77677 ns/op
BenchmarkSimpleHashAlternatives/iterative-4 20000 76802 ns/op
```
On the surface it might seem that the additional overhead is due to
the different allocation patterns of the implementations. The recursive
version uses a single `[][]byte` slices which it then re-slices at each level of the tree.
The iterative version reproduces `[][]byte` once within the function and
then rewrites sub-slices of that array at each level of the tree.
Eexperimenting by modifying the code to simply calculate the
hash and not store the result show little to no difference in performance.
These preliminary results suggest:
1. The performance of the current implementation is pretty good
2. Go has low overhead for recursive functions
3. The performance of the SimpleHashFromByteSlice routine is dominated
by the actual hashing of data
Although this work is in no way exhaustive, point #3 suggests that
optimizations of this routine would need to take an alternative
approach to make significant improvements on the current performance.
Finally, considering that the recursive implementation is easier to
read, it might not be worthwhile to switch to a less intuitive
implementation for so little benefit.
* re-add slice re-writing
* [crypto] Document SimpleHashFromByteSlicesIterative
* not related to linter: remove obsolete constants:
- `Insecure` and `Secure` and type `Security` are not used anywhere
* not related to linter: update example
- NewInsecure was deleted; change example to NewRemoteDB
* address: Binds to all network interfaces (gosec):
- bind to localhost instead of 0.0.0.0
- regenerate test key and cert for this purpose (was valid for ::) and
otherwise we would see:
transport: authentication handshake failed: x509: certificate is
valid for ::, not 127.0.0.1\"
(used https://github.com/google/keytransparency/blob/master/scripts/gen_server_keys.sh
to regenerate certs)
* use sha256 in tests instead of md5; time difference is negligible
* nolint usage of math/rand in test and add comment on its import
- crypto/rand is slower and we do not need sth more secure in tests
* enable linter in circle-ci
* another nolint math/rand in test
* replace another occurrence of md5
* consistent comment about importing math/rand
* Begin simple merkle compatibility PR
* Fix query_test
* Use trillian test vectors
* Change the split point per RFC 6962
* update spec
* refactor innerhash to match spec
* Update changelog
* Address @liamsi's comments
* Write the comment requested by @liamsi
This is a performance regression, but will also spare the types directory
from knowing about RFC 6962, which is a more correct abstraction. For txs
this performance hit will be fixed soon with #2603. For evidence, the
performance impact is negligible due to it being capped at a small number.
Marko
Erik Grinaker
Anton Kaliaev
Anthony
Ethan Buchman
Phil Salant
Sean Braithwaite
Ismail Khoffi
Thane Thomson
Dev Ojha
Jae Kwon