Skip to content

Commit

Permalink
fix: address linting & protobuf issues (#149)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jonathansumner authored Jun 27, 2023
1 parent 487cff7 commit 26432f8
Show file tree
Hide file tree
Showing 24 changed files with 746 additions and 115 deletions.
4 changes: 4 additions & 0 deletions .depguard.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
main:
files:
- "$all"
- "!$test"
44 changes: 31 additions & 13 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ run:
linters:
disable-all: true
enable:
- depguard
# - depguard
- dogsled
- exportloopref
- goconst
Expand All @@ -26,36 +26,54 @@ linters:
- unused

issues:
exclude:
- "directive `//nolint" # check up on this one here
- "directive `// nolint" # check up on this one here
exclude-rules:
- text: "Use of weak random number generator"
linters:
- gosec
- text: "ST1003:"
linters:
- stylecheck
# FIXME: Disabled until golangci-lint updates stylecheck with this fix:
# https://github.com/dominikh/go-tools/issues/389
- text: "ST1016:"
linters:
- stylecheck
- text: "should be written without leading space as"
linters:
- nolintlint
- path: "migrations"
text: "SA1019:"
linters:
- staticcheck

max-issues-per-linter: 10000
max-same-issues: 10000

linters-settings:
nakedret:
max-func-lines: 500
stylecheck:
checks:
- all
- '-ST1016'
- '-ST1003'
staticcheck:
checks:
- all
- '-SA1019'
gocritic:
disabled-checks:
- ifElseChain
- regexpMust
gosec:
excludes:
- G306
- G101
dogsled:
max-blank-identifiers: 3
maligned:
# print struct with more effective memory layout or not, false by default
suggest-new: true
nolintlint:
allow-unused: false
allow-no-explanation:
- errcheck
- misspell
- unparam
- staticcheck
- gocritic
- gosec
- interfacer
require-explanation: false
require-specific: false
2 changes: 2 additions & 0 deletions .markdownlint.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@
"MD036": false,
"MD040": false,
"MD041": false,
"MD051": false,
"MD052": false,
"no-hard-tabs": false
}
24 changes: 19 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ HTTPS_GIT := https://github.com/fetchai/cosmos-sdk.git
DOCKER := $(shell which docker)
DOCKER_BUF := $(DOCKER) run --rm -v $(CURDIR):/workspace --workdir /workspace bufbuild/buf

GO_VERSION := $(shell go version | grep -Eo '[0-9]\.[0-9]+\.[0-9]+')
GO_MAJOR_VERSION := $(shell echo $(GO_VERSION) | cut -d. -f1)
GO_MINOR_VERSION := $(shell echo $(GO_VERSION) | cut -d. -f2)
GO_MICRO_VERSION := $(shell echo $(GO_VERSION) | cut -d. -f3)
SUPPORTED_GO_VERSION = 1.18.10

IS_SUPPORTED_VERSION := $(shell expr "$(GO_VERSION)" "==" "$(SUPPORTED_GO_VERSION)")
ifeq "$(IS_SUPPORTED_VERSION)" "1"
$(info Go version is supported: $(GO_VERSION))
else
$(info WARN: Go version not supported, some tests might fail without version $(SUPPORTED_GO_VERSION))
endif

export GO111MODULE = on

# process build tags
Expand Down Expand Up @@ -315,11 +328,12 @@ containerMarkdownLintFix=cosmos-sdk-markdownlint-fix
golangci_lint_cmd=go run github.com/golangci/golangci-lint/cmd/golangci-lint

lint: lint-go
@if docker ps -a --format '{{.Names}}' | grep -Eq "^${containerMarkdownLint}$$"; then docker start -a $(containerMarkdownLint); else docker run --name $(containerMarkdownLint) -i -v "$(CURDIR):/work" $(markdownLintImage); fi
@#if docker ps -a --format '{{.Names}}' | grep -Eq "^${containerMarkdownLint}$$"; then docker start -a $(containerMarkdownLint); else docker run --name $(containerMarkdownLint) -i -v "$(CURDIR):/work" $(markdownLintImage); fi
docker run -i --rm -v "$(CURDIR):/work" $(containerMarkdownLintImage) .

lint-fix:
$(golangci_lint_cmd) run --fix --out-format=tab --issues-exit-code=0
@if docker ps -a --format '{{.Names}}' | grep -Eq "^${containerMarkdownLintFix}$$"; then docker start -a $(containerMarkdownLintFix); else docker run --name $(containerMarkdownLintFix) -i -v "$(CURDIR):/work" $(markdownLintImage) . --fix; fi
@#if docker ps -a --format '{{.Names}}' | grep -Eq "^${containerMarkdownLintFix}$$"; then docker start -a $(containerMarkdownLintFix); else docker run --name $(containerMarkdownLintFix) -i -v "$(CURDIR):/work" $(markdownLintImage) . --fix; fi
docker run -i --rm -v "$(CURDIR):/work" $(containerMarkdownLintImage) --fix .

lint-go:
echo $(GIT_DIFF)
Expand Down Expand Up @@ -400,7 +414,7 @@ proto-check-breaking:

