CLI: Improve minting user experience (#492)

* multi: return full batch on mint and finalize

* tapgarden: give synchronous feedback for batch finalization

Before, the batch finalization would always happen in the background and
only the state of the batch before freezing it would be returned.
With this change we actually wait for the batch transaction to be
created and broadcast before returning to the caller. This allows us to
return an updated state of the batch or an error in case one occurs.
This also prevents us from setting the pending batch to nil even if it
fails to be created (for example if there are no funds to create the
mint transaction). So a finalizatino can be re-attempted in case of a
temporary error.

* itest: fix batch state validation

* tapdb/sqlc: add asset_id to QueryUniverseLeaves query

* multi: add grouped assets to universe roots call

* taprpc: rename Batch to PendingBatch for Mint call

* taprpc+tapgarden: obtain pending batch directly from response chan

We can skip the on chain disk hit and instead can just rely on the
planter to send us the batch directly.

---------

Co-authored-by: Olaoluwa Osuntokun <[email protected]>

cmd/tapcli/assets.go

@@ -45,6 +45,7 @@ var (
 	batchKeyName          = "batch_key"
 	groupByGroupName      = "by_group"
 	assetIDName           = "asset_id"
+	shortResponseName     = "short"
 )
 
 var mintAssetCommand = cli.Command{
@@ -54,8 +55,9 @@ var mintAssetCommand = cli.Command{
 	Description: "Attempt to mint a new asset with the specified parameters",
 	Flags: []cli.Flag{
 		cli.StringFlag{
-			Name:  assetTypeName,
-			Usage: "the type of asset, must either be: normal, or collectible",
+			Name: assetTypeName,
+			Usage: "the type of asset, must either be: normal, " +
+				"or collectible",
 		},
 		cli.StringFlag{
 			Name:  assetTagName,
@@ -84,12 +86,21 @@ var mintAssetCommand = cli.Command{
 				"emission",
 		},
 		cli.StringFlag{
-			Name:  assetGroupKeyName,
-			Usage: "the specific group key to use to mint the asset",
+			Name: assetGroupKeyName,
+			Usage: "the specific group key to use to mint the " +
+				"asset",
 		},
 		cli.StringFlag{
-			Name:  assetGroupAnchorName,
-			Usage: "the other asset in this batch that the new asset be grouped with",
+			Name: assetGroupAnchorName,
+			Usage: "the other asset in this batch that the new " +
+				"asset be grouped with",
+		},
+		cli.BoolFlag{
+			Name: shortResponseName,
+			Usage: "if true, then the current assets within the " +
+				"batch will not be returned in the response " +
+				"in order to avoid printing a large amount " +
+				"of data in case of large batches",
 		},
 	},
 	Action: mintAsset,
@@ -173,6 +184,7 @@ func mintAsset(ctx *cli.Context) error {
 			GroupAnchor: ctx.String(assetGroupAnchorName),
 		},
 		EnableEmission: ctx.Bool(assetEmissionName),
+		ShortResponse:  ctx.Bool(shortResponseName),
 	})
 	if err != nil {
 		return fmt.Errorf("unable to mint asset: %w", err)
@@ -187,15 +199,26 @@ var finalizeBatchCommand = cli.Command{
 	ShortName:   "f",
 	Usage:       "finalize a batch",
 	Description: "Attempt to finalize a pending batch.",
-	Action:      finalizeBatch,
+	Flags: []cli.Flag{
+		cli.BoolFlag{
+			Name: shortResponseName,
+			Usage: "if true, then the current assets within the " +
+				"batch will not be returned in the response " +
+				"in order to avoid printing a large amount " +
+				"of data in case of large batches",
+		},
+	},
+	Action: finalizeBatch,
 }
 
 func finalizeBatch(ctx *cli.Context) error {
 	ctxc := getContext()
 	client, cleanUp := getMintClient(ctx)
 	defer cleanUp()
 
-	resp, err := client.FinalizeBatch(ctxc, &mintrpc.FinalizeBatchRequest{})
+	resp, err := client.FinalizeBatch(ctxc, &mintrpc.FinalizeBatchRequest{
+		ShortResponse: ctx.Bool(shortResponseName),
+	})
 	if err != nil {
 		return fmt.Errorf("unable to finalize batch: %w", err)
 	}

itest/assertions.go

@@ -141,41 +141,34 @@ func AssertAssetState(t *testing.T, assets map[string][]*taprpc.Asset,
 }
 
 // WaitForBatchState polls until the planter has reached the desired state with
-// the current batch.
+// the given batch.
 func WaitForBatchState(t *testing.T, ctx context.Context,
-	client mintrpc.MintClient, timeout time.Duration,
-	targetState mintrpc.BatchState) bool {
+	client mintrpc.MintClient, timeout time.Duration, batchKey []byte,
+	targetState mintrpc.BatchState) {
 
-	breakTimeout := time.After(timeout)
-	ticker := time.NewTicker(50 * time.Millisecond)
-	defer ticker.Stop()
-
-	isTargetState := func(b *mintrpc.MintingBatch) bool {
-		return b.State == targetState
-	}
-
-	batchCount := func() int {
+	err := wait.NoError(func() error {
 		batchResp, err := client.ListBatches(
-			ctx, &mintrpc.ListBatchRequest{},
+			ctx, &mintrpc.ListBatchRequest{
+				Filter: &mintrpc.ListBatchRequest_BatchKey{
+					BatchKey: batchKey,
+				},
+			},
 		)
 		require.NoError(t, err)
 
-		return fn.Count(batchResp.Batches, isTargetState)
-	}
-
-	initialBatchCount := batchCount()
+		if len(batchResp.Batches) != 1 {
+			return fmt.Errorf("expected one batch, got %d",
+				len(batchResp.Batches))
+		}
 
-	for {
-		select {
-		case <-breakTimeout:
-			return false
-		case <-ticker.C:
-			currentBatchCount := batchCount()
-			if currentBatchCount-initialBatchCount == 1 {
-				return true
-			}
+		if batchResp.Batches[0].State != targetState {
+			return fmt.Errorf("expected batch state %v, got %v",
+				targetState, batchResp.Batches[0].State)
 		}
-	}
+
+		return nil
+	}, timeout)
+	require.NoError(t, err)
 }
 
 // CommitmentKey returns the asset's commitment key given an RPC asset

itest/assets_test.go

@@ -156,7 +156,7 @@ func withMintingTimeout(timeout time.Duration) mintOption {
 // block.
 func mintAssetUnconfirmed(t *harnessTest, tapd *tapdHarness,
 	assetRequests []*mintrpc.MintAssetRequest,
-	opts ...mintOption) chainhash.Hash {
+	opts ...mintOption) (chainhash.Hash, []byte) {
 
 	options := defaultMintOptions()
 	for _, opt := range opts {
@@ -168,21 +168,28 @@ func mintAssetUnconfirmed(t *harnessTest, tapd *tapdHarness,
 	defer cancel()
 
 	// Mint all the assets in the same batch.
-	for _, assetRequest := range assetRequests {
+	for idx, assetRequest := range assetRequests {
 		assetResp, err := tapd.MintAsset(ctxt, assetRequest)
 		require.NoError(t.t, err)
-		require.NotEmpty(t.t, assetResp.BatchKey)
+		require.NotEmpty(t.t, assetResp.PendingBatch)
+		require.Len(t.t, assetResp.PendingBatch.Assets, idx+1)
 	}
 
 	// Instruct the daemon to finalize the batch.
 	batchResp, err := tapd.FinalizeBatch(
 		ctxt, &mintrpc.FinalizeBatchRequest{},
 	)
 	require.NoError(t.t, err)
-	require.NotEmpty(t.t, batchResp.BatchKey)
+	require.NotEmpty(t.t, batchResp.Batch)
+	require.Len(t.t, batchResp.Batch.Assets, len(assetRequests))
+	require.Equal(
+		t.t, mintrpc.BatchState_BATCH_STATE_BROADCAST,
+		batchResp.Batch.State,
+	)
 
 	WaitForBatchState(
 		t.t, ctxt, tapd, options.mintingTimeout,
+		batchResp.Batch.BatchKey,
 		mintrpc.BatchState_BATCH_STATE_BROADCAST,
 	)
 	hashes, err := waitForNTxsInMempool(
@@ -212,7 +219,7 @@ func mintAssetUnconfirmed(t *harnessTest, tapd *tapdHarness,
 		)
 	}
 
-	return *hashes[0]
+	return *hashes[0], batchResp.Batch.BatchKey
 }
 
 // mintAssetsConfirmBatch mints all given assets in the same batch, confirms the
@@ -221,7 +228,9 @@ func mintAssetsConfirmBatch(t *harnessTest, tapd *tapdHarness,
 	assetRequests []*mintrpc.MintAssetRequest,
 	opts ...mintOption) []*taprpc.Asset {
 
-	mintTXID := mintAssetUnconfirmed(t, tapd, assetRequests, opts...)
+	mintTXID, batchKey := mintAssetUnconfirmed(
+		t, tapd, assetRequests, opts...,
+	)
 
 	options := defaultMintOptions()
 	for _, opt := range opts {
@@ -236,7 +245,7 @@ func mintAssetsConfirmBatch(t *harnessTest, tapd *tapdHarness,
 	block := mineBlocks(t, t.lndHarness, 1, 1)[0]
 	blockHash := block.BlockHash()
 	WaitForBatchState(
-		t.t, ctxt, tapd, options.mintingTimeout,
+		t.t, ctxt, tapd, options.mintingTimeout, batchKey,
 		mintrpc.BatchState_BATCH_STATE_FINALIZED,
 	)
 
@@ -541,9 +550,11 @@ func testMintAssetNameCollisionError(t *harnessTest) {
 
 	// If we attempt to add both assets to the same batch, the second mint
 	// call should fail.
-	collideBatchKey, err := t.tapd.MintAsset(ctxt, &assetCollide)
+	collideResp, err := t.tapd.MintAsset(ctxt, &assetCollide)
 	require.NoError(t.t, err)
-	require.NotNil(t.t, collideBatchKey.BatchKey)
+	require.NotNil(t.t, collideResp.PendingBatch)
+	require.NotNil(t.t, collideResp.PendingBatch.BatchKey)
+	require.Len(t.t, collideResp.PendingBatch.Assets, 1)
 
 	_, batchNameErr := t.tapd.MintAsset(ctxt, &assetMint)
 	require.ErrorContains(t.t, batchNameErr, "already in batch")
@@ -575,14 +586,16 @@ func testMintAssetNameCollisionError(t *harnessTest) {
 		ctxt, &mintrpc.CancelBatchRequest{},
 	)
 	require.NoError(t.t, err)
-	require.Equal(t.t, cancelBatchKey.BatchKey, collideBatchKey.BatchKey)
+	require.Equal(
+		t.t, cancelBatchKey.BatchKey, collideResp.PendingBatch.BatchKey,
+	)
 
 	// The only change in the returned batch after cancellation should be
 	// the batch state.
 	cancelBatch, err := t.tapd.ListBatches(
 		ctxt, &mintrpc.ListBatchRequest{
 			Filter: &mintrpc.ListBatchRequest_BatchKey{
-				BatchKey: collideBatchKey.BatchKey,
+				BatchKey: collideResp.PendingBatch.BatchKey,
 			},
 		})
 	require.NoError(t.t, err)

itest/re-org_test.go

@@ -19,7 +19,7 @@ func testReOrgMint(t *harnessTest) {
 	mintRequests := []*mintrpc.MintAssetRequest{
 		issuableAssets[0], issuableAssets[1],
 	}
-	mintTXID := mintAssetUnconfirmed(t, t.tapd, mintRequests)
+	mintTXID, batchKey := mintAssetUnconfirmed(t, t.tapd, mintRequests)
 
 	ctxb := context.Background()
 	ctxt, cancel := context.WithTimeout(ctxb, defaultWaitTimeout)
@@ -34,7 +34,7 @@ func testReOrgMint(t *harnessTest) {
 	initialBlock := mineBlocks(t, t.lndHarness, 1, 1)[0]
 	initialBlockHash := initialBlock.BlockHash()
 	WaitForBatchState(
-		t.t, ctxt, t.tapd, defaultWaitTimeout,
+		t.t, ctxt, t.tapd, defaultWaitTimeout, batchKey,
 		mintrpc.BatchState_BATCH_STATE_FINALIZED,
 	)
 

itest/universe_test.go

@@ -285,9 +285,9 @@ func testUniverseREST(t *harnessTest) {
 			fmt.Sprintf("%s/roots/asset-id/%s", urlPrefix, assetID),
 		)
 		require.NoError(t.t, err)
-		require.Equal(
-			t.t, roots.UniverseRoots[assetID], assetRoot.AssetRoot,
-		)
+		require.True(t.t, AssertUniverseRootEqual(
+			roots.UniverseRoots[assetID], assetRoot.AssetRoot,
+		))
 	}
 
 	// Re-issuable assets are keyed by their group keys.
@@ -311,10 +311,10 @@ func testUniverseREST(t *harnessTest) {
 		assetRoot, err := getJSON[*unirpc.QueryRootResponse](queryURI)
 		require.NoError(t.t, err)
 
-		require.Equal(
-			t.t, roots.UniverseRoots[groupKeyID],
+		require.True(t.t, AssertUniverseRootEqual(
+			roots.UniverseRoots[groupKeyID],
 			assetRoot.AssetRoot,
-		)
+		))
 	}
 }