From 1a22a353841d7e625079661003690566e9efc849 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 14:47:49 -0500 Subject: [PATCH 1/7] lint: enable exhaustive, gocritic, nilerr In this commit, we enable extra linters to check for non-exhaustive switch statements, and if-else or switch statements that can be simplified. We also check for improper error returns and other small simplifications. --- .golangci.yml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index af57bba3e..d8690c064 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -23,8 +23,15 @@ linters-settings: excludes: - G402 # Look for bad TLS connection settings. - G306 # Poor file permissions used when writing to a new file. + exhaustive: + default-signifies-exhaustive: true + gocritic: + enabled-checks: + - singleCaseSwitch + - ifElseChain + - assignOp + - unlambda staticcheck: - go: "1.19" checks: ["-SA1019"] linters: @@ -34,6 +41,11 @@ linters: - tagliatelle - whitespace - gosec + - unused + - gocritic + - unconvert + - nilerr + - exhaustive issues: exclude-rules: From 8f40c64d17f5ca1c0f5e0cfc9d8a01c99e6937ec Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 14:55:09 -0500 Subject: [PATCH 2/7] multi: remove unusedparams found with gopls --- itest/mint_batch_stress_test.go | 2 +- itest/universe_pagination_test.go | 6 ++-- tapdb/asset_minting_test.go | 53 ++++++++++++++----------------- tapdb/sqlutils.go | 2 +- 4 files changed, 29 insertions(+), 34 deletions(-) diff --git a/itest/mint_batch_stress_test.go b/itest/mint_batch_stress_test.go index 124ae44ea..d4313622d 100644 --- a/itest/mint_batch_stress_test.go +++ b/itest/mint_batch_stress_test.go @@ -202,7 +202,7 @@ func mintBatchStressTest( // outpoints matching the chain anchor of the group anchor. mintOutpoint := collectibleAnchor.ChainAnchor.AnchorOutpoint - leafKeys, err := fetchAllLeafKeys(t, alice, &collectUniID) + leafKeys, err := fetchAllLeafKeys(alice, &collectUniID) require.NoError(t, err) require.Len(t, leafKeys, batchSize) diff --git a/itest/universe_pagination_test.go b/itest/universe_pagination_test.go index 9ef4c1065..95568a05c 100644 --- a/itest/universe_pagination_test.go +++ b/itest/universe_pagination_test.go @@ -180,7 +180,7 @@ func mintBatchAssetsTest( // all outpoints matching the chain anchor of the group anchor. mintOutpoint := collectibleAnchor.ChainAnchor.AnchorOutpoint - leafKeys, err := fetchAllLeafKeys(t, alice, &collectUniID) + leafKeys, err := fetchAllLeafKeys(alice, &collectUniID) require.NoError(t, err) require.Len(t, leafKeys, len(mintBatch)) @@ -218,8 +218,8 @@ func mintBatchAssetsTest( } // fetchAllLeafKeys fetches all leaf keys for a given universe ID. -func fetchAllLeafKeys(t *testing.T, alice TapdClient, - id *unirpc.ID) ([]*unirpc.AssetKey, error) { +func fetchAllLeafKeys(alice TapdClient, id *unirpc.ID) ([]*unirpc.AssetKey, + error) { keys := make([]*unirpc.AssetKey, 0) offset := int32(0) diff --git a/tapdb/asset_minting_test.go b/tapdb/asset_minting_test.go index a719f3e89..a9a6824b6 100644 --- a/tapdb/asset_minting_test.go +++ b/tapdb/asset_minting_test.go @@ -193,7 +193,7 @@ func treeFromBranch(t *testing.T, children [][]byte) (chainhash.Hash, // storeTapscriptTreeWrapper wraps a DB transaction that stores a tapscript // tree. -func storeTapscriptTreeWrapper(t *testing.T, ctx context.Context, isBranch bool, +func storeTapscriptTreeWrapper(ctx context.Context, isBranch bool, store *AssetMintingStore, rootHash []byte, nodes [][]byte) error { var writeTxOpts AssetStoreTxOptions @@ -207,9 +207,8 @@ func storeTapscriptTreeWrapper(t *testing.T, ctx context.Context, isBranch bool, // fetchTapscriptTreeWrapper wraps a DB transaction that fetches a tapscript // tree. -func fetchTapscriptTreeWrapper(t *testing.T, ctx context.Context, - rootHash []byte, store *AssetMintingStore) ([]TapscriptTreeNode, - error) { +func fetchTapscriptTreeWrapper(ctx context.Context, rootHash []byte, + store *AssetMintingStore) ([]TapscriptTreeNode, error) { var ( dbTreeNodes []TapscriptTreeNode @@ -228,8 +227,8 @@ func fetchTapscriptTreeWrapper(t *testing.T, ctx context.Context, // deleteTapscriptTreeWrapper wraps a DB transaction that deletes a tapscript // tree. -func deleteTapscriptTreeWrapper(t *testing.T, ctx context.Context, - rootHash []byte, store *AssetMintingStore) error { +func deleteTapscriptTreeWrapper(ctx context.Context, rootHash []byte, + store *AssetMintingStore) error { var writeTxOpts AssetStoreTxOptions return store.db.ExecTx(ctx, &writeTxOpts, @@ -242,7 +241,7 @@ func deleteTapscriptTreeWrapper(t *testing.T, ctx context.Context, func assertTreeDeletion(t *testing.T, ctx context.Context, rootHash []byte, store *AssetMintingStore) { - dbTree, err := fetchTapscriptTreeWrapper(t, ctx, rootHash, store) + dbTree, err := fetchTapscriptTreeWrapper(ctx, rootHash, store) require.NoError(t, err) require.Empty(t, dbTree) } @@ -252,7 +251,7 @@ func assertTreeDeletion(t *testing.T, ctx context.Context, rootHash []byte, func assertStoredTreeEqual(t *testing.T, ctx context.Context, isBranch bool, store *AssetMintingStore, rootHash []byte, expected [][]byte) { - dbTree, err := fetchTapscriptTreeWrapper(t, ctx, rootHash, store) + dbTree, err := fetchTapscriptTreeWrapper(ctx, rootHash, store) require.NoError(t, err) require.True(t, fn.All(dbTree, func(node TapscriptTreeNode) bool { @@ -356,8 +355,8 @@ func addRandSiblingToBatch(t *testing.T, batch *tapgarden.MintingBatch) ( // seedling is being issued into an existing group, and creates a multi-asset // group. Specifically, one seedling will have emission enabled, and the other // seedling will reference the first seedling as its group anchor. -func addMultiAssetGroupToBatch(t *testing.T, - seedlings map[string]*tapgarden.Seedling) (string, string) { +func addMultiAssetGroupToBatch(seedlings map[string]*tapgarden.Seedling) (string, + string) { seedlingNames := maps.Keys(seedlings) seedlingCount := len(seedlingNames) @@ -1268,7 +1267,7 @@ func TestGroupAnchors(t *testing.T) { _, seedlingGroups, _ := addRandGroupToBatch( t, assetStore, ctx, mintingBatch.Seedlings, ) - addMultiAssetGroupToBatch(t, mintingBatch.Seedlings) + addMultiAssetGroupToBatch(mintingBatch.Seedlings) err := assetStore.CommitMintingBatch(ctx, mintingBatch) require.NoError(t, err) @@ -1283,9 +1282,7 @@ func TestGroupAnchors(t *testing.T) { // Now we'll add an additional set of seedlings with // another multi-asset group. seedlings := tapgarden.RandSeedlings(t, numSeedlings) - secondAnchor, secondGrouped := addMultiAssetGroupToBatch( - t, seedlings, - ) + secondAnchor, secondGrouped := addMultiAssetGroupToBatch(seedlings) // We add seedlings one at a time, in order, as the planter does. mintingBatch.Seedlings = mergeMap(mintingBatch.Seedlings, seedlings) @@ -1424,24 +1421,24 @@ func TestTapscriptTreeStore(t *testing.T) { // Start with the cases where tree insertion should fail. badRootHashErr := storeTapscriptTreeWrapper( - t, ctx, false, assetStore, tree1Hash[1:], tree1, + ctx, false, assetStore, tree1Hash[1:], tree1, ) require.ErrorContains(t, badRootHashErr, "must be 32 bytes") emptyTreeErr := storeTapscriptTreeWrapper( - t, ctx, false, assetStore, tree1Hash[:], nil, + ctx, false, assetStore, tree1Hash[:], nil, ) require.ErrorContains(t, emptyTreeErr, "no tapscript tree nodes") invalidBranchErr := storeTapscriptTreeWrapper( - t, ctx, true, assetStore, tree4Hash[:], tree3, + ctx, true, assetStore, tree4Hash[:], tree3, ) require.ErrorContains(t, invalidBranchErr, "must be 2 nodes") // Now, let's insert the first tree, and then assert that we can fetch // and decode an identical tree. err := storeTapscriptTreeWrapper( - t, ctx, false, assetStore, tree1Hash[:], tree1, + ctx, false, assetStore, tree1Hash[:], tree1, ) require.NoError(t, err) @@ -1449,19 +1446,17 @@ func TestTapscriptTreeStore(t *testing.T) { // If we try to fetch a tree with a different root hash, that will not // return an error, but the results should be empty. - dbTree2, err := fetchTapscriptTreeWrapper( - t, ctx, tree2Hash[:], assetStore, - ) + dbTree2, err := fetchTapscriptTreeWrapper(ctx, tree2Hash[:], assetStore) require.Empty(t, dbTree2) require.Nil(t, err) // Trying to delete a tree we haven't inserted yet will not err. - err = deleteTapscriptTreeWrapper(t, ctx, tree2Hash[:], assetStore) + err = deleteTapscriptTreeWrapper(ctx, tree2Hash[:], assetStore) require.Nil(t, err) // Insert the second tree, which has one node already inserted. err = storeTapscriptTreeWrapper( - t, ctx, false, assetStore, tree2Hash[:], tree2, + ctx, false, assetStore, tree2Hash[:], tree2, ) require.NoError(t, err) @@ -1471,7 +1466,7 @@ func TestTapscriptTreeStore(t *testing.T) { // If we delete the first tree, we should still be able to fetch the // second tree intact. - err = deleteTapscriptTreeWrapper(t, ctx, tree1Hash[:], assetStore) + err = deleteTapscriptTreeWrapper(ctx, tree1Hash[:], assetStore) require.NoError(t, err) assertTreeDeletion(t, ctx, tree1Hash[:], assetStore) @@ -1480,7 +1475,7 @@ func TestTapscriptTreeStore(t *testing.T) { // Let's insert the third tree, which contains a node that's a duplicate // of an already-inserted node. err = storeTapscriptTreeWrapper( - t, ctx, false, assetStore, tree3Hash[:], tree3, + ctx, false, assetStore, tree3Hash[:], tree3, ) require.NoError(t, err) @@ -1489,7 +1484,7 @@ func TestTapscriptTreeStore(t *testing.T) { assertStoredTreeEqual(t, ctx, false, assetStore, tree3Hash[:], tree3) // Deleting the third tree should not affect the second tree. - err = deleteTapscriptTreeWrapper(t, ctx, tree3Hash[:], assetStore) + err = deleteTapscriptTreeWrapper(ctx, tree3Hash[:], assetStore) require.NoError(t, err) assertTreeDeletion(t, ctx, tree1Hash[:], assetStore) @@ -1497,7 +1492,7 @@ func TestTapscriptTreeStore(t *testing.T) { // Let's also test handling of tapscript branches. err = storeTapscriptTreeWrapper( - t, ctx, true, assetStore, tree4Hash[:], tree4, + ctx, true, assetStore, tree4Hash[:], tree4, ) require.NoError(t, err) @@ -1505,7 +1500,7 @@ func TestTapscriptTreeStore(t *testing.T) { // The second tapscript branch shares a node with the first. err = storeTapscriptTreeWrapper( - t, ctx, true, assetStore, tree5Hash[:], tree5, + ctx, true, assetStore, tree5Hash[:], tree5, ) require.NoError(t, err) @@ -1513,7 +1508,7 @@ func TestTapscriptTreeStore(t *testing.T) { assertStoredTreeEqual(t, ctx, true, assetStore, tree5Hash[:], tree5) // Deleting the first set of branches should not affect the second. - err = deleteTapscriptTreeWrapper(t, ctx, tree4Hash[:], assetStore) + err = deleteTapscriptTreeWrapper(ctx, tree4Hash[:], assetStore) require.NoError(t, err) assertTreeDeletion(t, ctx, tree4Hash[:], assetStore) diff --git a/tapdb/sqlutils.go b/tapdb/sqlutils.go index f2c781b7d..87ceba97b 100644 --- a/tapdb/sqlutils.go +++ b/tapdb/sqlutils.go @@ -101,7 +101,7 @@ func extractSqlInt32[T constraints.Integer](num sql.NullInt32) T { // NOTE: This function is intended to be used along with the wire.WriteOutPoint // function. Once the ReadOutPoint function is exported, then it can be used in // place of this. -func readOutPoint(r io.Reader, pver uint32, version int32, op *wire.OutPoint) error { +func readOutPoint(r io.Reader, _ uint32, _ uint32, op *wire.OutPoint) error { _, err := io.ReadFull(r, op.Hash[:]) if err != nil { return err From d9e86363af844fd2508f34de9a014d2b3cb50097 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 15:11:34 -0500 Subject: [PATCH 3/7] multi: makes switches exhaustive or simplify --- asset/mock.go | 2 + cmd/tapcli/addrs.go | 3 +- cmd/tapcli/proofs.go | 9 +-- cmd/tapcli/universe.go | 15 ++--- commitment/commitment_test.go | 7 ++- itest/send_test.go | 112 +++++++++------------------------- itest/utils.go | 41 +++++++++++++ mssmt/tree_test.go | 4 +- proof/tx.go | 9 +-- rfq/manager.go | 4 +- rpcserver.go | 3 +- tapdb/migrations.go | 1 + 12 files changed, 98 insertions(+), 112 deletions(-) diff --git a/asset/mock.go b/asset/mock.go index 7630335ae..473abd18e 100644 --- a/asset/mock.go +++ b/asset/mock.go @@ -270,6 +270,8 @@ func SignOutputRaw(priv *btcec.PrivateKey, tx *wire.MsgTx, signDesc.Output.Value, signDesc.Output.PkScript, leaf, signDesc.HashType, privKey, ) + default: + // A witness V0 sign method should never appear here. } if err != nil { return nil, err diff --git a/cmd/tapcli/addrs.go b/cmd/tapcli/addrs.go index 4ffa000d4..9c308350c 100644 --- a/cmd/tapcli/addrs.go +++ b/cmd/tapcli/addrs.go @@ -64,8 +64,7 @@ var newAddrCommand = cli.Command{ } func newAddr(ctx *cli.Context) error { - switch { - case ctx.String(assetIDName) == "": + if ctx.String(assetIDName) == "" { return cli.ShowSubcommandHelp(ctx) } diff --git a/cmd/tapcli/proofs.go b/cmd/tapcli/proofs.go index f9c795c17..294ca7d53 100644 --- a/cmd/tapcli/proofs.go +++ b/cmd/tapcli/proofs.go @@ -59,8 +59,7 @@ var verifyProofCommand = cli.Command{ } func verifyProof(ctx *cli.Context) error { - switch { - case ctx.String(proofPathName) == "": + if ctx.String(proofPathName) == "" { return cli.ShowSubcommandHelp(ctx) } @@ -126,8 +125,7 @@ func decodeProof(ctx *cli.Context) error { client, cleanUp := getClient(ctx) defer cleanUp() - switch { - case !ctx.IsSet(proofPathName): + if !ctx.IsSet(proofPathName) { _ = cli.ShowCommandHelp(ctx, "decode") return nil } @@ -176,8 +174,7 @@ var verifyOwnershipCommand = cli.Command{ } func verifyOwnershipProof(ctx *cli.Context) error { - switch { - case ctx.String(proofPathName) == "": + if ctx.String(proofPathName) == "" { return cli.ShowSubcommandHelp(ctx) } diff --git a/cmd/tapcli/universe.go b/cmd/tapcli/universe.go index 22801f9e3..6c933a138 100644 --- a/cmd/tapcli/universe.go +++ b/cmd/tapcli/universe.go @@ -516,8 +516,7 @@ var universeProofInsertInsert = cli.Command{ } func universeProofInsert(ctx *cli.Context) error { - switch { - case ctx.String(proofPathName) == "": + if ctx.String(proofPathName) == "" { return cli.ShowSubcommandHelp(ctx) } @@ -617,8 +616,7 @@ var universeSyncCommand = cli.Command{ } func universeSync(ctx *cli.Context) error { - switch { - case ctx.String(universeHostName) == "": + if ctx.String(universeHostName) == "" { return cli.ShowSubcommandHelp(ctx) } @@ -721,8 +719,7 @@ var universeFederationAddCommand = cli.Command{ } func universeFederationAdd(ctx *cli.Context) error { - switch { - case ctx.String(universeHostName) == "": + if ctx.String(universeHostName) == "" { return cli.ShowSubcommandHelp(ctx) } @@ -771,9 +768,9 @@ var universeFederationDelCommand = cli.Command{ } func universeFederationDel(ctx *cli.Context) error { - switch { - case ctx.String(universeHostName) == "" && - ctx.Int(universeServerID) == 0: + if ctx.String(universeHostName) == "" && + ctx.Int(universeServerID) == 0 { + return cli.ShowSubcommandHelp(ctx) } diff --git a/commitment/commitment_test.go b/commitment/commitment_test.go index 444cc8fc5..27ba5633b 100644 --- a/commitment/commitment_test.go +++ b/commitment/commitment_test.go @@ -450,15 +450,16 @@ func TestMintAndDeriveTapCommitment(t *testing.T) { var tapCommitment *TapCommitment - if includesAsset && includesAssetGroup { + switch { + case includesAsset && includesAssetGroup: tapCommitment, err = proof.DeriveByAssetInclusion( asset, ) - } else if includesAssetGroup { + case !includesAsset && includesAssetGroup: tapCommitment, err = proof.DeriveByAssetExclusion( asset.AssetCommitmentKey(), ) - } else { + case !includesAsset && !includesAssetGroup: tapCommitment, err = proof.DeriveByAssetCommitmentExclusion( asset.TapCommitmentKey(), ) diff --git a/itest/send_test.go b/itest/send_test.go index a0fa47b8e..6ab120c7c 100644 --- a/itest/send_test.go +++ b/itest/send_test.go @@ -54,20 +54,9 @@ func testBasicSendUnidirectional(t *harnessTest) { broadcastState := tapfreighter.SendStateBroadcast.String() targetEventSelector := func(event *taprpc.SendAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.SendAssetEvent_ExecuteSendStateEvent: - ev := eventTyped.ExecuteSendStateEvent - - // Log send state execution. - timestamp := time.UnixMicro(ev.Timestamp) - t.Logf("Executing send state (%v): %v", - timestamp.Format(time.RFC3339Nano), - ev.SendState) - - return ev.SendState == broadcastState - } - - return false + return AssertSendEventExecuteSendState( + t, event, broadcastState, + ) } timeout := 2 * defaultProofTransferReceiverAckTimeout @@ -168,20 +157,9 @@ func testRestartReceiverCheckBalance(t *harnessTest) { broadcastState := tapfreighter.SendStateBroadcast.String() targetEventSelector := func(event *taprpc.SendAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.SendAssetEvent_ExecuteSendStateEvent: - ev := eventTyped.ExecuteSendStateEvent - - // Log send state execution. - timestamp := time.UnixMicro(ev.Timestamp) - t.Logf("Executing send state (%v): %v", - timestamp.Format(time.RFC3339Nano), - ev.SendState) - - return ev.SendState == broadcastState - } - - return false + return AssertSendEventExecuteSendState( + t, event, broadcastState, + ) } timeout := 2 * defaultProofTransferReceiverAckTimeout @@ -620,18 +598,9 @@ func testReattemptFailedSendHashmailCourier(t *harnessTest) { // Define a target event selector to match the backoff wait // event. This function selects for a specific event type. targetEventSelector := func(event *taprpc.SendAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent: - ev := eventTyped.ProofTransferBackoffWaitEvent - if ev.TransferType != transferTypeSend { - return false - } - - t.Logf("Found event ntfs: %v", ev) - return true - } - - return false + return AssertSendEventProofTransferBackoffWaitTypeSend( + t, event, + ) } // Expected number of events is one less than the number of @@ -727,18 +696,9 @@ func testReattemptFailedSendUniCourier(t *harnessTest) { // Define a target event selector to match the backoff wait // event. This function selects for a specific event type. targetEventSelector := func(event *taprpc.SendAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent: - ev := eventTyped.ProofTransferBackoffWaitEvent - if ev.TransferType != transferTypeSend { - return false - } - - t.Logf("Found event ntfs: %v", ev) - return true - } - - return false + return AssertSendEventProofTransferBackoffWaitTypeSend( + t, event, + ) } // Expected number of events is one less than the number of @@ -889,22 +849,21 @@ func testReattemptFailedReceiveUniCourier(t *harnessTest) { // Define a target event selector to match the backoff wait event. This // function selects for a specific event type. targetEventSelector := func(event *taprpc.ReceiveAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.ReceiveAssetEvent_ProofTransferBackoffWaitEvent: - ev := eventTyped.ProofTransferBackoffWaitEvent - - // We are attempting to identify receive transfer types. - // Skip the event if it is not a receiving transfer - // type. - if ev.TransferType != taprpc.ProofTransferType_PROOF_TRANSFER_TYPE_RECEIVE { - return false - } + ev := event.GetProofTransferBackoffWaitEvent() + if ev == nil { + return false + } - t.Logf("Found event ntfs: %v", ev) - return true + // We are attempting to identify receive transfer types. + // Skip the event if it is not a receiving transfer + // type. + typeRecv := taprpc.ProofTransferType_PROOF_TRANSFER_TYPE_RECEIVE + if ev.TransferType != typeRecv { + return false } - return false + t.Logf("Found event ntfs: %v", ev) + return true } // Expected minimum number of events to receive. @@ -996,23 +955,12 @@ func testOfflineReceiverEventuallyReceives(t *harnessTest) { // Define a target event selector to match the backoff wait // event. This function selects for a specific event type. targetEventSelector := func(event *taprpc.SendAssetEvent) bool { - switch eventTyped := event.Event.(type) { - case *taprpc.SendAssetEvent_ProofTransferBackoffWaitEvent: - ev := eventTyped.ProofTransferBackoffWaitEvent - - // We're listening for events on the sender - // node. We therefore expect to receive - // deliver transfer type backoff wait events - // for sending transfers. - if ev.TransferType != transferTypeSend { - return false - } - - t.Logf("Found event ntfs: %v", ev) - return true - } - - return false + // We're listening for events on the sender node. We + // therefore expect to receive deliver transfer type + // backoff wait events for sending transfers. + return AssertSendEventProofTransferBackoffWaitTypeSend( + t, event, + ) } // Lower bound number of proof delivery attempts. diff --git a/itest/utils.go b/itest/utils.go index 1e2c2e865..2733cdd62 100644 --- a/itest/utils.go +++ b/itest/utils.go @@ -60,6 +60,47 @@ func ParseGenInfo(t *testing.T, genInfo *taprpc.GenesisInfo) *asset.Genesis { return &parsedGenesis } +// AssertSendEventExecuteSendState asserts that the send asset event is an +// ExecuteSendState event, and logs the event timestamp if so. +func AssertSendEventExecuteSendState(t *harnessTest, + event *taprpc.SendAssetEvent, broadcastState string) bool { + + ev := event.GetExecuteSendStateEvent() + if ev == nil { + return false + } + + // Log send state execution. + timestamp := time.UnixMicro(ev.Timestamp) + t.Logf("Executing send state (%v): %v", + timestamp.Format(time.RFC3339Nano), + ev.SendState) + + return ev.SendState == broadcastState +} + +// AssertSendEventProofTransferBackoffWait asserts that the send asset event is +// a ProofTransferBackoffWait event, with the transfer type set as send. +func AssertSendEventProofTransferBackoffWaitTypeSend(t *harnessTest, + event *taprpc.SendAssetEvent) bool { + + ev := event.GetProofTransferBackoffWaitEvent() + if ev == nil { + return false + } + + // We're listening for events on the sender node. We therefore expect to + // receive deliver transfer type backoff wait events for sending + // transfers. + typeSend := taprpc.ProofTransferType_PROOF_TRANSFER_TYPE_SEND + if ev.TransferType != typeSend { + return false + } + + t.Logf("Found event ntfs: %v", ev) + return true +} + // MineBlocks mine 'num' of blocks and check that blocks are present in // node blockchain. numTxs should be set to the number of transactions // (excluding the coinbase) we expect to be included in the first mined block. diff --git a/mssmt/tree_test.go b/mssmt/tree_test.go index 2965b47ef..939571504 100644 --- a/mssmt/tree_test.go +++ b/mssmt/tree_test.go @@ -68,8 +68,8 @@ func genTestStores(t *testing.T) map[string]makeTestTreeStoreFunc { } func printStoreStats(t *testing.T, store mssmt.TreeStore) { - switch s := store.(type) { - case *mssmt.DefaultStore: + s, ok := store.(*mssmt.DefaultStore) + if ok { t.Logf("%s: %s", t.Name(), s.Stats()) } } diff --git a/proof/tx.go b/proof/tx.go index 6772de9ad..e88253a8a 100644 --- a/proof/tx.go +++ b/proof/tx.go @@ -62,12 +62,13 @@ func NewTxMerkleProof(txs []*wire.MsgTx, txIdx int) (*TxMerkleProof, error) { // If we are the left child, a right sibling may not // exist. hash := hashes[currentIdx+1] - if hash != nil { + switch { + case hash != nil: sibling = *hash - } else if hashes[currentIdx] == nil { - return nil, errors.New("invalid merkle tree") - } else { + case hashes[currentIdx] != nil: sibling = *hashes[currentIdx] + default: + return nil, errors.New("invalid merkle tree") } } else { // If we are the right child, there'll always be a left diff --git a/rfq/manager.go b/rfq/manager.go index a558d9529..5cdef69dc 100644 --- a/rfq/manager.go +++ b/rfq/manager.go @@ -296,8 +296,8 @@ func (m *Manager) handleIncomingMessage(incomingMsg rfqmsg.IncomingMsg) error { // messages that will be sent to a peer. func (m *Manager) handleOutgoingMessage(outgoingMsg rfqmsg.OutgoingMsg) error { // Perform type specific handling of the outgoing message. - switch msg := outgoingMsg.(type) { - case *rfqmsg.Accept: + msg, ok := outgoingMsg.(*rfqmsg.Accept) + if ok { // Before sending an accept message to a peer, inform the HTLC // order handler that we've accepted the quote request. m.orderHandler.RegisterChannelRemit(*msg) diff --git a/rpcserver.go b/rpcserver.go index f8ea8e149..4852d135b 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -720,8 +720,7 @@ func (r *rpcServer) checkBalanceOverflow(ctx context.Context, func (r *rpcServer) ListAssets(ctx context.Context, req *taprpc.ListAssetRequest) (*taprpc.ListAssetResponse, error) { - switch { - case req.IncludeSpent && req.IncludeLeased: + if req.IncludeSpent && req.IncludeLeased { return nil, fmt.Errorf("cannot specify both include_spent " + "and include_leased") } diff --git a/tapdb/migrations.go b/tapdb/migrations.go index 7d97c7619..2b6680a7e 100644 --- a/tapdb/migrations.go +++ b/tapdb/migrations.go @@ -59,6 +59,7 @@ func (m *migrationLogger) Printf(format string, v ...interface{}) { m.log.Errorf(format, v...) case btclog.LevelCritical: m.log.Criticalf(format, v...) + case btclog.LevelOff: } } From 3f3dcabc5712fd0163a80ad7ef2c328ee9844b66 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 15:26:49 -0500 Subject: [PATCH 4/7] multi: fix err handling branches flagged by gopls --- proof/courier.go | 3 --- tapfreighter/chain_porter.go | 7 ++----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/proof/courier.go b/proof/courier.go index 431e26cef..a3cbe7b71 100644 --- a/proof/courier.go +++ b/proof/courier.go @@ -1162,9 +1162,6 @@ func (c *UniverseRpcCourier) ReceiveProof(ctx context.Context, receiveFunc := func() error { // Retrieve proof from courier. resp, err := c.client.QueryProof(ctx, &universeKey) - if err != nil { - return err - } if err != nil { return fmt.Errorf("error retrieving proof "+ "from universe courier service: %w", diff --git a/tapfreighter/chain_porter.go b/tapfreighter/chain_porter.go index 85c3e03da..8badf890f 100644 --- a/tapfreighter/chain_porter.go +++ b/tapfreighter/chain_porter.go @@ -657,10 +657,6 @@ func (p *ChainPorter) transferReceiverProof(pkg *sendPackage) error { // Deliver proof to proof courier service. err = courier.DeliverProof(ctx, receiverProof) - if err != nil { - return fmt.Errorf("failed to deliver proof via "+ - "courier service: %w", err) - } // If the proof courier returned a backoff error, then // we'll just return nil here so that we can retry @@ -670,7 +666,8 @@ func (p *ChainPorter) transferReceiverProof(pkg *sendPackage) error { return nil } if err != nil { - return fmt.Errorf("error delivering proof: %w", err) + return fmt.Errorf("failed to deliver proof via "+ + "courier service: %w", err) } return nil From 8d991370b15fc6b0534f752c7fcda1abcfcfeae6 Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 15:28:27 -0500 Subject: [PATCH 5/7] multi: fix conversions + unneeded lambdas --- asset/asset.go | 6 +----- cmd/tapcli/addrs.go | 2 +- commitment/taproot.go | 2 +- itest/mint_batch_stress_test.go | 2 +- itest/psbt_test.go | 2 +- itest/universe_pagination_test.go | 2 +- proof/tx.go | 2 +- rpcserver.go | 2 +- tapdb/addrs.go | 2 +- tapdb/asset_minting.go | 6 ++---- tapdb/multiverse.go | 12 ++++++------ tapdb/universe_federation.go | 2 +- tapfreighter/wallet.go | 2 +- universe/auto_syncer.go | 4 +--- universe_rpc_diff.go | 8 ++++---- 15 files changed, 24 insertions(+), 32 deletions(-) diff --git a/asset/asset.go b/asset/asset.go index 3e477ce82..be24a6ac6 100644 --- a/asset/asset.go +++ b/asset/asset.go @@ -778,11 +778,7 @@ func (g *GroupKey) IsEqual(otherGroupKey *GroupKey) bool { return false } - return slices.EqualFunc( - g.Witness, otherGroupKey.Witness, func(a, b []byte) bool { - return bytes.Equal(a, b) - }, - ) + return slices.EqualFunc(g.Witness, otherGroupKey.Witness, bytes.Equal) } // IsEqualGroup returns true if this group key describes the same asset group diff --git a/cmd/tapcli/addrs.go b/cmd/tapcli/addrs.go index 9c308350c..c0bf2423f 100644 --- a/cmd/tapcli/addrs.go +++ b/cmd/tapcli/addrs.go @@ -162,7 +162,7 @@ func queryAddr(ctx *cli.Context) error { addrs, err := client.QueryAddrs(ctxc, &taprpc.QueryAddrRequest{ CreatedAfter: start, - CreatedBefore: int64(end), + CreatedBefore: end, Limit: int32(ctx.Int64(limitName)), Offset: int32(ctx.Int64(offsetName)), }) diff --git a/commitment/taproot.go b/commitment/taproot.go index 1c4f0014e..79bc2e921 100644 --- a/commitment/taproot.go +++ b/commitment/taproot.go @@ -233,7 +233,7 @@ func (t *TapscriptPreimage) IsEmpty() bool { // Type returns the preimage type. func (t *TapscriptPreimage) Type() TapscriptPreimageType { - return TapscriptPreimageType(t.siblingType) + return t.siblingType } // TapHash returns the tap hash of this preimage according to its type. diff --git a/itest/mint_batch_stress_test.go b/itest/mint_batch_stress_test.go index d4313622d..6846f5636 100644 --- a/itest/mint_batch_stress_test.go +++ b/itest/mint_batch_stress_test.go @@ -121,7 +121,7 @@ func mintBatchStressTest( // Update the asset name and metadata to match an index. incrementMintAsset := func(asset *mintrpc.MintAsset, ind int) { - asset.Name = asset.Name + strconv.Itoa(ind) + asset.Name += strconv.Itoa(ind) binary.PutUvarint(metadataPrefix, uint64(ind)) copy(asset.AssetMeta.Data[0:metaPrefixSize], metadataPrefix) } diff --git a/itest/psbt_test.go b/itest/psbt_test.go index 90f298685..3529e6672 100644 --- a/itest/psbt_test.go +++ b/itest/psbt_test.go @@ -602,7 +602,7 @@ func runPsbtInteractiveSplitSendTest(ctxt context.Context, t *harnessTest, // Swap the sender and receiver nodes starting at the second // iteration. if i > 0 { - sendAmt = sendAmt / 2 + sendAmt /= 2 changeAmt = sendAmt sender, receiver = receiver, sender senderSum, receiverSum = receiverSum, senderSum diff --git a/itest/universe_pagination_test.go b/itest/universe_pagination_test.go index 95568a05c..6a718c024 100644 --- a/itest/universe_pagination_test.go +++ b/itest/universe_pagination_test.go @@ -88,7 +88,7 @@ func mintBatchAssetsTest( // Update the asset name and metadata to match an index. incrementMintAsset := func(asset *mintrpc.MintAsset, ind int) { - asset.Name = asset.Name + strconv.Itoa(ind) + asset.Name += strconv.Itoa(ind) binary.PutUvarint(metadataPrefix, uint64(ind)) copy(asset.AssetMeta.Data[0:metaPrefixSize], metadataPrefix) } diff --git a/proof/tx.go b/proof/tx.go index e88253a8a..e3fec2e72 100644 --- a/proof/tx.go +++ b/proof/tx.go @@ -89,7 +89,7 @@ func NewTxMerkleProof(txs []*wire.MsgTx, txIdx int) (*TxMerkleProof, error) { // Update the currentIdx to reflect the next level in the tree. // We divide by 2 since we always hash in pairs. - currentIdx = currentIdx / 2 + currentIdx /= 2 // We've arrived at the root so our proof is complete. if len(hashes) == 1 { diff --git a/rpcserver.go b/rpcserver.go index 4852d135b..e62201598 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -817,7 +817,7 @@ func (r *rpcServer) listBalancesByAsset(ctx context.Context, resp.AssetBalances[assetIDStr] = &taprpc.AssetBalance{ AssetGenesis: &taprpc.GenesisInfo{ - Version: int32(balance.Version), + Version: balance.Version, GenesisPoint: balance.GenesisPoint.String(), AssetType: taprpc.AssetType(balance.Type), Name: balance.Tag, diff --git a/tapdb/addrs.go b/tapdb/addrs.go index 26921fb25..e04c08c78 100644 --- a/tapdb/addrs.go +++ b/tapdb/addrs.go @@ -288,7 +288,7 @@ func (t *TapAddressBook) InsertAddrs(ctx context.Context, &addr.TaprootOutputKey, ), Amount: int64(addr.Amount), - AssetType: int16(assetGen.AssetType), + AssetType: assetGen.AssetType, CreationTime: addr.CreationTime.UTC(), ProofCourierAddr: proofCourierAddrBytes, }) diff --git a/tapdb/asset_minting.go b/tapdb/asset_minting.go index 78a22e02c..b84e12647 100644 --- a/tapdb/asset_minting.go +++ b/tapdb/asset_minting.go @@ -620,10 +620,8 @@ func fetchAssetSprouts(ctx context.Context, q PendingAssetStore, Index: extractSqlInt32[uint32]( sprout.GroupKeyIndex, ), - Family: keychain.KeyFamily( - extractSqlInt32[keychain.KeyFamily]( - sprout.GroupKeyFamily, - ), + Family: extractSqlInt32[keychain.KeyFamily]( + sprout.GroupKeyFamily, ), }, }, diff --git a/tapdb/multiverse.go b/tapdb/multiverse.go index bd5c9c581..ecf8c1e26 100644 --- a/tapdb/multiverse.go +++ b/tapdb/multiverse.go @@ -225,8 +225,8 @@ func newRootPageQuery(q universe.RootNodesQuery) rootPageQuery { withAmountsById: q.WithAmountsById, leafQuery: leafQuery{ sortDirection: q.SortDirection, - offset: int32(q.Offset), - limit: int32(q.Limit), + offset: q.Offset, + limit: q.Limit, }, } } @@ -401,8 +401,8 @@ type leafQuery struct { func newLeafQuery(q universe.UniverseLeafKeysQuery) leafQuery { return leafQuery{ sortDirection: q.SortDirection, - offset: int32(q.Offset), - limit: int32(q.Limit), + offset: q.Offset, + limit: q.Limit, } } @@ -725,13 +725,13 @@ func (b *MultiverseStore) RootNodes(ctx context.Context, params := sqlc.UniverseRootsParams{ SortDirection: sqlInt16(q.SortDirection), - NumOffset: int32(q.Offset), + NumOffset: q.Offset, NumLimit: func() int32 { if q.Limit == 0 { return universe.MaxPageSize } - return int32(q.Limit) + return q.Limit }(), } diff --git a/tapdb/universe_federation.go b/tapdb/universe_federation.go index 215e5562b..5529d6272 100644 --- a/tapdb/universe_federation.go +++ b/tapdb/universe_federation.go @@ -279,7 +279,7 @@ func (u *UniverseFederationDB) RemoveServers(ctx context.Context, // host string instead. This avoids bugs where a user // doesn't set the ID value, and we try to delete the // very first server. - uniID := int64(a.ID) + uniID := a.ID if a.HostStr() != "" { uniID = -1 } diff --git a/tapfreighter/wallet.go b/tapfreighter/wallet.go index a53a130ff..fe950acd4 100644 --- a/tapfreighter/wallet.go +++ b/tapfreighter/wallet.go @@ -333,7 +333,7 @@ func (s *CoinSelect) selectForAmount(minTotalAmount uint64, // Keep track of the total amount of assets we've seen // so far. - amountSum += uint64(anchoredCommitment.Asset.Amount) + amountSum += anchoredCommitment.Asset.Amount if amountSum >= minTotalAmount { // At this point a target min amount was // specified and has been reached. diff --git a/universe/auto_syncer.go b/universe/auto_syncer.go index acaeba352..a12c05da6 100644 --- a/universe/auto_syncer.go +++ b/universe/auto_syncer.go @@ -141,9 +141,7 @@ func (f *FederationEnvoy) Start() error { // Before we start the main goroutine, we'll add the set of // static Universe servers. addrs := f.cfg.StaticFederationMembers - serverAddrs := fn.Map(addrs, func(a string) ServerAddr { - return NewServerAddrFromStr(a) - }) + serverAddrs := fn.Map(addrs, NewServerAddrFromStr) serverAddrs = fn.Filter(serverAddrs, func(a ServerAddr) bool { // Before we add the server as a federation member, we diff --git a/universe_rpc_diff.go b/universe_rpc_diff.go index cfac22a6e..0324f5c2e 100644 --- a/universe_rpc_diff.go +++ b/universe_rpc_diff.go @@ -82,8 +82,8 @@ func (r *RpcUniverseDiff) RootNodes(ctx context.Context, universeRoots, err := r.conn.AssetRoots( ctx, &unirpc.AssetRootRequest{ WithAmountsById: q.WithAmountsById, - Offset: int32(q.Offset), - Limit: int32(q.Limit), + Offset: q.Offset, + Limit: q.Limit, Direction: unirpc.SortDirection(q.SortDirection), }, ) @@ -133,8 +133,8 @@ func (r *RpcUniverseDiff) UniverseLeafKeys(ctx context.Context, ctx, &unirpc.AssetLeafKeysRequest{ Id: uniID, Direction: unirpc.SortDirection(q.SortDirection), - Offset: int32(q.Offset), - Limit: int32(q.Limit), + Offset: q.Offset, + Limit: q.Limit, }, ) if err != nil { From 95d8f5904b4eadd1ffba3a17f7b568c5ebb11eff Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 16:12:44 -0500 Subject: [PATCH 6/7] multi: nilerr fixes --- tapdb/assets_store.go | 3 ++- tapgarden/mock.go | 4 ++-- universe/auto_syncer.go | 1 + 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tapdb/assets_store.go b/tapdb/assets_store.go index ab1167463..3a0d1f92e 100644 --- a/tapdb/assets_store.go +++ b/tapdb/assets_store.go @@ -1153,7 +1153,8 @@ func (a *AssetStore) FetchAssetProofs(ctx context.Context, // TODO(roasbeef): can modify the query to use IN somewhere // instead? then would take input params and insert into // virtual rows to use - for _, locator := range targetAssets { + for ind := range targetAssets { + locator := targetAssets[ind] args, err := locatorToProofQuery(locator) if err != nil { return err diff --git a/tapgarden/mock.go b/tapgarden/mock.go index 3ff14e347..fec825473 100644 --- a/tapgarden/mock.go +++ b/tapgarden/mock.go @@ -5,7 +5,6 @@ import ( "context" "encoding/hex" "fmt" - "github.com/lightninglabs/taproot-assets/tapsend" "math/rand" "testing" "time" @@ -23,6 +22,7 @@ import ( "github.com/lightninglabs/taproot-assets/asset" "github.com/lightninglabs/taproot-assets/internal/test" "github.com/lightninglabs/taproot-assets/proof" + "github.com/lightninglabs/taproot-assets/tapsend" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lnwallet" @@ -520,7 +520,7 @@ func (m *MockKeyRing) DeriveNextKey(ctx context.Context, priv, err := btcec.NewPrivateKey() if err != nil { - return keychain.KeyDescriptor{}, nil + return keychain.KeyDescriptor{}, err } loc := keychain.KeyLocator{ diff --git a/universe/auto_syncer.go b/universe/auto_syncer.go index a12c05da6..27ab9cbf6 100644 --- a/universe/auto_syncer.go +++ b/universe/auto_syncer.go @@ -848,6 +848,7 @@ func (f *FederationEnvoy) SyncAssetInfo(ctx context.Context, if err != nil { log.Debugf("asset lookup for %v failed with remote"+ "server: %v", assetID.String(), addr.HostStr()) + //lint:ignore nilerr failure is expected and logged return nil } From 734ebf181f8d26136d50a4fa0c43ee05472fad2a Mon Sep 17 00:00:00 2001 From: Jonathan Harvey-Buschel Date: Wed, 28 Feb 2024 17:34:20 -0500 Subject: [PATCH 7/7] tools: ignore vendor dir of builds when linting --- tools/check-go-version-dockerfile.sh | 2 +- tools/check-go-version-yaml.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/check-go-version-dockerfile.sh b/tools/check-go-version-dockerfile.sh index 6a310ddea..4e4fe635d 100755 --- a/tools/check-go-version-dockerfile.sh +++ b/tools/check-go-version-dockerfile.sh @@ -38,7 +38,7 @@ target_go_version="$1" while IFS= read -r -d '' file; do target_files+=("$file") done < <(find . \ - -path ./vendor -prune -o \ + -path ".**/vendor" -prune -o \ -type f \ \( -name "*.Dockerfile" -o -name "Dockerfile" \) \ -print0 \ diff --git a/tools/check-go-version-yaml.sh b/tools/check-go-version-yaml.sh index 9fac2678f..6091dee42 100755 --- a/tools/check-go-version-yaml.sh +++ b/tools/check-go-version-yaml.sh @@ -49,7 +49,7 @@ target_go_version="$1" while IFS= read -r -d '' file; do target_files+=("$file") done < <(find . \ - -path ./vendor -prune -o \ + -path ".**/vendor" -prune -o \ -type f \ \( -name "*.yaml" -o -name "*.yml" \) \ -print0 \