TM_URL = https://raw.githubusercontent.com/tendermint/tendermint/v0.34.0-rc6/proto/tendermint
GOGO_PROTO_URL = https://raw.githubusercontent.com/regen-network/protobuf/cosmos
COSMOS_PROTO_URL = https://raw.githubusercontent.com/regen-network/cosmos-proto/main
COSMOS_PROTO_URL = https://raw.githubusercontent.com/regen-network/cosmos-proto/master
CONFIO_URL = https://raw.githubusercontent.com/confio/ics23/v0.6.3

TM_CRYPTO_TYPES = third_party/proto/tendermint/crypto
Expand Down Expand Up @@ -452,7 +466,7 @@ proto-update-deps:
@curl -sSL $(CONFIO_URL)/proofs.proto > $(CONFIO_TYPES)/proofs.proto
## insert go package option into proofs.proto file
## Issue link: https://github.com/confio/ics23/issues/32
@sed -i '4ioption go_package = "github.com/confio/ics23/go";' $(CONFIO_TYPES)/proofs.proto
@sed -i'.bak' -r -e 's/^[[:space:]]*(package[[:space:]]+ics23[[:space:]]*;)[[:space:]]*$$/\1 option go_package = "github.com\/confio\/ics23\/go";/' third_party/proto/confio/proofs.proto

.PHONY: proto-all proto-gen proto-gen-any proto-swagger-gen proto-format proto-lint proto-check-breaking proto-update-deps

Expand Down
4 changes: 2 additions & 2 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Cosmos SDK v0.45.9 Release Notes

