Skip to content

Commit

Permalink
chore: Db linting (#12141)
Browse files Browse the repository at this point in the history
* mainly sdk.int to cosmossdk.io/math

* staking keys

* fumpt

* var-naming linter errors and a fumpt

* Update CHANGELOG.md

* Update .golangci.yml

* Update CHANGELOG.md

* Update test_helpers.go

* Update test_helpers.go

* fumpt and lint

* this lints the db module, and makes it easier to use.  It adds breaking name changes

* DBConnection -> Connection

* previous commit contained a merge error

* Update test_helpers.go

* Update test_helpers.go

* db renamings

* merge master

* changelog

* DBWriter -> Writer

* consistent multistore reciever

* standard recievers for multistore v2alpha1

* general cleanup of linting issues

* more linter fixes

* remove prealloc linter

* nolint the secp256k1 import

* nolint the secp256k1 package

* completenolint resulting in a diff that has only nolints
  • Loading branch information
faddat authored Jun 8, 2022
1 parent 9e66071 commit b0e82f9
Show file tree
Hide file tree
Showing 75 changed files with 440 additions and 477 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ linters:
- misspell
- nakedret
- nolintlint
- prealloc
- revive
- staticcheck
- structcheck
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,12 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking Changes

* (x/staking) [#12102](https://github.com/cosmos/cosmos-sdk/pull/12102) Staking keeper now is passed by reference instead of copy. Keeper's SetHooks no longer returns keeper. It updates the keeper in place instead.
* (linting) [#12141](https://github.com/cosmos/cosmos-sdk/pull/12141) Fix usability related linting for database. This means removing the infix Prefix from `prefix.NewPrefixWriter` and such so that it is `prefix.NewWriter` and making `db.DBConnection` and such into `db.Connection`


### Bug Fixes

* (linting) [#12135](https://github.com/cosmos/cosmos-sdk/pull/12135/) Fix variable naming issues per enabled linters. Run gofumpt to ensure easy reviews of ongoing linting work.
* (linting) [#12135](https://github.com/cosmos/cosmos-sdk/pull/12135) Fix variable naming issues per enabled linters. Run gofumpt to ensure easy reviews of ongoing linting work.
* (linting) [#12132](https://github.com/cosmos/cosmos-sdk/pull/12132) Change sdk.Int to math.Int, run `gofumpt -w -l .`, and `golangci-lint run ./... --fix`
* (cli) [#12127](https://github.com/cosmos/cosmos-sdk/pull/12127) Fix the CLI not always taking into account `--fee-payer` and `--fee-granter` flags.
* (migrations) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Fix v0.45->v0.46 in-place store migrations.
Expand Down
27 changes: 15 additions & 12 deletions client/debug/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ import (
"github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/version"

legacybech32 "github.com/cosmos/cosmos-sdk/types/bech32/legacybech32"
legacybech32 "github.com/cosmos/cosmos-sdk/types/bech32/legacybech32" //nolint:staticcheck // we do old keys, they're keys after all.
)

var flagPubkeyType = "type"
var (
flagPubkeyType = "type"
ed = "ed25519"
)

// Cmd creates a main CLI command
func Cmd() *cobra.Command {
Expand Down Expand Up @@ -69,7 +72,7 @@ $ %s debug pubkey '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"AurroA7jvfP
}

func bytesToPubkey(bz []byte, keytype string) (cryptotypes.PubKey, bool) {
if keytype == "ed25519" {
if keytype == ed {
if len(bz) == ed25519.PubKeySize {
return &ed25519.PubKey{Key: bz}, true
}
Expand Down Expand Up @@ -102,17 +105,17 @@ func getPubKeyFromRawString(pkstr string, keytype string) (cryptotypes.PubKey, e
}
}

pk, err := legacybech32.UnmarshalPubKey(legacybech32.AccPK, pkstr)
pk, err := legacybech32.UnmarshalPubKey(legacybech32.AccPK, pkstr) //nolint:staticcheck // we do old keys, they're keys after all.
if err == nil {
return pk, nil
}

pk, err = legacybech32.UnmarshalPubKey(legacybech32.ValPK, pkstr)
pk, err = legacybech32.UnmarshalPubKey(legacybech32.ValPK, pkstr) //nolint:staticcheck // we do old keys, they're keys after all.
if err == nil {
return pk, nil
}

pk, err = legacybech32.UnmarshalPubKey(legacybech32.ConsPK, pkstr)
pk, err = legacybech32.UnmarshalPubKey(legacybech32.ConsPK, pkstr) //nolint:staticcheck // we do old keys, they're keys after all.
if err == nil {
return pk, nil
}
Expand All @@ -138,7 +141,7 @@ $ %s debug pubkey-raw cosmos1e0jnq2sun3dzjh8p2xq95kk0expwmd7shwjpfg
return err
}
pubkeyType = strings.ToLower(pubkeyType)
if pubkeyType != "secp256k1" && pubkeyType != "ed25519" {
if pubkeyType != "secp256k1" && pubkeyType != ed {
return errors.Wrapf(errors.ErrInvalidType, "invalid pubkey type, expected oneof ed25519 or secp256k1")
}

Expand All @@ -149,8 +152,8 @@ $ %s debug pubkey-raw cosmos1e0jnq2sun3dzjh8p2xq95kk0expwmd7shwjpfg

var consensusPub string
edPK, ok := pk.(*ed25519.PubKey)
if ok && pubkeyType == "ed25519" {
consensusPub, err = legacybech32.MarshalPubKey(legacybech32.ConsPK, edPK)
if ok && pubkeyType == ed {
consensusPub, err = legacybech32.MarshalPubKey(legacybech32.ConsPK, edPK) //nolint:staticcheck // we do old keys, they're keys after all.
if err != nil {
return err
}
Expand All @@ -163,11 +166,11 @@ $ %s debug pubkey-raw cosmos1e0jnq2sun3dzjh8p2xq95kk0expwmd7shwjpfg
if err != nil {
return err
}
accPub, err := legacybech32.MarshalPubKey(legacybech32.AccPK, pk)
accPub, err := legacybech32.MarshalPubKey(legacybech32.AccPK, pk) //nolint:staticcheck // we do old keys, they're keys after all.
if err != nil {
return err
}
valPub, err := legacybech32.MarshalPubKey(legacybech32.ValPK, pk)
valPub, err := legacybech32.MarshalPubKey(legacybech32.ValPK, pk) //nolint:staticcheck // we do old keys, they're keys after all.
if err != nil {
return err
}
Expand All @@ -182,7 +185,7 @@ $ %s debug pubkey-raw cosmos1e0jnq2sun3dzjh8p2xq95kk0expwmd7shwjpfg
return nil
},
}
cmd.Flags().StringP(flagPubkeyType, "t", "ed25519", "Pubkey type to decode (oneof secp256k1, ed25519)")
cmd.Flags().StringP(flagPubkeyType, "t", ed, "Pubkey type to decode (oneof secp256k1, ed25519)")
return cmd
}

Expand Down
2 changes: 1 addition & 1 deletion codec/proto_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"fmt"
"strings"

legacyproto "github.com/golang/protobuf/proto"
legacyproto "github.com/golang/protobuf/proto" //nolint:staticcheck // we're aware this is deprecated and using it anyhow.
"google.golang.org/grpc/encoding"
"google.golang.org/protobuf/proto"

Expand Down
2 changes: 1 addition & 1 deletion crypto/keys/secp256k1/internal/secp256k1/curve.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

//nolint // this nolint lets us use this file in its original and unmodified form.
package secp256k1

import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//go:build !gofuzz && cgo
// +build !gofuzz,cgo

//nolint // this nolint lets us use this file in its original and unmodified form.
package secp256k1

import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//go:build gofuzz || !cgo
// +build gofuzz !cgo

//nolint // this nolint lets us use this file in its original and unmodified form.
package secp256k1

import "math/big"
Expand Down
2 changes: 1 addition & 1 deletion db/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Databases supporting mappings of arbitrary byte sequences.

The database interface types consist of objects to encapsulate the singular connection to the DB, transactions being made to it, historical version state, and iteration.

### `DBConnection`
### `Connection`

This interface represents a connection to a versioned key-value database. All versioning operations are performed using methods on this type.

Expand Down
4 changes: 2 additions & 2 deletions db/adapter.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package db

type readerRWAdapter struct{ DBReader }
type readerRWAdapter struct{ Reader }

// ReaderAsReadWriter returns a ReadWriter that forwards to a reader and errors if writes are
// attempted. Can be used to pass a Reader when a ReadWriter is expected
// but no writes will actually occur.
func ReaderAsReadWriter(r DBReader) DBReadWriter {
func ReaderAsReadWriter(r Reader) ReadWriter {
return readerRWAdapter{r}
}

Expand Down
22 changes: 11 additions & 11 deletions db/badgerdb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
var versionsFilename = "versions.csv"

var (
_ db.DBConnection = (*BadgerDB)(nil)
_ db.DBReader = (*badgerTxn)(nil)
_ db.DBWriter = (*badgerWriter)(nil)
_ db.DBReadWriter = (*badgerWriter)(nil)
_ db.Connection = (*BadgerDB)(nil)
_ db.Reader = (*badgerTxn)(nil)
_ db.Writer = (*badgerWriter)(nil)
_ db.ReadWriter = (*badgerWriter)(nil)
)

// BadgerDB is a connection to a BadgerDB key-value database.
Expand Down Expand Up @@ -164,14 +164,14 @@ func writeVersionsFile(vm *versionManager, path string) error {
return w.WriteAll(rows)
}

func (b *BadgerDB) Reader() db.DBReader {
func (b *BadgerDB) Reader() db.Reader {
b.mtx.RLock()
ts := b.vmgr.lastTs
b.mtx.RUnlock()
return &badgerTxn{txn: b.db.NewTransactionAt(ts, false), db: b}
}

func (b *BadgerDB) ReaderAt(version uint64) (db.DBReader, error) {
func (b *BadgerDB) ReaderAt(version uint64) (db.Reader, error) {
b.mtx.RLock()
defer b.mtx.RUnlock()
ts, has := b.vmgr.versionTs(version)
Expand All @@ -181,15 +181,15 @@ func (b *BadgerDB) ReaderAt(version uint64) (db.DBReader, error) {
return &badgerTxn{txn: b.db.NewTransactionAt(ts, false), db: b}, nil
}

func (b *BadgerDB) ReadWriter() db.DBReadWriter {
func (b *BadgerDB) ReadWriter() db.ReadWriter {
atomic.AddInt32(&b.openWriters, 1)
b.mtx.RLock()
ts := b.vmgr.lastTs
b.mtx.RUnlock()
return &badgerWriter{badgerTxn{txn: b.db.NewTransactionAt(ts, true), db: b}, false}
}

func (b *BadgerDB) Writer() db.DBWriter {
func (b *BadgerDB) Writer() db.Writer {
// Badger has a WriteBatch, but it doesn't support conflict detection
return b.ReadWriter()
}
Expand All @@ -201,7 +201,7 @@ func (b *BadgerDB) Close() error {
return b.db.Close()
}

// Versions implements DBConnection.
// Versions implements Connection.
// Returns a VersionSet that is valid until the next call to SaveVersion or DeleteVersion.
func (b *BadgerDB) Versions() (db.VersionSet, error) {
b.mtx.RLock()
Expand All @@ -219,12 +219,12 @@ func (b *BadgerDB) save(target uint64) (uint64, error) {
return b.vmgr.Save(target)
}

// SaveNextVersion implements DBConnection.
// SaveNextVersion implements Connection.
func (b *BadgerDB) SaveNextVersion() (uint64, error) {
return b.save(0)
}

// SaveVersion implements DBConnection.
// SaveVersion implements Connection.
func (b *BadgerDB) SaveVersion(target uint64) error {
if target == 0 {
return db.ErrInvalidVersion
Expand Down
2 changes: 1 addition & 1 deletion db/badgerdb/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/db/dbtest"
)

func load(t *testing.T, dir string) db.DBConnection {
func load(t *testing.T, dir string) db.Connection {
d, err := NewDB(dir)
require.NoError(t, err)
return d
Expand Down
10 changes: 5 additions & 5 deletions db/dbtest/benchmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func BytesToInt64(buf []byte) int64 {
return int64(binary.BigEndian.Uint64(buf))
}

func BenchmarkRangeScans(b *testing.B, db dbm.DBReadWriter, dbSize int64) {
func BenchmarkRangeScans(b *testing.B, db dbm.ReadWriter, dbSize int64) {
b.StopTimer()

rangeSize := int64(10000)
Expand All @@ -40,7 +40,7 @@ func BenchmarkRangeScans(b *testing.B, db dbm.DBReadWriter, dbSize int64) {
b.StartTimer()

for i := 0; i < b.N; i++ {
start := rand.Int63n(dbSize - rangeSize) // nolint: gosec
start := rand.Int63n(dbSize - rangeSize)
end := start + rangeSize
iter, err := db.Iterator(Int64ToBytes(start), Int64ToBytes(end))
require.NoError(b, err)
Expand All @@ -53,7 +53,7 @@ func BenchmarkRangeScans(b *testing.B, db dbm.DBReadWriter, dbSize int64) {
}
}

func BenchmarkRandomReadsWrites(b *testing.B, db dbm.DBReadWriter) {
func BenchmarkRandomReadsWrites(b *testing.B, db dbm.ReadWriter) {
b.StopTimer()

// create dummy data
Expand All @@ -67,7 +67,7 @@ func BenchmarkRandomReadsWrites(b *testing.B, db dbm.DBReadWriter) {

for i := 0; i < b.N; i++ {
{
idx := rand.Int63n(numItems) // nolint: gosec
idx := rand.Int63n(numItems)
internal[idx]++
val := internal[idx]
idxBytes := Int64ToBytes(idx)
Expand All @@ -80,7 +80,7 @@ func BenchmarkRandomReadsWrites(b *testing.B, db dbm.DBReadWriter) {
}

{
idx := rand.Int63n(numItems) // nolint: gosec
idx := rand.Int63n(numItems)
valExp := internal[idx]
idxBytes := Int64ToBytes(idx)
valBytes, err := db.Get(idxBytes)
Expand Down
17 changes: 8 additions & 9 deletions db/dbtest/testcases.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
dbm "github.com/cosmos/cosmos-sdk/db"
)

type Loader func(*testing.T, string) dbm.DBConnection
type Loader func(*testing.T, string) dbm.Connection

func ikey(i int) []byte { return []byte(fmt.Sprintf("key-%03d", i)) }
func ival(i int) []byte { return []byte(fmt.Sprintf("val-%03d", i)) }
Expand All @@ -20,10 +20,9 @@ func DoTestGetSetHasDelete(t *testing.T, load Loader) {
t.Helper()
db := load(t, t.TempDir())

var txn dbm.DBReadWriter
var view dbm.DBReader
var txn dbm.ReadWriter
view := db.Reader()

view = db.Reader()
require.NotNil(t, view)

// A nonexistent key should return nil.
Expand Down Expand Up @@ -261,11 +260,11 @@ func DoTestVersioning(t *testing.T, load Loader) {
require.False(t, has)
require.NoError(t, view.Discard())

view, err = db.ReaderAt(versions.Last() + 1)
view, err = db.ReaderAt(versions.Last() + 1) //nolint:staticcheck // we nolint here because we are checking for the absence of an error.
require.Equal(t, dbm.ErrVersionDoesNotExist, err, "should fail to read a nonexistent version")

require.NoError(t, db.DeleteVersion(v2), "should delete version v2")
view, err = db.ReaderAt(v2)
view, err = db.ReaderAt(v2) //nolint:staticcheck // we nolint here because we are checking for the absence of an error.
require.Equal(t, dbm.ErrVersionDoesNotExist, err)

// Ensure latest version is accurate
Expand Down Expand Up @@ -298,9 +297,9 @@ func DoTestTransactions(t *testing.T, load Loader, multipleWriters bool) {
t.Helper()
db := load(t, t.TempDir())
// Both methods should work in a DBWriter context
writerFuncs := []func() dbm.DBWriter{
writerFuncs := []func() dbm.Writer{
db.Writer,
func() dbm.DBWriter { return db.ReadWriter() },
func() dbm.Writer { return db.ReadWriter() },
}

for _, getWriter := range writerFuncs {
Expand Down Expand Up @@ -397,7 +396,7 @@ func DoTestRevert(t *testing.T, load Loader, reload bool) {
t.Helper()
dirname := t.TempDir()
db := load(t, dirname)
var txn dbm.DBWriter
var txn dbm.Writer

initContents := func() {
txn = db.Writer()
Expand Down
2 changes: 1 addition & 1 deletion db/dbtest/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func AssertKeyPanics(t *testing.T, itr dbm.Iterator) {
assert.Panics(t, func() { itr.Key() }, "checkKeyPanics expected panic but didn't")
}

func AssertValue(t *testing.T, db dbm.DBReader, key, valueWanted []byte) {
func AssertValue(t *testing.T, db dbm.Reader, key, valueWanted []byte) {
t.Helper()
valueGot, err := db.Get(key)
assert.NoError(t, err)
Expand Down
Loading

0 comments on commit b0e82f9

Please sign in to comment.