From f541d4804556c866e6dffd8f62c06cfd4d626eed Mon Sep 17 00:00:00 2001 From: ffranr Date: Mon, 4 Nov 2024 14:43:21 +0000 Subject: [PATCH 1/5] rpc: improve log message clarity --- rpcserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpcserver.go b/rpcserver.go index b53af7697..4da523b1e 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -600,7 +600,7 @@ func (r *rpcServer) MintAsset(ctx context.Context, } rpcsLog.Infof("[MintAsset]: version=%v, type=%v, name=%v, amt=%v, "+ - "issuance=%v", seedling.AssetVersion, seedling.AssetType, + "enable_emission=%v", seedling.AssetVersion, seedling.AssetType, seedling.AssetName, seedling.Amount, seedling.EnableEmission) if scriptKey != nil { From b706ade0628cf8a1c6357df591cdf4c76019fe72 Mon Sep 17 00:00:00 2001 From: ffranr Date: Tue, 5 Nov 2024 13:08:18 +0000 Subject: [PATCH 2/5] tapgardner: refine comments Remove inaccurate statement in `prepAssetSeedling` doc comment. Improve clarity of comment at `prepAssetSeedling` call site. --- tapgarden/caretaker.go | 4 ++-- tapgarden/planter.go | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tapgarden/caretaker.go b/tapgarden/caretaker.go index 6968251f0..06e4968aa 100644 --- a/tapgarden/caretaker.go +++ b/tapgarden/caretaker.go @@ -59,10 +59,10 @@ const ( // BatchCaretakerConfig houses all the items that the BatchCaretaker needs to // carry out its duties. type BatchCaretakerConfig struct { - // Batch is the minting batch that this caretaker is responsible for? + // Batch is the minting batch that this caretaker is responsible for. Batch *MintingBatch - // BatchFeeRate is an optional manually-set feerate specified when + // BatchFeeRate is an optional manually-set fee rate specified when // finalizing a batch. BatchFeeRate *chainfee.SatPerKWeight diff --git a/tapgarden/planter.go b/tapgarden/planter.go index e1c2fd6d0..5a97280cd 100644 --- a/tapgarden/planter.go +++ b/tapgarden/planter.go @@ -1437,6 +1437,10 @@ func (c *ChainPlanter) gardener() { // After some basic validation, prepare the asset // seedling (soon to be a sprout) by committing it to // disk as part of the latest batch. + // + // This method will also include the seedling in any + // existing pending batch or create a new pending batch + // if necessary. ctx, cancel := c.WithCtxQuit() err := c.prepAssetSeedling(ctx, req) cancel() @@ -2223,8 +2227,7 @@ func (c *ChainPlanter) CancelBatch() (*btcec.PublicKey, error) { } // prepAssetSeedling performs some basic validation for the Seedling, then -// either adds it to an existing pending batch or creates a new batch for it. A -// bool indicating if a new batch should immediately be created is returned. +// either adds it to an existing pending batch or creates a new batch for it. func (c *ChainPlanter) prepAssetSeedling(ctx context.Context, req *Seedling) error { From 2bda40c31bcce94746262eee9b2258f12602f116 Mon Sep 17 00:00:00 2001 From: ffranr Date: Tue, 5 Nov 2024 13:45:57 +0000 Subject: [PATCH 3/5] tapgardner: clarify PendingBatch docs and test Update doc comment for PendingBatch to improve clarity. Test to verify that nil is returned when no pending batch exists. --- tapgarden/planter.go | 4 ++-- tapgarden/planter_test.go | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tapgarden/planter.go b/tapgarden/planter.go index 5a97280cd..a65f6255c 100644 --- a/tapgarden/planter.go +++ b/tapgarden/planter.go @@ -2140,8 +2140,8 @@ func (c *ChainPlanter) finalizeBatch(params FinalizeParams) (*BatchCaretaker, return caretaker, nil } -// PendingBatch returns the current pending batch. If there's no pending batch, -// then an error is returned. +// PendingBatch returns the current pending batch, or nil if no batch is +// pending. func (c *ChainPlanter) PendingBatch() (*MintingBatch, error) { req := newStateReq[*MintingBatch](reqTypePendingBatch) diff --git a/tapgarden/planter_test.go b/tapgarden/planter_test.go index 4b3f35ce6..8c84418eb 100644 --- a/tapgarden/planter_test.go +++ b/tapgarden/planter_test.go @@ -1658,6 +1658,12 @@ func testFundSealBeforeFinalize(t *mintingTestHarness) { // harness. t.refreshChainPlanter() + // A pending batch should not exist yet. Therefore, `PendingBatch` + // should return nil and no error. + batch, err := t.planter.PendingBatch() + require.Nil(t, batch) + require.NoError(t, err) + var ( wg sync.WaitGroup respChan = make(chan *FundBatchResp, 1) From 25d341bb504387c50ead816d2cee96bd1d6667ba Mon Sep 17 00:00:00 2001 From: ffranr Date: Fri, 24 Jan 2025 14:45:50 +0000 Subject: [PATCH 4/5] tapgarden: add nil check for group anchor during validation Introduce a nil pointer check for group anchors to safeguard against potential nil pointer dereference during validation. While all current call sites already perform this check, this change is preemptive and should not have any functional impact at this time. --- tapgarden/batch.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tapgarden/batch.go b/tapgarden/batch.go index 635e2bce6..326e2e07d 100644 --- a/tapgarden/batch.go +++ b/tapgarden/batch.go @@ -131,6 +131,10 @@ func (m *MintingBatch) Copy() *MintingBatch { // validateGroupAnchor checks if the group anchor for a seedling is valid. // A valid anchor must already be part of the batch and have emission enabled. func (m *MintingBatch) validateGroupAnchor(s *Seedling) error { + if s.GroupAnchor == nil { + return fmt.Errorf("group anchor unspecified") + } + anchor, ok := m.Seedlings[*s.GroupAnchor] if anchor == nil || !ok { From 01eb2f64314256d5fbad0901dc586834269ecbd3 Mon Sep 17 00:00:00 2001 From: ffranr Date: Fri, 24 Jan 2025 15:18:47 +0000 Subject: [PATCH 5/5] tapgarden: enforce output count in extractAnchorOutputIndex Update extractAnchorOutputIndex to raise an error when the number of unsigned transaction outputs is not exactly two. The function logic assumes two outputs, so this change preemptively guards against potential developer errors if additional outputs are ever introduced. --- tapgarden/caretaker.go | 18 +++++++++++++----- tapgarden/planter.go | 18 +++++++++++++++--- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/tapgarden/caretaker.go b/tapgarden/caretaker.go index 06e4968aa..633c958e6 100644 --- a/tapgarden/caretaker.go +++ b/tapgarden/caretaker.go @@ -414,16 +414,20 @@ func (b *BatchCaretaker) assetCultivator() { // extractAnchorOutputIndex extracts the anchor output index from a funded // genesis packet. -func extractAnchorOutputIndex(genesisPkt *tapsend.FundedPsbt) uint32 { +func extractAnchorOutputIndex(genesisPkt *tapsend.FundedPsbt) (uint32, error) { + if len(genesisPkt.Pkt.UnsignedTx.TxOut) != 2 { + return 0, fmt.Errorf("funded genesis packet has unexpected "+ + "number of outputs, expected 2 (txout_len=%d)", + len(genesisPkt.Pkt.UnsignedTx.TxOut)) + } + anchorOutputIndex := uint32(0) - // TODO(jhb): Does funding guarantee that minting TXs always have - // exactly two outputs? If not this func should be fallible. if genesisPkt.ChangeOutputIndex == 0 { anchorOutputIndex = 1 } - return anchorOutputIndex + return anchorOutputIndex, nil } // extractGenesisOutpoint extracts the genesis point (the first input from the @@ -614,9 +618,13 @@ func (b *BatchCaretaker) stateStep(currentState BatchState) (BatchState, error) // and vice versa. // TODO(jhb): return the anchor index instead of change? or both // so this works for N outputs - b.anchorOutputIndex = extractAnchorOutputIndex( + b.anchorOutputIndex, err = extractAnchorOutputIndex( b.cfg.Batch.GenesisPacket, ) + if err != nil { + return 0, err + } + genesisPoint := extractGenesisOutpoint(genesisTxPkt.UnsignedTx) // First, we'll turn all the seedlings into actual taproot assets. diff --git a/tapgarden/planter.go b/tapgarden/planter.go index a65f6255c..5d7bbf42b 100644 --- a/tapgarden/planter.go +++ b/tapgarden/planter.go @@ -1032,7 +1032,11 @@ func fetchFinalizedBatch(ctx context.Context, batchStore MintingStore, // Collect genesis TX information from the batch to build the proof // locators. - anchorOutputIndex := extractAnchorOutputIndex(batch.GenesisPacket) + anchorOutputIndex, err := extractAnchorOutputIndex(batch.GenesisPacket) + if err != nil { + return nil, err + } + signedTx, err := psbt.Extract(batch.GenesisPacket.Pkt) if err != nil { return nil, err @@ -1268,9 +1272,13 @@ func newVerboseBatch(currentBatch *MintingBatch, // Before we can build the group key requests for each seedling, we must // fetch the genesis point and anchor index for the batch. - anchorOutputIndex := extractAnchorOutputIndex( + anchorOutputIndex, err := extractAnchorOutputIndex( currentBatch.GenesisPacket, ) + if err != nil { + return nil, err + } + genesisPoint := extractGenesisOutpoint( currentBatch.GenesisPacket.Pkt.UnsignedTx, ) @@ -1847,9 +1855,13 @@ func (c *ChainPlanter) sealBatch(ctx context.Context, params SealParams, // Before we can build the group key requests for each seedling, we must // fetch the genesis point and anchor index for the batch. - anchorOutputIndex := extractAnchorOutputIndex( + anchorOutputIndex, err := extractAnchorOutputIndex( workingBatch.GenesisPacket, ) + if err != nil { + return nil, err + } + genesisPoint := extractGenesisOutpoint( workingBatch.GenesisPacket.Pkt.UnsignedTx, )