Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: enable go-critic #3000

Merged
merged 7 commits into from
Jan 23, 2024
Merged

refactor: enable go-critic #3000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b0a217e
fix: adding gocritics and fixing lint errors
moldis Jan 10, 2024
0a3d3d1
fix: gocritic more fixes
moldis Jan 11, 2024
74f4007
Merge branch 'main' into fix/go-critics-support
nodersteam Jan 11, 2024
771f569
more lint updates
moldis Jan 12, 2024
5a466bc
Merge branch 'celestiaorg:main' into fix/go-critics-support
nodersteam Jan 15, 2024
6ff49d8
Merge branch 'main' into fix/go-critics-support
rootulp Jan 21, 2024
6767e70
Merge branch 'main' into fix/go-critics-support
evan-forbes Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ linters:
- prealloc
- stylecheck
- unconvert
- gocritic
moldis marked this conversation as resolved.
Show resolved Hide resolved

linters-settings:
nakedret:
Expand Down
3 changes: 2 additions & 1 deletion app/test/prepare_proposal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) {
accnts[numBlobTxs:],
testutil.ChainID,
)
txs := append(blobTxs, coretypes.Txs(normalTxs).ToSliceOfBytes()...)
txs := blobTxs
txs = append(txs, coretypes.Txs(normalTxs).ToSliceOfBytes()...)