This is a security release for the
[Dragonberry security advisory](https://forum.cosmos.network/t/ibc-security-advisory-dragonberry/7702).
This is a security release for the
[Dragonberry security advisory](https://forum.cosmos.network/t/ibc-security-advisory-dragonberry/7702).
Please upgrade ASAP.

Next to this, we have also included a few minor bugfixes.
Expand Down
10 changes: 5 additions & 5 deletions RELEASE_PROCESS.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Only the following major release series have a stable release status:
### Patch Releases

Once a Cosmos-SDK release has been completed and published, updates for it are released under certain circumstances
and must follow the [Patch Release Procedure](CONTRIBUTING.md#patch-release-procedure)[Point Release Procedure].
and must follow the [Patch Release Procedure][CONTRIBUTING.md#patch-release-procedure](Point Release Procedure).

### Rationale

Expand Down Expand Up @@ -113,7 +113,7 @@ To smoothen the update to the latest stable release, the SDK includes a set of C

* **High-impact bugs**
* Bugs that may directly cause a security vulnerability.
* *Severe regressions* from a Cosmos-SDK's previous release. This includes all sort of issues
* _Severe regressions_ from a Cosmos-SDK's previous release. This includes all sort of issues
that may cause the core packages or the `x/` modules unusable.
* Bugs that may cause **loss of user's data**.
* Other safe cases:
Expand Down Expand Up @@ -150,10 +150,10 @@ As rule of thumb, the following changes will **NOT** be automatically accepted i
* **State machine changes**.
* **Protobug-breaking changes**, as specified in [ADR-044](./docs/architecture/adr-044-protobuf-updates- guidelines.md).
* **Client-breaking changes**, i.e. changes that prevent gRPC, HTTP and RPC clients to continue interacting with the node without any change.
* **API-breaking changes**, i.e. changes that prevent client applications to *build without modifications* to the client application's source code.
* **API-breaking changes**, i.e. changes that prevent client applications to _build without modifications_ to the client application's source code.
* **CLI-breaking changes**, i.e. changes that require usage changes for CLI users.

In some circumstances, PRs that don't meet the aforementioned criteria might be raised and asked to be granted a *Stable Release Exception*.
In some circumstances, PRs that don't meet the aforementioned criteria might be raised and asked to be granted a _Stable Release Exception_.

### Stable Release Exception - Procedure

Expand All @@ -164,7 +164,7 @@ As rule of thumb, the following changes will **NOT** be automatically accepted i
* A **[Test Case]** section containing detailed instructions on how to reproduce the bug.
* A **[Regression Potential]** section with a clear assessment on how regressions are most likely to manifest as a result of the pull request that aims to fix the bug in the target stable release.

3. **Stable Release Managers** will review and discuss the PR. Once *consensus* surrounding the rationale has been reached and the technical review has successfully concluded, the pull request will be merged in the respective point-release target branch (e.g. `release/v0.43.x`) and the PR included in the point-release's respective milestone (e.g. `v0.43.5`).
3. **Stable Release Managers** will review and discuss the PR. Once _consensus_ surrounding the rationale has been reached and the technical review has successfully concluded, the pull request will be merged in the respective point-release target branch (e.g. `release/v0.43.x`) and the PR included in the point-release's respective milestone (e.g. `v0.43.5`).

#### Stable Release Exception - Bug template

Expand Down
1 change: 0 additions & 1 deletion RELEASING.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ Edit it and:
- Tick the `This is a pre-release` box (until mainnet release)
- Hit `Publish release`


## Next steps

To use this new release, `fetchd` needs to be updated.
Expand Down
2 changes: 1 addition & 1 deletion baseapp/testutil/buf.gen.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version: v1
version: v1beta1
plugins:
- name: gocosmos
out: ../..
Expand Down
2 changes: 1 addition & 1 deletion baseapp/testutil/buf.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version: v1
version: v1beta1
deps:
- buf.build/cosmos/gogo-proto
- buf.build/cosmos/cosmos-proto
50 changes: 24 additions & 26 deletions baseapp/testutil/messages.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion baseapp/testutil/messages.proto
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
syntax = "proto3";
package testdata;

import "gogoproto/gogo.proto";
import "google/protobuf/any.proto";

option go_package = "github.com/cosmos/cosmos-sdk/baseapp/testutil";
Expand Down
59 changes: 30 additions & 29 deletions crypto/keys/secp256k1/internal/secp256k1/libsecp256k1/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Optimized C library for EC operations on curve secp256k1.
This library is a work in progress and is being used to research best practices. Use at your own risk.

Features:

* secp256k1 ECDSA signing/verification and key generation.
* Adding/multiplying private/public keys.
* Serialization/parsing of private keys, public keys, signatures.
Expand All @@ -19,43 +20,43 @@ Implementation details
----------------------

* General
* No runtime heap allocation.
* Extensive testing infrastructure.
* Structured to facilitate review and analysis.
* Intended to be portable to any system with a C89 compiler and uint64_t support.
* Expose only higher level interfaces to minimize the API surface and improve application security. ("Be difficult to use insecurely.")
* No runtime heap allocation.
* Extensive testing infrastructure.
* Structured to facilitate review and analysis.
* Intended to be portable to any system with a C89 compiler and uint64_t support.
* Expose only higher level interfaces to minimize the API surface and improve application security. ("Be difficult to use insecurely.")
* Field operations
* Optimized implementation of arithmetic modulo the curve's field size (2^256 - 0x1000003D1).
* Using 5 52-bit limbs (including hand-optimized assembly for x86_64, by Diederik Huys).
* Using 10 26-bit limbs.
* Field inverses and square roots using a sliding window over blocks of 1s (by Peter Dettman).
* Optimized implementation of arithmetic modulo the curve's field size (2^256 - 0x1000003D1).
* Using 5 52-bit limbs (including hand-optimized assembly for x86_64, by Diederik Huys).
* Using 10 26-bit limbs.
* Field inverses and square roots using a sliding window over blocks of 1s (by Peter Dettman).
* Scalar operations
* Optimized implementation without data-dependent branches of arithmetic modulo the curve's order.
* Using 4 64-bit limbs (relying on __int128 support in the compiler).
* Using 8 32-bit limbs.
* Optimized implementation without data-dependent branches of arithmetic modulo the curve's order.
* Using 4 64-bit limbs (relying on __int128 support in the compiler).
* Using 8 32-bit limbs.
* Group operations
* Point addition formula specifically simplified for the curve equation (y^2 = x^3 + 7).
* Use addition between points in Jacobian and affine coordinates where possible.
* Use a unified addition/doubling formula where necessary to avoid data-dependent branches.
* Point/x comparison without a field inversion by comparison in the Jacobian coordinate space.
* Point addition formula specifically simplified for the curve equation (y^2 = x^3 + 7).
* Use addition between points in Jacobian and affine coordinates where possible.
* Use a unified addition/doubling formula where necessary to avoid data-dependent branches.
* Point/x comparison without a field inversion by comparison in the Jacobian coordinate space.
* Point multiplication for verification (a*P + b*G).
* Use wNAF notation for point multiplicands.
* Use a much larger window for multiples of G, using precomputed multiples.
* Use Shamir's trick to do the multiplication with the public key and the generator simultaneously.
* Optionally (off by default) use secp256k1's efficiently-computable endomorphism to split the P multiplicand into 2 half-sized ones.
* Use wNAF notation for point multiplicands.
* Use a much larger window for multiples of G, using precomputed multiples.
* Use Shamir's trick to do the multiplication with the public key and the generator simultaneously.
* Optionally (off by default) use secp256k1's efficiently-computable endomorphism to split the P multiplicand into 2 half-sized ones.
* Point multiplication for signing
* Use a precomputed table of multiples of powers of 16 multiplied with the generator, so general multiplication becomes a series of additions.
* Access the table with branch-free conditional moves so memory access is uniform.
* No data-dependent branches
* The precomputed tables add and eventually subtract points for which no known scalar (private key) is known, preventing even an attacker with control over the private key used to control the data internally.
* Use a precomputed table of multiples of powers of 16 multiplied with the generator, so general multiplication becomes a series of additions.
* Access the table with branch-free conditional moves so memory access is uniform.
* No data-dependent branches
* The precomputed tables add and eventually subtract points for which no known scalar (private key) is known, preventing even an attacker with control over the private key used to control the data internally.

Build steps
-----------

libsecp256k1 is built using autotools:

$ ./autogen.sh
$ ./configure
$ make
$ ./tests
$ sudo make install # optional
./autogen.sh
./configure
make
./tests
sudo make install # optional
Loading

0 comments on commit 26432f8

Please sign in to comment.