Skip to content

Commit

Permalink
feat!: enable golangci-lint (#208)
Browse files Browse the repository at this point in the history
Closes #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)
  • Loading branch information
rootulp authored Jul 4, 2023
1 parent ed26fa9 commit e8e042f
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 98 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -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 }}
17 changes: 17 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
28 changes: 20 additions & 8 deletions datasquare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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()
Expand Down
59 changes: 31 additions & 28 deletions datasquare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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))
Expand All @@ -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),
Expand Down
39 changes: 32 additions & 7 deletions extendeddatacrossword.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -396,19 +407,33 @@ 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()
}

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()
}
Loading

0 comments on commit e8e042f

Please sign in to comment.