Merge commit 'refs/pull/2758/head' of https://github.com/digitalbitbox/bitbox-wallet-app

Author: benma

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ env:
   # https://docs.github.com/en/packages/guides/pushing-and-pulling-docker-images
   #
   # Keep this in sync with default in scripts/github-ci.sh.
-  CI_IMAGE: ghcr.io/bitboxswiss/bitbox-wallet-app-ci:22
+  CI_IMAGE: ghcr.io/bitboxswiss/bitbox-wallet-app-ci:23
   GITHUB_BUILD_DIR: ${{github.workspace}}
 
 jobs:

.golangci.yml

@@ -3,13 +3,13 @@
 
 # options for analysis running
 run:
-  go: 1.21
+  go: "1.21"
 
   # default concurrency is a available CPU number
   concurrency: 4
 
   # timeout for analysis, e.g. 30s, 5m, default is 1m
-  deadline: 10m
+  timeout: 10m
 
   # exit code when at least one issue was found, default is 1
   issues-exit-code: 1
@@ -23,7 +23,9 @@ run:
 # output configuration options
 output:
   # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
-  format: colored-line-number
+  formats: 
+    - format: colored-line-number
+      path: stdout
 
   # print lines of code with issue, default is true
   print-issued-lines: true
@@ -36,31 +38,10 @@ output:
 linters-settings:
   gocritic:
     enabled-checks:
-      - assignOp
       - builtinShadow
-      - captLocal
-      - caseOrder
-      - defaultCaseOrder
-      - dupArg
-      - dupBranchBody
-      - dupCase
-      - elseif
-      - flagDeref
-      - ifElseChain
       - nilValReturn
-      - rangeExprCopy
       - rangeValCopy
-      - regexpMust
-      - sloppyLen
-      - switchTrue
-      - typeSwitchVar
       - typeUnparen
-      - underef
-      - unlambda
-      - unslice
-  maligned:
-    # print struct with more effective memory layout or not, false by default
-    suggest-new: true
   dupl:
     # tokens count to trigger issue, 150 by default
     threshold: 150
@@ -78,8 +59,6 @@ linters-settings:
     # max line length, lines longer will be reported. Default is 120.
     # '\t' is counted as 1 character by default, and can be changed with the tab-width option
     line-length: 120
-    # allow long comments.
-    exclude: '//'
   revive:
     rules:
       - name: exported
@@ -88,22 +67,18 @@ linters-settings:
 linters:
   enable-all: true
   disable:
-    - golint
-    - scopelint
-    - interfacer
     - gosec
     - bodyclose
     - dogsled
     - dupl
+    - err113 # can't switch to errors.Is due to GopherJS still on go1.12
     - funlen
     - gochecknoglobals
     - gochecknoinits
     - gocognit
     - godox
-    - goerr113 # can't switch to errors.Is due to GopherJS still on go1.12
     - gomnd
     - lll
-    - maligned
     - nestif
     - prealloc
     - stylecheck
@@ -119,12 +94,10 @@ linters:
     - noctx
     - nlreturn
     - cyclop
-    - ifshort
     - exhaustive
     - nilnil
     - nilerr
     - forcetypeassert
-    - exhaustivestruct
     - exhaustruct
     - maintidx
     - errorlint
@@ -133,35 +106,49 @@ linters:
     - errname
     - forbidigo
     - makezero
+    - mnd
     - nolintlint
     - gocyclo
-    - nosnakecase
     - interfacebloat
     - musttag
-    - structcheck
-    - deadcode
-    - varcheck
     - depguard
     - inamedparam
     - protogetter
+    - perfsprint
     - gosmopolitan
-    # todo: enable this, but there are a lot of hits
-    - testifylint
+    # TODO: enable this after merge https://github.com/karalabe/hid/pull/52
     - gomoddirectives
   disable-all: false
 
 issues:
-  # List of regexps of issue texts to exclude, empty list by default.
-  # But independently from this option we use default exclude patterns,
-  # it can be disabled by `exclude-use-default: false`. To list all
-  # excluded by default patterns execute `golangci-lint run --help`
-  exclude:
-  # Use linters in their original configuration. golangci-linter changes the default config, for
-  # example skipping the docstring comment checks of golint.
+  # Independently of option `exclude` we use default exclude patterns,
+  # it can be disabled by this option.
+  # To list all excluded by default patterns execute `golangci-lint run --help`.
+  # Default: true
   exclude-use-default: false
 
   # Excluding configuration per-path, per-linter, per-text and per-source.
   exclude-rules:
+    - linters:
+        - gocritic
+      text: "singleCaseSwitch: should rewrite switch statement to if statement"
+    - linters:
+        - gocritic
+      text: "badCond: `ours == from && ours == to` condition is suspicious"
+    - linters:
+        - gocritic
+      text: "builtinShadow: shadowing of predeclared identifier: max"
+    - linters:
+        - canonicalheader
+      text: "non-canonical header \"X-API-KEY\", instead use: \"X-Api-Key\""
+    - linters:
+        - testifylint
+      text: "go-require: require must only be used in the goroutine running the test function"
+    - linters:
+        - testifylint
+      text: "go-require: do not use require in http handlers"
+    - path: _test\.go
+      text: "float-compare"
     # In addition to always disalbed linters above, exclude some more in tests.
     - path: _test\.go
       linters:

backend/accounts_test.go

@@ -87,8 +87,8 @@ func TestAccounts(t *testing.T) {
 	b := newBackend(t, testnetDisabled, regtestDisabled)
 	defer b.Close()
 
-	require.Len(t, b.Accounts(), 0)
-	require.Len(t, b.Config().AccountsConfig().Accounts, 0)
+	require.Empty(t, b.Accounts())
+	require.Empty(t, b.Config().AccountsConfig().Accounts)
 
 	// 1) Registering a new keystore persists a set of initial default accounts.
 	b.registerKeystore(ks)
@@ -670,7 +670,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
 		)
 		require.NoError(t, err)
 		require.Nil(t, b.Config().AccountsConfig().Lookup("v0-55555555-eth-0"))
-		require.Equal(t, accountsCount, len(b.Config().AccountsConfig().Accounts))
+		require.Len(t, b.Config().AccountsConfig().Accounts, accountsCount)
 
 		// Try to add another Bitcoin account - can't, only one account supported.
 		_, err = b.CreateAndPersistAccountConfig(
@@ -679,7 +679,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
 			bitbox01LikeKeystore,
 		)
 		require.Equal(t, errAccountLimitReached, errp.Cause(err))
-		require.Equal(t, accountsCount, len(b.Config().AccountsConfig().Accounts))
+		require.Len(t, b.Config().AccountsConfig().Accounts, accountsCount)
 
 		// Try to add another Litecoin account - can't, only one account supported.
 		_, err = b.CreateAndPersistAccountConfig(
@@ -688,7 +688,7 @@ func TestCreateAndPersistAccountConfig(t *testing.T) {
 			bitbox01LikeKeystore,
 		)
 		require.Equal(t, errAccountLimitReached, errp.Cause(err))
-		require.Equal(t, accountsCount, len(b.Config().AccountsConfig().Accounts))
+		require.Len(t, b.Config().AccountsConfig().Accounts, accountsCount)
 	})
 
 	// If the keystore cannot retrieve an xpub (e.g. USB communication error), no account should be
