diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index dadfbc51..77d98916 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -35,4 +35,20 @@ jobs: - name: Clippy linting on workspace (host functions only) working-directory: ./rust run: cargo clippy --tests --no-default-features --features host-functions -- -D warnings + + golangci: + name: golangci-lint + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v3 + with: + # ci is set to go1.19 to match developer setups + go-version: 1.19.2 + - uses: actions/checkout@v3 + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version + version: v1.50.0 + working-directory: go \ No newline at end of file diff --git a/go/.golangci.yml b/go/.golangci.yml new file mode 100644 index 00000000..9adf3ee2 --- /dev/null +++ b/go/.golangci.yml @@ -0,0 +1,26 @@ +run: + tests: false + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 5m + +linters: + disable-all: true + enable: + - depguard + - dogsled + - exportloopref + - goconst + - gocritic + - gofumpt + - gosec + - gosimple + - govet + - ineffassign + - misspell + - nakedret + - nolintlint + - staticcheck + - stylecheck + - typecheck + - unconvert + - unused diff --git a/go/Makefile b/go/Makefile index aebfcb20..fe975519 100644 --- a/go/Makefile +++ b/go/Makefile @@ -18,3 +18,24 @@ install-proto-dep: @go install github.com/regen-network/cosmos-proto/protoc-gen-gocosmos +golangci_lint_cmd=golangci-lint +golangci_version=v1.50.0 + +lint: + @echo "--> Running linter" + @go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(golangci_version) + @$(golangci_lint_cmd) run --timeout=10m + +lint-fix: + @echo "--> Running linter" + @go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(golangci_version) + @$(golangci_lint_cmd) run --fix --out-format=tab --issues-exit-code=0 + +.PHONY: lint lint-fix + +format: + @go install mvdan.cc/gofumpt@latest + @go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(golangci_version) + find . -name '*.go' -type f -not -name "*.pb.go" | xargs gofumpt -w -l + $(golangci_lint_cmd) run --fix +.PHONY: format diff --git a/go/ics23.go b/go/ics23.go index 700d0bde..f72f0f9d 100644 --- a/go/ics23.go +++ b/go/ics23.go @@ -1,20 +1,21 @@ -/** +/* +* This implements the client side functions as specified in https://github.com/cosmos/ibc/tree/main/spec/core/ics-023-vector-commitments In particular: - // Assumes ExistenceProof - type verifyMembership = (root: CommitmentRoot, proof: CommitmentProof, key: Key, value: Value) => boolean + // Assumes ExistenceProof + type verifyMembership = (root: CommitmentRoot, proof: CommitmentProof, key: Key, value: Value) => boolean - // Assumes NonExistenceProof - type verifyNonMembership = (root: CommitmentRoot, proof: CommitmentProof, key: Key) => boolean + // Assumes NonExistenceProof + type verifyNonMembership = (root: CommitmentRoot, proof: CommitmentProof, key: Key) => boolean - // Assumes BatchProof - required ExistenceProofs may be a subset of all items proven - type batchVerifyMembership = (root: CommitmentRoot, proof: CommitmentProof, items: Map) => boolean + // Assumes BatchProof - required ExistenceProofs may be a subset of all items proven + type batchVerifyMembership = (root: CommitmentRoot, proof: CommitmentProof, items: Map) => boolean - // Assumes BatchProof - required NonExistenceProofs may be a subset of all items proven - type batchVerifyNonMembership = (root: CommitmentRoot, proof: CommitmentProof, keys: Set) => boolean + // Assumes BatchProof - required NonExistenceProofs may be a subset of all items proven + type batchVerifyNonMembership = (root: CommitmentRoot, proof: CommitmentProof, keys: Set) => boolean We make an adjustment to accept a Spec to ensure the provided proof is in the format of the expected merkle store. This can avoid an range of attacks on fake preimages, as we need to be careful on how to map key, value -> leaf diff --git a/go/ops.go b/go/ops.go index 77045d72..b84dbb78 100644 --- a/go/ops.go +++ b/go/ops.go @@ -13,16 +13,16 @@ import ( _ "crypto/sha512" // adds ripemd160 capability to crypto.RIPEMD160 - _ "golang.org/x/crypto/ripemd160" + _ "golang.org/x/crypto/ripemd160" //nolint:staticcheck ) // Apply will calculate the leaf hash given the key and value being proven func (op *LeafOp) Apply(key []byte, value []byte) ([]byte, error) { if len(key) == 0 { - return nil, errors.New("Leaf op needs key") + return nil, errors.New("leaf op needs key") } if len(value) == 0 { - return nil, errors.New("Leaf op needs value") + return nil, errors.New("leaf op needs value") } pkey, err := prepareLeafData(op.PrehashKey, op.Length, key) if err != nil { @@ -32,8 +32,11 @@ func (op *LeafOp) Apply(key []byte, value []byte) ([]byte, error) { if err != nil { return nil, fmt.Errorf("prehash value, %w", err) } - data := append(op.Prefix, pkey...) + + data := op.Prefix + data = append(data, pkey...) data = append(data, pvalue...) + return doHash(op.Hash, data) } @@ -42,19 +45,19 @@ func (op *LeafOp) CheckAgainstSpec(spec *ProofSpec) error { lspec := spec.LeafSpec if op.Hash != lspec.Hash { - return fmt.Errorf("Unexpected HashOp: %d", op.Hash) + return fmt.Errorf("unexpected HashOp: %d", op.Hash) } if op.PrehashKey != lspec.PrehashKey { - return fmt.Errorf("Unexpected PrehashKey: %d", op.PrehashKey) + return fmt.Errorf("unexpected PrehashKey: %d", op.PrehashKey) } if op.PrehashValue != lspec.PrehashValue { - return fmt.Errorf("Unexpected PrehashValue: %d", op.PrehashValue) + return fmt.Errorf("unexpected PrehashValue: %d", op.PrehashValue) } if op.Length != lspec.Length { - return fmt.Errorf("Unexpected LengthOp: %d", op.Length) + return fmt.Errorf("unexpected LengthOp: %d", op.Length) } if !bytes.HasPrefix(op.Prefix, lspec.Prefix) { - return fmt.Errorf("Leaf Prefix doesn't start with %X", lspec.Prefix) + return fmt.Errorf("leaf Prefix doesn't start with %X", lspec.Prefix) } return nil } @@ -62,29 +65,32 @@ func (op *LeafOp) CheckAgainstSpec(spec *ProofSpec) error { // Apply will calculate the hash of the next step, given the hash of the previous step func (op *InnerOp) Apply(child []byte) ([]byte, error) { if len(child) == 0 { - return nil, errors.New("Inner op needs child value") + return nil, errors.New("inner op needs child value") } - preimage := append(op.Prefix, child...) + + preimage := op.Prefix + preimage = append(preimage, child...) preimage = append(preimage, op.Suffix...) + return doHash(op.Hash, preimage) } // CheckAgainstSpec will verify the InnerOp is in the format defined in spec func (op *InnerOp) CheckAgainstSpec(spec *ProofSpec) error { if op.Hash != spec.InnerSpec.Hash { - return fmt.Errorf("Unexpected HashOp: %d", op.Hash) + return fmt.Errorf("unexpected HashOp: %d", op.Hash) } leafPrefix := spec.LeafSpec.Prefix if bytes.HasPrefix(op.Prefix, leafPrefix) { - return fmt.Errorf("Inner Prefix starts with %X", leafPrefix) + return fmt.Errorf("inner Prefix starts with %X", leafPrefix) } if len(op.Prefix) < int(spec.InnerSpec.MinPrefixLength) { - return fmt.Errorf("InnerOp prefix too short (%d)", len(op.Prefix)) + return fmt.Errorf("innerOp prefix too short (%d)", len(op.Prefix)) } maxLeftChildBytes := (len(spec.InnerSpec.ChildOrder) - 1) * int(spec.InnerSpec.ChildSize) if len(op.Prefix) > int(spec.InnerSpec.MaxPrefixLength)+maxLeftChildBytes { - return fmt.Errorf("InnerOp prefix too long (%d)", len(op.Prefix)) + return fmt.Errorf("innerOp prefix too long (%d)", len(op.Prefix)) } return nil } @@ -137,7 +143,7 @@ func doHash(hashOp HashOp, preimage []byte) ([]byte, error) { hash.Write(preimage) return hash.Sum(nil), nil } - return nil, fmt.Errorf("Unsupported hashop: %d", hashOp) + return nil, fmt.Errorf("unsupported hashop: %d", hashOp) } // doLengthOp will calculate the proper prefix and return it prepended @@ -152,12 +158,12 @@ func doLengthOp(lengthOp LengthOp, data []byte) ([]byte, error) { return res, nil case LengthOp_REQUIRE_32_BYTES: if len(data) != 32 { - return nil, fmt.Errorf("Data was %d bytes, not 32", len(data)) + return nil, fmt.Errorf("data was %d bytes, not 32", len(data)) } return data, nil case LengthOp_REQUIRE_64_BYTES: if len(data) != 64 { - return nil, fmt.Errorf("Data was %d bytes, not 64", len(data)) + return nil, fmt.Errorf("data was %d bytes, not 64", len(data)) } return data, nil case LengthOp_FIXED32_LITTLE: @@ -171,7 +177,7 @@ func doLengthOp(lengthOp LengthOp, data []byte) ([]byte, error) { // case LengthOp_FIXED64_BIG: // case LengthOp_FIXED64_LITTLE: } - return nil, fmt.Errorf("Unsupported lengthop: %d", lengthOp) + return nil, fmt.Errorf("unsupported lengthop: %d", lengthOp) } func encodeVarintProto(l int) []byte { diff --git a/go/proof.go b/go/proof.go index ea6c3f36..fef9742d 100644 --- a/go/proof.go +++ b/go/proof.go @@ -98,22 +98,21 @@ func (p *ExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []byte } if !bytes.Equal(key, p.Key) { - return fmt.Errorf("Provided key doesn't match proof") + return fmt.Errorf("provided key doesn't match proof") } if !bytes.Equal(value, p.Value) { - return fmt.Errorf("Provided value doesn't match proof") + return fmt.Errorf("provided value doesn't match proof") } calc, err := p.Calculate() if err != nil { - return fmt.Errorf("Error calculating root, %w", err) + return fmt.Errorf("error calculating root, %w", err) } if !bytes.Equal(root, calc) { - return fmt.Errorf("Calculcated root doesn't match provided root") + return fmt.Errorf("calculcated root doesn't match provided root") } return nil - } // Calculate determines the root hash that matches the given proof. @@ -121,7 +120,7 @@ func (p *ExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []byte // Returns error if the calculations cannot be performed. func (p *ExistenceProof) Calculate() (CommitmentRoot, error) { if p.GetLeaf() == nil { - return nil, errors.New("Existence Proof needs defined LeafOp") + return nil, errors.New("existence Proof needs defined LeafOp") } // leaf step takes the key and value as input @@ -151,24 +150,24 @@ func (p *NonExistenceProof) Calculate() (CommitmentRoot, error) { case p.Right != nil: return p.Right.Calculate() default: - return nil, errors.New("Nonexistence proof has empty Left and Right proof") + return nil, errors.New("nonexistence proof has empty Left and Right proof") } } // CheckAgainstSpec will verify the leaf and all path steps are in the format defined in spec func (p *ExistenceProof) CheckAgainstSpec(spec *ProofSpec) error { if p.GetLeaf() == nil { - return errors.New("Existence Proof needs defined LeafOp") + return errors.New("existence Proof needs defined LeafOp") } err := p.Leaf.CheckAgainstSpec(spec) if err != nil { return fmt.Errorf("leaf, %w", err) } if spec.MinDepth > 0 && len(p.Path) < int(spec.MinDepth) { - return fmt.Errorf("InnerOps depth too short: %d", len(p.Path)) + return fmt.Errorf("innerOps depth too short: %d", len(p.Path)) } if spec.MaxDepth > 0 && len(p.Path) > int(spec.MaxDepth) { - return fmt.Errorf("InnerOps depth too long: %d", len(p.Path)) + return fmt.Errorf("innerOps depth too long: %d", len(p.Path)) } for _, inner := range p.Path { @@ -208,25 +207,28 @@ func (p *NonExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []b return errors.New("key is not left of right proof") } } + if leftKey != nil { if bytes.Compare(key, leftKey) <= 0 { return errors.New("key is not right of left proof") } } - if leftKey == nil { + switch { + case leftKey == nil: if !IsLeftMost(spec.InnerSpec, p.Right.Path) { return errors.New("left proof missing, right proof must be left-most") } - } else if rightKey == nil { + case rightKey == nil: if !IsRightMost(spec.InnerSpec, p.Left.Path) { return errors.New("right proof missing, left proof must be right-most") } - } else { // in the middle + default: if !IsLeftNeighbor(spec.InnerSpec, p.Left.Path, p.Right.Path) { return errors.New("right proof missing, left proof must be right-most") } } + return nil } @@ -387,14 +389,14 @@ func rightBranchesAreEmpty(spec *InnerSpec, op *InnerOp) bool { // the index of this branch func getPosition(order []int32, branch int32) int { if branch < 0 || int(branch) >= len(order) { - panic(fmt.Errorf("Invalid branch: %d", branch)) + panic(fmt.Errorf("invalid branch: %d", branch)) } for i, item := range order { if branch == item { return i } } - panic(fmt.Errorf("Branch %d not found in order %v", branch, order)) + panic(fmt.Errorf("branch %d not found in order %v", branch, order)) } // This will look at the proof and determine which order it is... @@ -407,5 +409,5 @@ func orderFromPadding(spec *InnerSpec, inner *InnerOp) (int32, error) { return branch, nil } } - return 0, errors.New("Cannot find any valid spacing for this node") + return 0, errors.New("cannot find any valid spacing for this node") } diff --git a/go/proof_data_test.go b/go/proof_data_test.go index 2f2fca22..449c3bd0 100644 --- a/go/proof_data_test.go +++ b/go/proof_data_test.go @@ -94,10 +94,10 @@ type EmptyBranchTestStruct struct { } func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { - var emptyChild = SpecWithEmptyChild.InnerSpec.EmptyChild + emptyChild := SpecWithEmptyChild.InnerSpec.EmptyChild return []EmptyBranchTestStruct{ - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: append([]byte{1}, emptyChild...), Suffix: nil, @@ -107,7 +107,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsLeft: true, IsRight: false, }, - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: []byte{1}, Suffix: emptyChild, @@ -118,7 +118,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsRight: true, }, // non-empty cases - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: append([]byte{1}, make([]byte, 32)...), Suffix: nil, @@ -128,7 +128,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsLeft: false, IsRight: false, }, - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: []byte{1}, Suffix: make([]byte, 32), @@ -138,7 +138,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsLeft: false, IsRight: false, }, - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: append(append([]byte{1}, emptyChild[0:28]...), []byte("xxxx")...), Suffix: nil, @@ -148,7 +148,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsLeft: false, IsRight: false, }, - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: []byte{1}, Suffix: append(append([]byte(nil), emptyChild[0:28]...), []byte("xxxx")...), @@ -159,7 +159,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsRight: false, }, // some cases using a spec with no empty child - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: append([]byte{1}, make([]byte, 32)...), Suffix: nil, @@ -169,7 +169,7 @@ func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct { IsLeft: false, IsRight: false, }, - EmptyBranchTestStruct{ + { Op: &InnerOp{ Prefix: []byte{1}, Suffix: make([]byte, 32), diff --git a/go/vectors_data_test.go b/go/vectors_data_test.go index ce12b7dc..80fc1d88 100644 --- a/go/vectors_data_test.go +++ b/go/vectors_data_test.go @@ -233,7 +233,7 @@ func loadBatch(t *testing.T, dir string, filename string) (*CommitmentProof, []* t.Fatalf("Unmarshal protobuf: %+v", err) } root := mustHex(t, data.RootHash) - var refs = make([]*RefData, len(data.Items)) + refs := make([]*RefData, len(data.Items)) for i, item := range data.Items { refs[i] = &RefData{ RootHash: root, diff --git a/go/vectors_test.go b/go/vectors_test.go index 4054e4eb..b6e0e909 100644 --- a/go/vectors_test.go +++ b/go/vectors_test.go @@ -106,7 +106,6 @@ func TestDecompressBatchVectors(t *testing.T) { if !bytes.Equal(resmall, small) { t.Fatal("Decompressed batch proof differs from original") } - }) } }