The code in the Tendermint repository makes heavy use of import aliasing.
This is made necessary by our extensive reuse of common base package names, and
by repetition of similar names across different subdirectories.
Unfortunately we have not been very consistent about which packages we alias in
various circumstances, and the aliases we use vary. In the spirit of the advice
in the style guide and https://github.com/golang/go/wiki/CodeReviewComments#imports,
his change makes an effort to clean up and normalize import aliasing.
This change makes no API or behavioral changes. It is a pure cleanup intended
o help make the code more readable to developers (including myself) trying to
understand what is being imported where.
Only unexported names have been modified, and the changes were generated and
applied mechanically with gofmt -r and comby, respecting the lexical and
syntactic rules of Go. Even so, I did not fix every inconsistency. Where the
changes would be too disruptive, I left it alone.
The principles I followed in this cleanup are:
- Remove aliases that restate the package name.
- Remove aliases where the base package name is unambiguous.
- Move overly-terse abbreviations from the import to the usage site.
- Fix lexical issues (remove underscores, remove capitalization).
- Fix import groupings to more closely match the style guide.
- Group blank (side-effecting) imports and ensure they are commented.
- Add aliases to multiple imports with the same base package name.
Update those break statements inside case clauses that are intended to reach an
enclosing for loop, so that they correctly exit the loop.
The candidate files for this change were located using:
% staticcheck -checks SA4011 ./... | cut -d: -f-2
This change is intended to preserve the intended semantics of the code, but
since the code as-written did not have its intended effect, some behaviour may
change. Specifically: Some loops may have run longer than they were supposed
to, prior to this change.
In one case I was not able to clearly determine the intended outcome. That case
has been commented but otherwise left as-written.
Fixes#6780.
At Oasis we have spend some time writing a new Ed25519/X25519/sr25519 implementation called curve25519-voi. This PR switches the import from ed25519consensus/go-schnorrkel, which should lead to performance gains on most systems.
Summary of changes:
* curve25519-voi is now used for Ed25519 operations, following the existing ZIP-215 semantics.
* curve25519-voi's public key cache is enabled (hardcoded size of 4096 entries, should be tuned, see the code comment) to accelerate repeated Ed25519 verification with the same public key(s).
* (BREAKING) curve25519-voi is now used for sr25519 operations. This is a breaking change as the current sr25519 support does something decidedly non-standard when going from a MiniSecretKey to a SecretKey and or PublicKey (The expansion routine is called twice). While I believe the new behavior (that expands once and only once) to be more "correct", this changes the semantics as implemented.
* curve25519-voi is now used for merlin since the included STROBE implementation produces much less garbage on the heap.
Side issues fixed:
* The version of go-schnorrkel that is currently imported by tendermint has a badly broken batch verification implementation. Upstream has fixed the issue after I reported it, so the version should be bumped in the interim.
Open design questions/issues:
* As noted, the public key cache size should be tuned. It is currently backed by a trivial thread-safe LRU cache, which is not scan-resistant, but replacing it with something better is a matter of implementing an interface.
* As far as I can tell, the only reason why serial verification on batch failure is necessary is to provide more detailed error messages (that are only used in some unit tests). If you trust the batch verification to be consistent with serial verification then the fallback can be eliminated entirely (the BatchVerifier provided by the new library supports an option that omits the fallback if this is chosen as the way forward).
* curve25519-voi's sr25519 support could use more optimization and more eyes on the code. The algorithm unfortunately is woefully under-specified, and the implementation was done primarily because I got really sad when I actually looked at go-schnorrkel, and we do not use the algorithm at this time.
## Description
Internalize some libs. This reduces the amount ot public API tendermint is supporting. The moved libraries are mainly ones that are used within Tendermint-core.
## Description
In blocks_results we use the proto definition of abciResponses: 2672b91ab0/rpc/core/blocks.go (L152-L155,) this leads to the use of the proto definition of the pubkey which is an interface in go (oneof). The interface must be registered with the JSON encoder to have it work correctly.
A clearer divide between proto types and native types is needed.
Closes: #XXX
## Description
Remove secp256k1 as discussed in the tendermint dev call. The implementation has been moved to the [Cosmos-SDK](443e0c1f89/crypto/keys/secp256k1)
Closes: #XXX
## Description
This PR aims to make the crypto.PubKey interface more intuitive.
Changes:
- `VerfiyBytes` -> `VerifySignature`
Before `Bytes()` was amino encoded, now since it is the byte representation should we get rid of it entirely?
EDIT: decided to keep `Bytes()` as it is useful if you are using the interface instead of the concrete key
Closes: #XXX
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?
## Description
This Pr changes `GenPrivKeySecp256k1` to `GenPrivKeyFromSecret` to be consistent with the other keys. Also the previous name was not descriptive on what it did.
Closes: #XXX
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
partially cleanup in preparation for errcheck
i ignored a bunch of defer errors in tests but with the update to go 1.14 we can use `t.Cleanup(func() { if err := <>; err != nil {..}}` to cover those errors, I will do this in pr number two of enabling errcheck.
ref #5059
## 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
Migrates the `rpc` package to use new JSON encoder in #4955. Branched off of that PR.
Tests pass, but I haven't done any manual testing beyond that. This should be handled as part of broader 0.34 testing.
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`.
Callum Waters
M. J. Fromberger
Sam Kleinman
Yawning Angel
Marko
Cuong Manh Le
Erik Grinaker
Anton Kaliaev