@@ -885,41 +885,41 @@ func TestInactiveAccount(t *testing.T) {
 	checkShownAccountsLen(t, b, 3, 3)
 	require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0"))
 	require.False(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0").Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
 	require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-55555555-ltc-0"))
 	require.False(t, b.Config().AccountsConfig().Lookup("v0-55555555-ltc-0").Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-ltc-0").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-ltc-0").Config().Config.Inactive)
 	require.NotNil(t, b.Config().AccountsConfig().Lookup("v0-55555555-eth-0"))
 	require.False(t, b.Config().AccountsConfig().Lookup("v0-55555555-eth-0").Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
 
 	// Deactive an account.
 	require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", false))
 	checkShownAccountsLen(t, b, 3, 3)
 	require.True(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0").Inactive)
-	require.False(t, !b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
+	require.True(t, b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
 
 	// Reactivate.
 	require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", true))
 	checkShownAccountsLen(t, b, 3, 3)
 	require.False(t, b.Config().AccountsConfig().Lookup("v0-55555555-btc-0").Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-btc-0").Config().Config.Inactive)
 
 	// Deactivating an ETH account with tokens also removes the tokens
 	require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-usdt", true))
 	require.NoError(t, b.SetTokenActive("v0-55555555-eth-0", "eth-erc20-bat", true))
 	checkShownAccountsLen(t, b, 5, 3)
 	require.NoError(t, b.SetAccountActive("v0-55555555-eth-0", false))
 	checkShownAccountsLen(t, b, 5, 3)
-	require.False(t, !b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
-	require.False(t, !b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
-	require.False(t, !b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
+	require.True(t, b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
+	require.True(t, b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
+	require.True(t, b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
 	// Reactivating restores them again.
 	require.NoError(t, b.SetAccountActive("v0-55555555-eth-0", true))
 	checkShownAccountsLen(t, b, 5, 3)
-	require.True(t, !b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
-	require.True(t, !b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-eth-0").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-usdt").Config().Config.Inactive)
+	require.False(t, b.Accounts().lookup("v0-55555555-eth-0-eth-erc20-bat").Config().Config.Inactive)
 
 	// Deactivate all accounts.
 	require.NoError(t, b.SetAccountActive("v0-55555555-btc-0", false))

backend/aopp_test.go

@@ -154,7 +154,7 @@ func TestAOPPSuccess(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run("", func(t *testing.T) {
-			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				require.Equal(t, "POST", r.Method)
 				require.Equal(t,
 					[]string{"application/json"},
@@ -168,7 +168,8 @@ func TestAOPPSuccess(t *testing.T) {
 					string(body),
 				)
 				w.WriteHeader(http.StatusNoContent)
-			}))
+			})
+			server := httptest.NewServer(handler)
 			defer server.Close()
 
 			b := newBackend(t, testnetDisabled, regtestDisabled)
@@ -313,7 +314,7 @@ func TestAOPPSuccess(t *testing.T) {
 		params := defaultParams()
 		b.registerKeystore(makeKeystore(t, scriptTypeRef(signing.ScriptTypeP2WPKH), keystoreHelper.ExtendedPublicKey))
 		fingerprint, err := b.keystore.RootFingerprint()
-		require.Nil(t, err)
+		require.NoError(t, err)
 		b.SetWatchonly(fingerprint, true)
 		b.DeregisterKeystore()
 

backend/backend_test.go

@@ -403,7 +403,7 @@ func TestRegisterKeystore(t *testing.T) {
 	require.Equal(t, rootFingerprint1, []byte(b.Config().AccountsConfig().Keystores[0].RootFingerprint))
 	// LastConnected might not be `time.Now()` anymore as some time may have passed in the unit
 	// tests, but we check that it was set and recent.
-	require.True(t, time.Since(b.Config().AccountsConfig().Keystores[0].LastConnected) < 10*time.Second)
+	require.Less(t, time.Since(b.Config().AccountsConfig().Keystores[0].LastConnected), 10*time.Second)
 
 	// Deregistering the keystore leaves the loaded accounts (watchonly), and leaves the persisted
 	// accounts and keystores.

backend/banners/banners_test.go

@@ -25,23 +25,24 @@ import (
 )
 
 func TestInit(t *testing.T) {
-	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+	handler := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		// Test request parameters
 		_, err := rw.Write([]byte(`
-{
-    "bitbox01": {
-        "id": "some-id",
-        "message": {
-            "en": "some-msg"
-        },
-        "link": {
-            "href": "some-link"
-        }
-    }
-}
+		{
+			"bitbox01": {
+				"id": "some-id",
+				"message": {
+					"en": "some-msg"
+				},
+				"link": {
+					"href": "some-link"
+				}
+			}
+		}
 		`))
 		require.NoError(t, err)
-	}))
+	})
+	server := httptest.NewServer(handler)
 	defer server.Close()
 
 	banners := NewBanners()

backend/bitsurance/bitsurance_test.go

@@ -57,6 +57,6 @@ func TestBitsuranceGetId(t *testing.T) {
 		},
 	}
 	id, err := GetBitsuranceId(mockedInterface)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, expectedId, id)
 }

backend/coins/btc/account_test.go

@@ -164,10 +164,10 @@ func TestInsuredAccountAddresses(t *testing.T) {
 	require.NoError(t, account.Initialize())
 
 	// check the number of available addresses for native and wrapped segwit.
-	require.Equal(t, len(account.GetUnusedReceiveAddresses()[0].Addresses), 20)
-	require.Equal(t, *account.GetUnusedReceiveAddresses()[0].ScriptType, signing.ScriptTypeP2WPKHP2SH)
-	require.Equal(t, len(account.GetUnusedReceiveAddresses()[1].Addresses), 20)
-	require.Equal(t, *account.GetUnusedReceiveAddresses()[1].ScriptType, signing.ScriptTypeP2WPKH)
+	require.Len(t, account.GetUnusedReceiveAddresses()[0].Addresses, 20)
+	require.Equal(t, signing.ScriptTypeP2WPKHP2SH, *account.GetUnusedReceiveAddresses()[0].ScriptType)
+	require.Len(t, account.GetUnusedReceiveAddresses()[1].Addresses, 20)
+	require.Equal(t, signing.ScriptTypeP2WPKH, *account.GetUnusedReceiveAddresses()[1].ScriptType)
 
 	// Create a new insured account.
 	account2 := mockAccount(t, &config.Account{
@@ -180,9 +180,9 @@ func TestInsuredAccountAddresses(t *testing.T) {
 	require.NoError(t, account2.Initialize())
 
 	// native segwit is the only address type available.
-	require.Equal(t, len(account2.GetUnusedReceiveAddresses()), 1)
-	require.Equal(t, len(account2.GetUnusedReceiveAddresses()[0].Addresses), 20)
-	require.Equal(t, *account2.GetUnusedReceiveAddresses()[0].ScriptType, signing.ScriptTypeP2WPKH)
+	require.Len(t, account2.GetUnusedReceiveAddresses(), 1)
+	require.Len(t, account2.GetUnusedReceiveAddresses()[0].Addresses, 20)
+	require.Equal(t, signing.ScriptTypeP2WPKH, *account2.GetUnusedReceiveAddresses()[0].ScriptType)
 
 }