Merge pull request #2483 from jorgemmsilva/fix/txbuilder-consistency

fix(txbuilder): edge case that could cause an inconsistency

packages/vm/errors.go

@@ -7,7 +7,7 @@ import (
 var (
 	ErrOverflow                             = coreerrors.Register("overflow").Create()
 	ErrNotEnoughBaseTokensBalance           = coreerrors.Register("not enough base tokens balance").Create()
-	ErrNotEnoughNativeAssetBalance          = coreerrors.Register("not enough native assets balance").Create()
+	ErrNotEnoughNativeAssetBalance          = coreerrors.Register("not enough native tokens balance").Create()
 	ErrNotEnoughFundsForAllowance           = coreerrors.Register("not enough funds for allowance").Create()
 	ErrCreateFoundryMaxSupplyMustBePositive = coreerrors.Register("max supply must be positive").Create()
 	ErrCreateFoundryMaxSupplyTooBig         = coreerrors.Register("max supply is too big").Create()

packages/vm/vmcontext/vmtxbuilder/nfts.go

@@ -13,7 +13,7 @@ type nftIncluded struct {
 	ID          iotago.NFTID
 	outputID    iotago.OutputID // only available when the input is already accounted for (NFT was deposited in a previous block)
 	in          *iotago.NFTOutput
-	out         *iotago.NFTOutput
+	out         *iotago.NFTOutput // this is not the same as in the `nativeTokenBalance` struct, this can be the accounting output, or the output leaving the chain. // TODO should refactor to follow the same logic so its easier to grok
 	sentOutside bool
 }
 

packages/vm/vmcontext/vmtxbuilder/tokens.go

@@ -15,24 +15,24 @@ import (
 
 // nativeTokenBalance represents on-chain account of the specific native token
 type nativeTokenBalance struct {
-	nativeTokenID iotago.NativeTokenID
-	outputID      iotago.OutputID     // if in != nil, otherwise zeroOutputID
-	in            *iotago.BasicOutput // if nil it means output does not exist, this is new account for the token_id
-	out           *iotago.BasicOutput // current balance of the token_id on the chain
+	nativeTokenID      iotago.NativeTokenID
+	accountingoutputID iotago.OutputID     // if in != nil, otherwise zeroOutputID
+	in                 *iotago.BasicOutput // if nil it means output does not exist, this is new account for the token_id
+	accountingOutput   *iotago.BasicOutput // current balance of the token_id on the chain
 }
 
 func (n *nativeTokenBalance) Clone() *nativeTokenBalance {
 	nativeTokenID := iotago.NativeTokenID{}
 	copy(nativeTokenID[:], n.nativeTokenID[:])
 
 	outputID := iotago.OutputID{}
-	copy(outputID[:], n.outputID[:])
+	copy(outputID[:], n.accountingoutputID[:])
 
 	return &nativeTokenBalance{
-		nativeTokenID: nativeTokenID,
-		outputID:      outputID,
-		in:            cloneInternalBasicOutputOrNil(n.in),
-		out:           cloneInternalBasicOutputOrNil(n.out),
+		nativeTokenID:      nativeTokenID,
+		accountingoutputID: outputID,
+		in:                 cloneInternalBasicOutputOrNil(n.in),
+		accountingOutput:   cloneInternalBasicOutputOrNil(n.accountingOutput),
 	}
 }
 
@@ -59,7 +59,7 @@ func (n *nativeTokenBalance) requiresExistingAccountingUTXOAsInput() bool {
 }
 
 func (n *nativeTokenBalance) getOutValue() *big.Int {
-	return n.out.NativeTokens[0].Amount
+	return n.accountingOutput.NativeTokens[0].Amount
 }
 
 func (n *nativeTokenBalance) add(delta *big.Int) *nativeTokenBalance {
@@ -71,40 +71,40 @@ func (n *nativeTokenBalance) add(delta *big.Int) *nativeTokenBalance {
 	if amount.Cmp(util.MaxUint256) > 0 {
 		panic(vm.ErrOverflow)
 	}
-	n.out.NativeTokens[0].Amount = amount
+	n.accountingOutput.NativeTokens[0].Amount = amount
 	return n
 }
 
 // updateMinSD uptates the resulting output to have the minimum SD
 func (n *nativeTokenBalance) updateMinSD() {
-	minSD := parameters.L1().Protocol.RentStructure.MinRent(n.out)
-	if minSD > n.out.Amount {
+	minSD := parameters.L1().Protocol.RentStructure.MinRent(n.accountingOutput)
+	if minSD > n.accountingOutput.Amount {
 		// sd for internal output can only ever increase
-		n.out.Amount = minSD
+		n.accountingOutput.Amount = minSD
 	}
 }
 
 func (n *nativeTokenBalance) identicalInOut() bool {
 	switch {
-	case n.in == n.out:
+	case n.in == n.accountingOutput:
 		panic("identicalBasicOutputs: internal inconsistency 1")
-	case n.in == nil || n.out == nil:
+	case n.in == nil || n.accountingOutput == nil:
 		return false
-	case !n.in.Ident().Equal(n.out.Ident()):
+	case !n.in.Ident().Equal(n.accountingOutput.Ident()):
 		return false
-	case n.in.Amount != n.out.Amount:
+	case n.in.Amount != n.accountingOutput.Amount:
 		return false
-	case !n.in.NativeTokens.Equal(n.out.NativeTokens):
+	case !n.in.NativeTokens.Equal(n.accountingOutput.NativeTokens):
 		return false
-	case !n.in.Features.Equal(n.out.Features):
+	case !n.in.Features.Equal(n.accountingOutput.Features):
 		return false
 	case len(n.in.NativeTokens) != 1:
 		panic("identicalBasicOutputs: internal inconsistency 2")
-	case len(n.out.NativeTokens) != 1:
+	case len(n.accountingOutput.NativeTokens) != 1:
 		panic("identicalBasicOutputs: internal inconsistency 3")
 	case n.in.NativeTokens[0].ID != n.nativeTokenID:
 		panic("identicalBasicOutputs: internal inconsistency 4")
-	case n.out.NativeTokens[0].ID != n.nativeTokenID:
+	case n.accountingOutput.NativeTokens[0].ID != n.nativeTokenID:
 		panic("identicalBasicOutputs: internal inconsistency 5")
 	}
 	return true
@@ -166,7 +166,7 @@ func (txb *AnchorTransactionBuilder) NativeTokenRecordsToBeUpdated() ([]iotago.N
 func (txb *AnchorTransactionBuilder) NativeTokenOutputsByTokenIDs(ids []iotago.NativeTokenID) map[iotago.NativeTokenID]*iotago.BasicOutput {
 	ret := make(map[iotago.NativeTokenID]*iotago.BasicOutput)
 	for _, id := range ids {
-		ret[id] = txb.balanceNativeTokens[id].out
+		ret[id] = txb.balanceNativeTokens[id].accountingOutput
 	}
 	return ret
 }
@@ -189,15 +189,15 @@ func (txb *AnchorTransactionBuilder) addNativeTokenBalanceDelta(nativeTokenID io
 		// 0 native tokens on the output side
 		if nt.in == nil {
 			// in this case the internar accounting output that would be created is not needed anymore, reiburse the SD
-			return int64(nt.out.Amount)
+			return int64(nt.accountingOutput.Amount)
 		}
 		return int64(nt.in.Amount)
 	}
 
 	// update the SD in case the storage deposit has changed from the last time this output was used
-	oldSD := nt.out.Amount
+	oldSD := nt.accountingOutput.Amount
 	nt.updateMinSD()
-	updatedSD := nt.out.Amount
+	updatedSD := nt.accountingOutput.Amount
 
 	return int64(oldSD) - int64(updatedSD)
 }
@@ -228,10 +228,10 @@ func (txb *AnchorTransactionBuilder) ensureNativeTokenBalance(nativeTokenID iota
 	}
 
 	nativeTokenBalance := &nativeTokenBalance{
-		nativeTokenID: nativeTokenID,
-		outputID:      outputID,
-		in:            basicOutputIn,
-		out:           basicOutputOut,
+		nativeTokenID:      nativeTokenID,
+		accountingoutputID: outputID,
+		in:                 basicOutputIn,
+		accountingOutput:   basicOutputOut,
 	}
 	txb.balanceNativeTokens[nativeTokenID] = nativeTokenBalance
 	return nativeTokenBalance

packages/vm/vmcontext/vmtxbuilder/totals.go

@@ -105,7 +105,7 @@ func (txb *AnchorTransactionBuilder) sumOutputs() *TransactionTotals {
 		s.Add(s, ntb.getOutValue())
 		totals.NativeTokenBalances[id] = s
 		// sum up storage deposit in inputs of internal UTXOs
-		totals.TotalBaseTokensInStorageDeposit += ntb.out.Amount
+		totals.TotalBaseTokensInStorageDeposit += ntb.accountingOutput.Amount
 	}
 	for _, f := range txb.invokedFoundries {
 		if !f.producesAccountingOutput() {

packages/vm/vmcontext/vmtxbuilder/txbuilder.go

@@ -103,10 +103,13 @@ func (txb *AnchorTransactionBuilder) SplitAssetsIntoInternalOutputs(req isc.OnLe
 	for _, nativeToken := range req.Assets().NativeTokens {
 		// ensure this NT is in the txbuilder, update it
 		nt := txb.ensureNativeTokenBalance(nativeToken.ID)
-		sdBefore := nt.out.Amount
+		sdBefore := nt.accountingOutput.Amount
+		if util.IsZeroBigInt(nt.getOutValue()) {
+			sdBefore = 0 // accounting output was zero'ed this block, meaning the existing SD was released
+		}
 		nt.add(nativeToken.Amount)
 		nt.updateMinSD()
-		sdAfter := nt.out.Amount
+		sdAfter := nt.accountingOutput.Amount
 		// user pays for the difference (in case SD has increased, will be the full SD cost if the output is new)
 		requiredSD += sdAfter - sdBefore
 	}
@@ -236,7 +239,7 @@ func (txb *AnchorTransactionBuilder) inputs() (iotago.OutputSet, iotago.OutputID
 	// internal native token outputs
 	for _, nativeTokenBalance := range txb.nativeTokenOutputsSorted() {
 		if nativeTokenBalance.requiresExistingAccountingUTXOAsInput() {
-			outputID := nativeTokenBalance.outputID
+			outputID := nativeTokenBalance.accountingoutputID
 			outputIDs = append(outputIDs, outputID)
 			inputs[outputID] = nativeTokenBalance.in
 		}
@@ -317,7 +320,7 @@ func (txb *AnchorTransactionBuilder) outputs(stateMetadata []byte) iotago.Output
 	nativeTokensToBeUpdated, _ := txb.NativeTokenRecordsToBeUpdated()
 	for _, id := range nativeTokensToBeUpdated {
 		// create one output for each token ID of internal account
-		ret = append(ret, txb.balanceNativeTokens[id].out)
+		ret = append(ret, txb.balanceNativeTokens[id].accountingOutput)
 	}
 	// creating outputs for updated foundries
 	foundriesToBeUpdated, _ := txb.FoundriesToBeUpdated()

packages/vm/vmcontext/vmtxbuilder/txbuilder_test.go

@@ -164,23 +164,20 @@ func TestTxBuilderConsistency(t *testing.T) {
 	}
 	anchorID := tpkg.RandOutputID(0)
 
-	var nativeTokenIDs []iotago.NativeTokenID
-	var txb *AnchorTransactionBuilder
-
-	initTest := func(numTokenIDs int) *mockAccountContractRead {
+	initTest := func(numTokenIDs int) (*AnchorTransactionBuilder, *mockAccountContractRead, []iotago.NativeTokenID) {
 		mockedAccounts := newMockAccountsContractRead(anchor)
-		txb = NewAnchorTransactionBuilder(
+		txb := NewAnchorTransactionBuilder(
 			anchor,
 			anchorID,
 			parameters.L1().Protocol.RentStructure.MinRent(anchor),
 			mockedAccounts.Read(),
 		)
 
-		nativeTokenIDs = make([]iotago.NativeTokenID, 0)
+		nativeTokenIDs := make([]iotago.NativeTokenID, 0)
 		for i := 0; i < numTokenIDs; i++ {
 			nativeTokenIDs = append(nativeTokenIDs, testiotago.RandNativeTokenID())
 		}
-		return mockedAccounts
+		return txb, mockedAccounts, nativeTokenIDs
 	}
 
 	// return deposit in BaseToken
@@ -241,13 +238,10 @@ func TestTxBuilderConsistency(t *testing.T) {
 		const testAmount = 10
 		const numTokenIDs = 4
 
-		mockedAccounts := initTest(numTokenIDs)
+		txb, mockedAccounts, nativeTokenIDs := initTest(numTokenIDs)
 		for i := 0; i < runTimes; i++ {
 			idx := rand.Intn(numTokenIDs)
 			consumeUTXO(t, txb, nativeTokenIDs[idx], testAmount, mockedAccounts)
-
-			txb.BuildTransactionEssence(dummyStateMetadata)
-			txb.MustBalanced()
 		}
 
 		essence, _ := txb.BuildTransactionEssence(dummyStateMetadata)
@@ -258,9 +252,9 @@ func TestTxBuilderConsistency(t *testing.T) {
 		t.Logf("essence bytes len = %d", len(essenceBytes))
 	})
 
-	runConsume := func(numTokenIDs, numRun int, amountNative uint64, mockedAccounts *mockAccountContractRead) {
+	runConsume := func(txb *AnchorTransactionBuilder, nativeTokenIDs []iotago.NativeTokenID, numRun int, amountNative uint64, mockedAccounts *mockAccountContractRead) {
 		for i := 0; i < numRun; i++ {
-			idx := i % numTokenIDs
+			idx := i % len(nativeTokenIDs)
 			consumeUTXO(t, txb, nativeTokenIDs[idx], amountNative, mockedAccounts)
 			txb.BuildTransactionEssence(dummyStateMetadata)
 			txb.MustBalanced()
@@ -272,9 +266,9 @@ func TestTxBuilderConsistency(t *testing.T) {
 		const testAmount = 10
 		const numTokenIDs = 4
 
-		mockedAccounts := initTest(numTokenIDs)
+		txb, mockedAccounts, nativeTokenIDs := initTest(numTokenIDs)
 		err := panicutil.CatchPanicReturnError(func() {
-			runConsume(numTokenIDs, runTimes, testAmount, mockedAccounts)
+			runConsume(txb, nativeTokenIDs, runTimes, testAmount, mockedAccounts)
 		}, vmexceptions.ErrInputLimitExceeded)
 		require.Error(t, err, vmexceptions.ErrInputLimitExceeded)
 
@@ -290,8 +284,8 @@ func TestTxBuilderConsistency(t *testing.T) {
 		const runTimesOutputs = 130
 		const numTokenIDs = 5
 
-		mockedAccounts := initTest(5)
-		runConsume(numTokenIDs, runTimesInputs, 10, mockedAccounts)
+		txb, mockedAccounts, nativeTokenIDs := initTest(numTokenIDs)
+		runConsume(txb, nativeTokenIDs, runTimesInputs, 10, mockedAccounts)
 		txb.BuildTransactionEssence(dummyStateMetadata)
 		txb.MustBalanced()
 
@@ -315,7 +309,7 @@ func TestTxBuilderConsistency(t *testing.T) {
 		const runTimes = 30
 		const numTokenIDs = 5
 
-		mockedAccounts := initTest(numTokenIDs)
+		txb, mockedAccounts, nativeTokenIDs := initTest(numTokenIDs)
 		for _, id := range nativeTokenIDs {
 			consumeUTXO(t, txb, id, 10, mockedAccounts)
 		}
@@ -324,7 +318,9 @@ func TestTxBuilderConsistency(t *testing.T) {
 			idx1 := rand.Intn(numTokenIDs)
 			consumeUTXO(t, txb, nativeTokenIDs[idx1], 1, mockedAccounts)
 			idx2 := rand.Intn(numTokenIDs)
-			addOutput(txb, 1, nativeTokenIDs[idx2], mockedAccounts)
+			if mockedAccounts.assets.AmountNativeToken(nativeTokenIDs[idx2]).Uint64() > 0 {
+				addOutput(txb, 1, nativeTokenIDs[idx2], mockedAccounts)
+			}
 		}
 		essence, _ := txb.BuildTransactionEssence(dummyStateMetadata)
 		txb.MustBalanced()
@@ -337,7 +333,7 @@ func TestTxBuilderConsistency(t *testing.T) {
 		const runTimes = 7
 		const numTokenIDs = 5
 
-		mockedAccounts := initTest(numTokenIDs)
+		txb, mockedAccounts, nativeTokenIDs := initTest(numTokenIDs)
 		for _, id := range nativeTokenIDs {
 			consumeUTXO(t, txb, id, 100, mockedAccounts)
 		}
@@ -357,7 +353,7 @@ func TestTxBuilderConsistency(t *testing.T) {
 		txbClone.BuildTransactionEssence(dummyStateMetadata)
 	})
 	t.Run("send some of the tokens in balance", func(t *testing.T) {
-		mockedAccounts := initTest(5)
+		txb, mockedAccounts, nativeTokenIDs := initTest(5)
 		setNativeTokenAccountsBalance := func(id iotago.NativeTokenID, amount int64) {
 			mockedAccounts.assets.AddNativeTokens(id, amount)
 			// create internal accounting outputs with 0 base tokens (they must be updated in the output side)
@@ -381,6 +377,21 @@ func TestTxBuilderConsistency(t *testing.T) {
 		require.NoError(t, err)
 		t.Logf("essence bytes len = %d", len(essenceBytes))
 	})
+
+	t.Run("test consistency - consume send out, consume again", func(t *testing.T) {
+		txb, mockedAccounts, nativeTokenIDs := initTest(1)
+		tokenID := nativeTokenIDs[0]
+		consumeUTXO(t, txb, tokenID, 1, mockedAccounts)
+		addOutput(txb, 1, tokenID, mockedAccounts)
+		consumeUTXO(t, txb, tokenID, 1, mockedAccounts)
+
+		essence, _ := txb.BuildTransactionEssence(dummyStateMetadata)
+		txb.MustBalanced()
+
+		essenceBytes, err := essence.Serialize(serializer.DeSeriModeNoValidation, nil)
+		require.NoError(t, err)
+		t.Logf("essence bytes len = %d", len(essenceBytes))
+	})
 }
 
 func TestFoundries(t *testing.T) {