From e8e042fdc8dcf485510c093a9c831f5b2fac2d3b Mon Sep 17 00:00:00 2001 From: Rootul P Date: Tue, 4 Jul 2023 16:09:09 -0400 Subject: [PATCH] feat!: enable golangci-lint (#208) Closes https://github.com/celestiaorg/rsmt2d/issues/204 Marked as breaking b/c the function signature for exported functions (`RowRoots` and `ColRoots`) was modified to include an error return parameter. ## Testing 1. `golangci-lint run` passes locally 2. `golangci-lint run` passes in CI [here](https://github.com/celestiaorg/rsmt2d/actions/runs/5456354873/jobs/9929053344?pr=208#step:4:27) --- .github/workflows/lint.yml | 28 +++++++++++++++++ .golangci.yml | 17 ++++++++++ README.md | 14 +++++++-- datasquare.go | 28 ++++++++++++----- datasquare_test.go | 59 ++++++++++++++++++----------------- extendeddatacrossword.go | 39 ++++++++++++++++++----- extendeddatacrossword_test.go | 43 +++++++++++++++++++------ extendeddatasquare.go | 20 ++++++++---- extendeddatasquare_test.go | 57 ++++++++++++++++++++------------- leopard.go | 19 ++++++----- rsmt2d_test.go | 16 +++++++--- tree.go | 2 +- 12 files changed, 244 insertions(+), 98 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..5366ce8 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,28 @@ +# lint runs golangci-lint +name: lint + +on: + pull_request: + push: + branches: + - master + - release/** + +env: + GO_VERSION: '1.20' + +jobs: + golangci-lint: + name: golangci-lint + runs-on: ubuntu-latest + timeout-minutes: 8 + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: ${{ env.GO_VERSION }} + - uses: golangci/golangci-lint-action@v3.6.0 + with: + version: v1.52.2 + args: --timeout 10m + github-token: ${{ secrets.github_token }} diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..371f88b --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,17 @@ +run: + timeout: 5m + modules-download-mode: readonly + +linters: + enable: + - exportloopref + - gofumpt + - misspell + - nakedret + - revive + - prealloc + +linters-settings: + nakedret: + # Ban the use of naked returns because they reduce code readability. + max-func-lines: 0 # override the default: 30 diff --git a/README.md b/README.md index c27e73d..2f70add 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,20 @@ func main() { } ``` -## Building From Source +## Contributing -Run benchmarks +1. [Install Go](https://go.dev/doc/install) 1.19+ +1. [Install golangci-lint](https://golangci-lint.run/usage/install/) + +### Helpful Commands ```sh +# Run unit tests +go test ./... + +# Run benchmarks go test -benchmem -bench=. + +# Run linter +golangci-lint run ``` diff --git a/datasquare.go b/datasquare.go index 5105a10..4a5ca0c 100644 --- a/datasquare.go +++ b/datasquare.go @@ -235,12 +235,15 @@ func (ds *dataSquare) computeRoots() error { } // getRowRoots returns the Merkle roots of all the rows in the square. -func (ds *dataSquare) getRowRoots() [][]byte { +func (ds *dataSquare) getRowRoots() ([][]byte, error) { if ds.rowRoots == nil { - ds.computeRoots() + err := ds.computeRoots() + if err != nil { + return nil, err + } } - return ds.rowRoots + return ds.rowRoots, nil } // getRowRoot calculates and returns the root of the selected row. Note: unlike the @@ -252,19 +255,25 @@ func (ds *dataSquare) getRowRoot(x uint) ([]byte, error) { tree := ds.createTreeFn(Row, x) for _, d := range ds.row(x) { - tree.Push(d) + err := tree.Push(d) + if err != nil { + return nil, err + } } return tree.Root() } // getColRoots returns the Merkle roots of all the columns in the square. -func (ds *dataSquare) getColRoots() [][]byte { +func (ds *dataSquare) getColRoots() ([][]byte, error) { if ds.colRoots == nil { - ds.computeRoots() + err := ds.computeRoots() + if err != nil { + return nil, err + } } - return ds.colRoots + return ds.colRoots, nil } // getColRoot calculates and returns the root of the selected row. Note: unlike the @@ -276,7 +285,10 @@ func (ds *dataSquare) getColRoot(y uint) ([]byte, error) { tree := ds.createTreeFn(Col, y) for _, d := range ds.col(y) { - tree.Push(d) + err := tree.Push(d) + if err != nil { + return nil, err + } } return tree.Root() diff --git a/datasquare_test.go b/datasquare_test.go index f816a65..4771b74 100644 --- a/datasquare_test.go +++ b/datasquare_test.go @@ -186,14 +186,19 @@ func TestInvalidSquareExtension(t *testing.T) { } } +// TestRoots verifies that the row roots and column roots are equal for a 1x1 +// square. func TestRoots(t *testing.T) { result, err := newDataSquare([][]byte{{1, 2}}, NewDefaultTree) - if err != nil { - panic(err) - } - if !reflect.DeepEqual(result.getRowRoots(), result.getColRoots()) { - t.Errorf("computing roots failed; expecting row and column roots for 1x1 square to be equal") - } + assert.NoError(t, err) + + rowRoots, err := result.getRowRoots() + assert.NoError(t, err) + + colRoots, err := result.getColRoots() + assert.NoError(t, err) + + assert.Equal(t, rowRoots, colRoots) } func TestLazyRootGeneration(t *testing.T) { @@ -245,22 +250,19 @@ func TestRootAPI(t *testing.T) { for i := uint(0); i < square.width; i++ { rowRoot, err := square.getRowRoot(i) assert.NoError(t, err) - if !reflect.DeepEqual(square.getRowRoots()[i], rowRoot) { - t.Errorf( - "Row root API results in different roots, expected %v got %v", - square.getRowRoots()[i], - rowRoot, - ) - } + + rowRoots, err := square.getRowRoots() + assert.NoError(t, err) + + assert.Equal(t, rowRoots[i], rowRoot) + colRoot, err := square.getColRoot(i) assert.NoError(t, err) - if !reflect.DeepEqual(square.getColRoots()[i], colRoot) { - t.Errorf( - "Column root API results in different roots, expected %v got %v", - square.getColRoots()[i], - colRoot, - ) - } + + colRoots, err := square.getColRoots() + assert.NoError(t, err) + + assert.Equal(t, colRoots[i], colRoot) } } @@ -335,10 +337,9 @@ func Test_setRowSlice(t *testing.T) { if tc.wantErr { assert.Error(t, err) return - } else { - assert.NoError(t, err) - assert.Equal(t, tc.want, ds.Flattened()) } + assert.NoError(t, err) + assert.Equal(t, tc.want, ds.Flattened()) } } @@ -392,10 +393,9 @@ func Test_setColSlice(t *testing.T) { if tc.wantErr { assert.Error(t, err) return - } else { - assert.NoError(t, err) - assert.Equal(t, tc.want, ds.Flattened()) } + assert.NoError(t, err) + assert.Equal(t, tc.want, ds.Flattened()) } } @@ -423,7 +423,10 @@ func computeRowProof(ds *dataSquare, x uint, y uint) ([]byte, [][]byte, uint, ui data := ds.row(x) for i := uint(0); i < ds.width; i++ { - tree.Push(data[i]) + err := tree.Push(data[i]) + if err != nil { + return nil, nil, 0, 0, err + } } merkleRoot, proof, proofIndex, numLeaves := treeProve(tree.(*DefaultTree), int(y)) @@ -445,7 +448,7 @@ type errorTree struct { leaves [][]byte } -func newErrorTree(axis Axis, index uint) Tree { +func newErrorTree(_ Axis, _ uint) Tree { return &errorTree{ Tree: merkletree.New(sha256.New()), leaves: make([][]byte, 0, 128), diff --git a/extendeddatacrossword.go b/extendeddatacrossword.go index 125c7ff..1928be7 100644 --- a/extendeddatacrossword.go +++ b/extendeddatacrossword.go @@ -178,7 +178,13 @@ func (eds *ExtendedDataSquare) solveCrosswordRow( // Insert rebuilt shares into square. for c, s := range rebuiltShares { - eds.SetCell(uint(r), uint(c), s) + cellToSet := eds.GetCell(uint(r), uint(c)) + if cellToSet == nil { + err := eds.SetCell(uint(r), uint(c), s) + if err != nil { + return false, false, err + } + } } return true, true, nil @@ -204,7 +210,6 @@ func (eds *ExtendedDataSquare) solveCrosswordCol( vectorData := eds.col(uint(c)) for r := 0; r < int(eds.width); r++ { shares[r] = vectorData[r] - } // Attempt rebuild @@ -242,7 +247,13 @@ func (eds *ExtendedDataSquare) solveCrosswordCol( // Insert rebuilt shares into square. for r, s := range rebuiltShares { - eds.SetCell(uint(r), uint(c), s) + cellToSet := eds.GetCell(uint(r), uint(c)) + if cellToSet == nil { + err := eds.SetCell(uint(r), uint(c), s) + if err != nil { + return false, false, err + } + } } return true, true, nil @@ -396,7 +407,10 @@ func noMissingData(input [][]byte, rebuiltIndex int) bool { func (eds *ExtendedDataSquare) computeSharesRoot(shares [][]byte, axis Axis, i uint) ([]byte, error) { tree := eds.createTreeFn(axis, i) for _, d := range shares { - tree.Push(d) + err := tree.Push(d) + if err != nil { + return nil, err + } } return tree.Root() } @@ -404,11 +418,22 @@ func (eds *ExtendedDataSquare) computeSharesRoot(shares [][]byte, axis Axis, i u func (eds *ExtendedDataSquare) computeSharesRootWithRebuiltShare(shares [][]byte, axis Axis, i uint, rebuiltIndex int, rebuiltShare []byte) ([]byte, error) { tree := eds.createTreeFn(axis, i) for _, d := range shares[:rebuiltIndex] { - tree.Push(d) + err := tree.Push(d) + if err != nil { + return nil, err + } + } + + err := tree.Push(rebuiltShare) + if err != nil { + return nil, err } - tree.Push(rebuiltShare) + for _, d := range shares[rebuiltIndex+1:] { - tree.Push(d) + err := tree.Push(d) + if err != nil { + return nil, err + } } return tree.Root() } diff --git a/extendeddatacrossword_test.go b/extendeddatacrossword_test.go index ac59495..bd76ce9 100644 --- a/extendeddatacrossword_test.go +++ b/extendeddatacrossword_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // PseudoFraudProof is an example fraud proof. @@ -34,8 +35,11 @@ func TestRepairExtendedDataSquare(t *testing.T) { name, codec, shareSize := test.name, test.codec, test.shareSize original := createTestEds(codec, shareSize) - rowRoots := original.RowRoots() - colRoots := original.ColRoots() + rowRoots, err := original.RowRoots() + require.NoError(t, err) + + colRoots, err := original.ColRoots() + require.NoError(t, err) // Verify that an EDS can be repaired after the maximum amount of erasures t.Run("MaximumErasures", func(t *testing.T) { @@ -108,7 +112,15 @@ func TestValidFraudProof(t *testing.T) { t.Fatalf("unexpected err while copying original data: %v, codec: :%s", err, name) } corrupted.setCell(0, 0, corruptChunk) - err = corrupted.Repair(corrupted.getRowRoots(), corrupted.getColRoots()) + assert.NoError(t, err) + + rowRoots, err := corrupted.getRowRoots() + assert.NoError(t, err) + + colRoots, err := corrupted.getColRoots() + assert.NoError(t, err) + + err = corrupted.Repair(rowRoots, colRoots) errors.As(err, &byzData) // Construct the fraud proof @@ -155,11 +167,15 @@ func TestCannotRepairSquareWithBadRoots(t *testing.T) { codec, shareSize := test.codec, test.shareSize original := createTestEds(codec, shareSize) - rowRoots := original.RowRoots() - colRoots := original.ColRoots() + rowRoots, err := original.RowRoots() + require.NoError(t, err) + + colRoots, err := original.ColRoots() + require.NoError(t, err) original.setCell(0, 0, corruptChunk) - err := original.Repair(rowRoots, colRoots) + require.NoError(t, err) + err = original.Repair(rowRoots, colRoots) if err == nil { t.Errorf("did not return an error on trying to repair a square with bad roots") } @@ -208,7 +224,13 @@ func TestCorruptedEdsReturnsErrByzantineData(t *testing.T) { y := coords[1] eds.setCell(x, y, test.values[i]) } - err := eds.Repair(eds.getRowRoots(), eds.getColRoots()) + rowRoots, err := eds.getRowRoots() + assert.NoError(t, err) + + colRoots, err := eds.getColRoots() + assert.NoError(t, err) + + err = eds.Repair(rowRoots, colRoots) assert.Error(t, err) // due to parallelisation, the ErrByzantineData axis may be either row or col @@ -237,8 +259,11 @@ func BenchmarkRepair(b *testing.B) { } extendedDataWidth := originalDataWidth * 2 - rowRoots := eds.RowRoots() - colRoots := eds.ColRoots() + rowRoots, err := eds.RowRoots() + assert.NoError(b, err) + + colRoots, err := eds.ColRoots() + assert.NoError(b, err) b.Run( fmt.Sprintf( diff --git a/extendeddatasquare.go b/extendeddatasquare.go index a8a8296..1e71184 100644 --- a/extendeddatasquare.go +++ b/extendeddatasquare.go @@ -185,8 +185,8 @@ func (eds *ExtendedDataSquare) erasureExtendCol(codec Codec, i uint) error { } func (eds *ExtendedDataSquare) deepCopy(codec Codec) (ExtendedDataSquare, error) { - copy, err := ImportExtendedDataSquare(eds.Flattened(), codec, eds.createTreeFn) - return *copy, err + imported, err := ImportExtendedDataSquare(eds.Flattened(), codec, eds.createTreeFn) + return *imported, err } // Col returns a column slice. @@ -196,8 +196,12 @@ func (eds *ExtendedDataSquare) Col(y uint) [][]byte { } // ColRoots returns the Merkle roots of all the columns in the square. -func (eds *ExtendedDataSquare) ColRoots() [][]byte { - return deepCopy(eds.getColRoots()) +func (eds *ExtendedDataSquare) ColRoots() ([][]byte, error) { + colRoots, err := eds.getColRoots() + if err != nil { + return nil, err + } + return deepCopy(colRoots), nil } // Row returns a row slice. @@ -207,8 +211,12 @@ func (eds *ExtendedDataSquare) Row(x uint) [][]byte { } // RowRoots returns the Merkle roots of all the rows in the square. -func (eds *ExtendedDataSquare) RowRoots() [][]byte { - return deepCopy(eds.getRowRoots()) +func (eds *ExtendedDataSquare) RowRoots() ([][]byte, error) { + rowRoots, err := eds.getRowRoots() + if err != nil { + return nil, err + } + return deepCopy(rowRoots), nil } func deepCopy(original [][]byte) [][]byte { diff --git a/extendeddatasquare_test.go b/extendeddatasquare_test.go index f8f1abc..683d9f4 100644 --- a/extendeddatasquare_test.go +++ b/extendeddatasquare_test.go @@ -11,19 +11,19 @@ import ( "github.com/stretchr/testify/assert" ) -const SHARD_SIZE = 64 +const ShardSize = 64 var ( - zeros = bytes.Repeat([]byte{0}, SHARD_SIZE) - ones = bytes.Repeat([]byte{1}, SHARD_SIZE) - twos = bytes.Repeat([]byte{2}, SHARD_SIZE) - threes = bytes.Repeat([]byte{3}, SHARD_SIZE) - fours = bytes.Repeat([]byte{4}, SHARD_SIZE) - fives = bytes.Repeat([]byte{5}, SHARD_SIZE) - eights = bytes.Repeat([]byte{8}, SHARD_SIZE) - elevens = bytes.Repeat([]byte{11}, SHARD_SIZE) - thirteens = bytes.Repeat([]byte{13}, SHARD_SIZE) - fifteens = bytes.Repeat([]byte{15}, SHARD_SIZE) + zeros = bytes.Repeat([]byte{0}, ShardSize) + ones = bytes.Repeat([]byte{1}, ShardSize) + twos = bytes.Repeat([]byte{2}, ShardSize) + threes = bytes.Repeat([]byte{3}, ShardSize) + fours = bytes.Repeat([]byte{4}, ShardSize) + fives = bytes.Repeat([]byte{5}, ShardSize) + eights = bytes.Repeat([]byte{8}, ShardSize) + elevens = bytes.Repeat([]byte{11}, ShardSize) + thirteens = bytes.Repeat([]byte{13}, ShardSize) + fifteens = bytes.Repeat([]byte{15}, ShardSize) ) func TestComputeExtendedDataSquare(t *testing.T) { @@ -106,15 +106,27 @@ func TestImmutableRoots(t *testing.T) { panic(err) } - row := result.RowRoots() - row[0][0]++ - if reflect.DeepEqual(row, result.RowRoots()) { + mutatedRowRoots, err := result.RowRoots() + assert.NoError(t, err) + + mutatedRowRoots[0][0]++ // mutate + + rowRoots, err := result.RowRoots() + assert.NoError(t, err) + + if reflect.DeepEqual(mutatedRowRoots, rowRoots) { t.Errorf("Exported EDS RowRoots was mutable") } - col := result.ColRoots() - col[0][0]++ - if reflect.DeepEqual(col, result.ColRoots()) { + mutatedColRoots, err := result.ColRoots() + assert.NoError(t, err) + + mutatedColRoots[0][0]++ // mutate + + colRoots, err := result.ColRoots() + assert.NoError(t, err) + + if reflect.DeepEqual(mutatedColRoots, colRoots) { t.Errorf("Exported EDS ColRoots was mutable") } } @@ -170,7 +182,6 @@ func BenchmarkExtensionEncoding(b *testing.B) { }, ) } - } } @@ -193,14 +204,13 @@ func BenchmarkExtensionWithRoots(b *testing.B) { if err != nil { b.Error(err) } - _ = eds.RowRoots() - _ = eds.ColRoots() + _, _ = eds.RowRoots() + _, _ = eds.ColRoots() dump = eds } }, ) } - } } @@ -211,7 +221,10 @@ func genRandDS(width int) [][]byte { count := width * width for i := 0; i < count; i++ { share := make([]byte, 256) - rand.Read(share) + _, err := rand.Read(share) + if err != nil { + panic(err) + } ds = append(ds, share) } return ds diff --git a/leopard.go b/leopard.go index cc4138f..390c796 100644 --- a/leopard.go +++ b/leopard.go @@ -6,13 +6,13 @@ import ( "github.com/klauspost/reedsolomon" ) -var _ Codec = &leoRSCodec{} +var _ Codec = &LeoRSCodec{} func init() { registerCodec(Leopard, NewLeoRSCodec()) } -type leoRSCodec struct { +type LeoRSCodec struct { // Cache the encoders of various sizes to not have to re-instantiate those // as it is costly. // @@ -24,7 +24,7 @@ type leoRSCodec struct { encCache sync.Map } -func (l *leoRSCodec) Encode(data [][]byte) ([][]byte, error) { +func (l *LeoRSCodec) Encode(data [][]byte) ([][]byte, error) { dataLen := len(data) enc, err := l.loadOrInitEncoder(dataLen) if err != nil { @@ -43,7 +43,7 @@ func (l *leoRSCodec) Encode(data [][]byte) ([][]byte, error) { return shards[dataLen:], nil } -func (l *leoRSCodec) Decode(data [][]byte) ([][]byte, error) { +func (l *LeoRSCodec) Decode(data [][]byte) ([][]byte, error) { half := len(data) / 2 enc, err := l.loadOrInitEncoder(half) if err != nil { @@ -53,7 +53,7 @@ func (l *leoRSCodec) Decode(data [][]byte) ([][]byte, error) { return data, err } -func (l *leoRSCodec) loadOrInitEncoder(dataLen int) (reedsolomon.Encoder, error) { +func (l *LeoRSCodec) loadOrInitEncoder(dataLen int) (reedsolomon.Encoder, error) { enc, ok := l.encCache.Load(dataLen) if !ok { var err error @@ -64,17 +64,16 @@ func (l *leoRSCodec) loadOrInitEncoder(dataLen int) (reedsolomon.Encoder, error) l.encCache.Store(dataLen, enc) } return enc.(reedsolomon.Encoder), nil - } -func (l *leoRSCodec) MaxChunks() int { +func (l *LeoRSCodec) MaxChunks() int { return 32768 * 32768 } -func (l *leoRSCodec) Name() string { +func (l *LeoRSCodec) Name() string { return Leopard } -func NewLeoRSCodec() *leoRSCodec { - return &leoRSCodec{} +func NewLeoRSCodec() *LeoRSCodec { + return &LeoRSCodec{} } diff --git a/rsmt2d_test.go b/rsmt2d_test.go index e6c004c..011a07b 100644 --- a/rsmt2d_test.go +++ b/rsmt2d_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/celestiaorg/rsmt2d" + "github.com/stretchr/testify/assert" ) func TestEdsRepairRoundtripSimple(t *testing.T) { @@ -39,8 +40,11 @@ func TestEdsRepairRoundtripSimple(t *testing.T) { t.Errorf("ComputeExtendedDataSquare failed: %v", err) } - rowRoots := eds.RowRoots() - colRoots := eds.ColRoots() + rowRoots, err := eds.RowRoots() + assert.NoError(t, err) + + colRoots, err := eds.ColRoots() + assert.NoError(t, err) // Save all shares in flattened form. flattened := make([][]byte, 0, eds.Width()*eds.Width()) @@ -105,8 +109,11 @@ func TestEdsRepairTwice(t *testing.T) { t.Errorf("ComputeExtendedDataSquare failed: %v", err) } - rowRoots := eds.RowRoots() - colRoots := eds.ColRoots() + rowRoots, err := eds.RowRoots() + assert.NoError(t, err) + + colRoots, err := eds.ColRoots() + assert.NoError(t, err) // Save all shares in flattened form. flattened := make([][]byte, 0, eds.Width()*eds.Width()) @@ -155,7 +162,6 @@ func TestEdsRepairTwice(t *testing.T) { // Should now pass, since sufficient data. t.Errorf("RepairExtendedDataSquare failed: %v", err) } - }) } } diff --git a/tree.go b/tree.go index fa9f043..9a56965 100644 --- a/tree.go +++ b/tree.go @@ -30,7 +30,7 @@ type DefaultTree struct { root []byte } -func NewDefaultTree(axis Axis, index uint) Tree { +func NewDefaultTree(_ Axis, _ uint) Tree { return &DefaultTree{ Tree: merkletree.New(sha256.New()), leaves: make([][]byte, 0, 128),