height := testApp.LastBlockHeight() + 1
blockTime := time.Now()
Expand Down
2 changes: 1 addition & 1 deletion pkg/da/data_availability_header.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func SquareSize(len int) int {
func RoundUpPowerOfTwo[I constraints.Integer](input I) I {
var result I = 1
for result < input {
result = result << 1
result <<= 1
}
return result
}
6 changes: 3 additions & 3 deletions pkg/inclusion/commitment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func CreateCommitment(blob *blob.Blob) ([]byte, error) {
cursor := uint64(0)
for i, treeSize := range treeSizes {
leafSets[i] = appshares.ToBytes(shares[cursor : cursor+treeSize])
cursor = cursor + treeSize
cursor += treeSize
}

// create the commitments by pushing each leaf set onto an nmt
Expand Down Expand Up @@ -99,14 +99,14 @@ func MerkleMountainRangeSizes(totalSize, maxTreeSize uint64) ([]uint64, error) {
switch {
case totalSize >= maxTreeSize:
treeSizes = append(treeSizes, maxTreeSize)
totalSize = totalSize - maxTreeSize
totalSize -= maxTreeSize
case totalSize < maxTreeSize:
treeSize, err := appshares.RoundDownPowerOfTwo(totalSize)
if err != nil {
return treeSizes, err
}
treeSizes = append(treeSizes, treeSize)
totalSize = totalSize - treeSize
totalSize -= treeSize
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/inclusion/nmt_caching_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func TestEDSSubRootCacher(t *testing.T) {
func calculateSubTreeRoots(t *testing.T, row [][]byte, depth int) [][]byte {
subLeafRange := len(row)
for i := 0; i < depth; i++ {
subLeafRange = subLeafRange / 2
subLeafRange /= 2
}

if subLeafRange == 0 || subLeafRange%2 != 0 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/inclusion/paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ func calculateSubTreeRootCoordinates(maxDepth, minDepth, start, end int) []coord
default:
lastLeafCursor = leafCursor
lastNodeCursor = nodeCursor
leafCursor = leafCursor + nodeRangeCursor
nodeRangeCursor = nodeRangeCursor * 2
leafCursor += nodeRangeCursor
nodeRangeCursor *= 2
nodeCursor = nodeCursor.climb()
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/inclusion/paths_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,9 @@ func pathToString(p path) string {
for _, wi := range p.instructions {
switch wi {
case WalkLeft:
s = s + "l"
s += "l"
case WalkRight:
s = s + "r"
s += "r"
}
}
return s
Expand Down
2 changes: 1 addition & 1 deletion pkg/shares/powers_of_two.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
func RoundUpPowerOfTwo[I constraints.Integer](input I) I {
var result I = 1
for result < input {
result = result << 1
result <<= 1
}
return result
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/shares/share_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (b *Builder) FlipSequenceStart() {

// the sequence start indicator is the last bit of the info byte so flip the
// last bit
b.rawShareData[infoByteIndex] = b.rawShareData[infoByteIndex] ^ 0x01
b.rawShareData[infoByteIndex] ^= 0x01
}

func (b *Builder) prepareCompactShare() error {
Expand Down
10 changes: 6 additions & 4 deletions pkg/shares/shares_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@ func TestSequenceLen(t *testing.T) {
wantLen uint32
wantErr bool
}
sparseNamespaceID := bytes.Repeat([]byte{1}, appconsts.NamespaceSize)
firstShare := append(sparseNamespaceID,
firstShare := append(bytes.Repeat([]byte{1},
appconsts.NamespaceSize),
[]byte{
1, // info byte
0, 0, 0, 10, // sequence len
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
}...)
firstShareWithLongSequence := append(sparseNamespaceID,
firstShareWithLongSequence := append(bytes.Repeat([]byte{1},
appconsts.NamespaceSize),
[]byte{
1, // info byte
0, 0, 1, 67, // sequence len
}...)
continuationShare := append(sparseNamespaceID,
continuationShare := append(bytes.Repeat([]byte{1},
appconsts.NamespaceSize),
[]byte{
0, // info byte
}...)
Expand Down
2 changes: 1 addition & 1 deletion pkg/square/square.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func WriteSquare(
}

square := make([]shares.Share, totalShares)
copy(square[:], txShares)
copy(square, txShares)
copy(square[pfbStartIndex:], pfbShares)
if blobWriter.Count() > 0 {
copy(square[paddingStartIndex:], padding)
Expand Down
3 changes: 2 additions & 1 deletion pkg/square/square_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ func TestSquareConstruction(t *testing.T) {
sendTxs := blobfactory.GenerateManyRawSendTxs(signer, 250)
pfbTxs := blobfactory.RandBlobTxs(signer, rand, 10000, 1, 1024)
t.Run("normal transactions after PFB transactions", func(t *testing.T) {
txs := append(sendTxs[:5], append(pfbTxs, sendTxs[5:]...)...)
txs := sendTxs[:5]
txs = append(txs, append(pfbTxs, sendTxs[5:]...)...)
_, err := square.Construct(coretypes.Txs(txs).ToSliceOfBytes(), appconsts.LatestVersion, appconsts.DefaultSquareSizeUpperBound)
require.Error(t, err)
})
Expand Down
3 changes: 1 addition & 2 deletions x/blob/client/testutil/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,7 @@ func (s *IntegrationTestSuite) TestSubmitPayForBlob() {

events := txResp.Logs[0].GetEvents()
for _, e := range events {
switch e.Type {
case types.EventTypePayForBlob:
if e.Type == types.EventTypePayForBlob {
signer := e.GetAttributes()[0].GetValue()
_, err = sdk.AccAddressFromBech32(signer)
require.NoError(err)
Expand Down
6 changes: 4 additions & 2 deletions x/blob/types/payforblob_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ func totalBlobSize(size int) int {
func validMsgPayForBlobs(t *testing.T) *types.MsgPayForBlobs {
signer, err := testnode.NewOfflineSigner()
require.NoError(t, err)
ns1 := append(appns.NamespaceVersionZeroPrefix, bytes.Repeat([]byte{0x01}, appns.NamespaceVersionZeroIDSize)...)
ns1 := appns.NamespaceVersionZeroPrefix
ns1 = append(ns1, bytes.Repeat([]byte{0x01}, appns.NamespaceVersionZeroIDSize)...)
data := bytes.Repeat([]byte{2}, totalBlobSize(appconsts.ContinuationSparseShareContentSize*12))

pblob := &blob.Blob{
Expand All @@ -188,7 +189,8 @@ func validMsgPayForBlobs(t *testing.T) *types.MsgPayForBlobs {
func invalidNamespaceVersionMsgPayForBlobs(t *testing.T) *types.MsgPayForBlobs {
signer, err := testnode.NewOfflineSigner()
require.NoError(t, err)
ns1 := append(appns.NamespaceVersionZeroPrefix, bytes.Repeat([]byte{0x01}, appns.NamespaceVersionZeroIDSize)...)
ns1 := appns.NamespaceVersionZeroPrefix
ns1 = append(ns1, bytes.Repeat([]byte{0x01}, appns.NamespaceVersionZeroIDSize)...)
data := bytes.Repeat([]byte{2}, totalBlobSize(appconsts.ContinuationSparseShareContentSize*12))

pblob := &blob.Blob{
Expand Down
2 changes: 1 addition & 1 deletion x/blobstream/types/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (ibv InternalBridgeValidators) Sort() {

// EVMAddrLessThan migrates the EVM address less than function.
func EVMAddrLessThan(e common.Address, o common.Address) bool {
return bytes.Compare([]byte(e.Hex())[:], []byte(o.Hex())[:]) == -1
return bytes.Compare([]byte(e.Hex()), []byte(o.Hex())) == -1
}

// PowerDiff returns the difference in power between two bridge validator sets.
Expand Down
7 changes: 2 additions & 5 deletions x/upgrade/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@ type Fraction struct {
// between 2/3 and 3/3 providing 1/6 fault tolerance to halting the
// network during an upgrade period. It can be modified through a
// hard fork change that modified the app version
func SignalThreshold(version uint64) Fraction {
switch version {
default:
return defaultSignalTheshold
}
func SignalThreshold(_ uint64) Fraction {
return defaultSignalTheshold
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The change to the SignalThreshold function in x/upgrade/keeper.go to always return defaultSignalThreshold and ignore the version parameter could affect the upgrade logic, as the GetVotingPowerThreshold function relies on it to calculate the voting power threshold for upgrades. This threshold previously could vary with the app version, which is no longer the case after the change.

Analysis chain

The SignalThreshold function has been simplified to always return defaultSignalTheshold. This change removes the parameter and the switch statement, which is consistent with the summary. However, it's important to ensure that this change does not affect any other part of the system that might rely on the removed logic.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of SignalThreshold to ensure the change doesn't affect other parts of the system.
rg --type go "SignalThreshold"

Length of output: 269

Script:

#!/bin/bash
# Search for any logic in the keeper.go file that might have depended on the version parameter in SignalThreshold.
rg --type go -C 10 "SignalThreshold" x/upgrade/keeper.go

Length of output: 1761

}

type Keeper struct {
Expand Down
Loading