Feat: added more reliable fails in test case

dapps/valuetransfers/packages/tangle/tangle.go

@@ -1151,7 +1151,7 @@ func (tangle *Tangle) processSolidificationStackEntry(solidificationStack *list.
 	}
 
 	// abort if the payload is not solid or invalid
-	payloadSolid, payloadSolidityErr := tangle.checkPayloadSolidity(currentPayload, currentPayloadMetadata, consumedBranches)
+	payloadSolid, payloadSolidityErr := tangle.payloadBecameNewlySolid(currentPayload, currentPayloadMetadata, consumedBranches)
 	if payloadSolidityErr != nil {
 		tangle.Events.PayloadInvalid.Trigger(solidificationStackEntry.CachedPayload, solidificationStackEntry.CachedPayloadMetadata, payloadSolidityErr)
 
@@ -1164,7 +1164,7 @@ func (tangle *Tangle) processSolidificationStackEntry(solidificationStack *list.
 	}
 
 	// book the solid entities
-	transactionBooked, payloadBooked, decisionPending, bookingErr := tangle.book(solidificationStackEntry.Retain())
+	transactionBooked, _, decisionPending, bookingErr := tangle.book(solidificationStackEntry.Retain())
 	if bookingErr != nil {
 		tangle.Events.Error.Trigger(bookingErr)
 
@@ -1177,12 +1177,9 @@ func (tangle *Tangle) processSolidificationStackEntry(solidificationStack *list.
 	// trigger events and schedule check of approvers / consumers
 	if transactionBooked {
 		tangle.Events.TransactionBooked.Trigger(solidificationStackEntry.CachedTransaction, solidificationStackEntry.CachedTransactionMetadata, decisionPending)
-
-		tangle.ForEachConsumers(currentTransaction, tangle.createValuePayloadFutureConeIterator(solidificationStack, processedPayloads))
-	}
-	if payloadBooked {
-		tangle.ForeachApprovers(currentPayload.ID(), tangle.createValuePayloadFutureConeIterator(solidificationStack, processedPayloads))
 	}
+	tangle.ForEachConsumers(currentTransaction, tangle.createValuePayloadFutureConeIterator(solidificationStack, processedPayloads))
+	tangle.ForeachApprovers(currentPayload.ID(), tangle.createValuePayloadFutureConeIterator(solidificationStack, processedPayloads))
 }
 
 func (tangle *Tangle) book(entitiesToBook *valuePayloadPropagationStackEntry) (transactionBooked bool, payloadBooked bool, decisionPending bool, err error) {
@@ -1396,15 +1393,15 @@ func (tangle *Tangle) payloadBranchID(payloadID payload.ID) branchmanager.Branch
 	return payloadMetadata.BranchID()
 }
 
-// checkPayloadSolidity returns true if the given payload is solid. A payload is considered to be solid, if it is either
-// already marked as solid or if its referenced payloads are marked as solid.
-func (tangle *Tangle) checkPayloadSolidity(p *payload.Payload, payloadMetadata *PayloadMetadata, transactionBranches []branchmanager.BranchID) (solid bool, err error) {
+// payloadBecameNewlySolid returns true if the given payload is solid but was not marked as solid. yet.
+func (tangle *Tangle) payloadBecameNewlySolid(p *payload.Payload, payloadMetadata *PayloadMetadata, transactionBranches []branchmanager.BranchID) (solid bool, err error) {
+	// abort if the payload was deleted
 	if p == nil || p.IsDeleted() || payloadMetadata == nil || payloadMetadata.IsDeleted() {
 		return
 	}
 
+	// abort if the payload was marked as solid already
 	if payloadMetadata.IsSolid() {
-		solid = payloadMetadata.BranchID() != branchmanager.UndefinedBranchID
 		return
 	}
 

dapps/valuetransfers/packages/tangle/tangle_concurrency_test.go

@@ -5,16 +5,17 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/iotaledger/hive.go/events"
+	"github.com/iotaledger/hive.go/kvstore/mapdb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/address"
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/balance"
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/branchmanager"
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/payload"
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/tipmanager"
 	"github.com/iotaledger/goshimmer/dapps/valuetransfers/packages/transaction"
-	"github.com/iotaledger/hive.go/events"
-	"github.com/iotaledger/hive.go/kvstore/mapdb"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestConcurrency(t *testing.T) {
@@ -214,143 +215,146 @@ func TestReverseValueObjectSolidification(t *testing.T) {
 }
 
 func TestReverseTransactionSolidification(t *testing.T) {
-	// img/reverse-transaction-solidification.png
-	// Builds a UTXO-DAG with `txChains` spending outputs from the corresponding chain.
-	// All value objects reference the previous value object, effectively creating a chain.
-	// The test attaches the prepared value objects concurrently in reverse order.
-	//name, err := ioutil.TempDir("", "example")
-	//require.NoError(t, err)
-	//bDB, err := database.NewDB(name)
-	//require.NoError(t, err)
-	//
-	//tangle := New(bDB.NewStore())
-	tangle := New(mapdb.NewMapDB())
-
-	tangle.Events.Error.Attach(events.NewClosure(func(err error) {
-		fmt.Println(err)
-	}))
-
-	tangle.Events.TransactionInvalid.Attach(events.NewClosure(func(_ *transaction.CachedTransaction, _ *CachedTransactionMetadata, err error) {
-		panic(err)
-	}))
-	tangle.Events.PayloadInvalid.Attach(events.NewClosure(func(_ *payload.CachedPayload, _ *CachedPayloadMetadata, err error) {
-		panic(err)
-	}))
-
-	tipManager := tipmanager.New()
-
-	txChains := 1
-	count := 100
-	threads := 10
-	countTotal := txChains * threads * count
+	testIterations := 500
+
+	// repeat the test a few times
+	for k := 0; k < testIterations; k++ {
+		// img/reverse-transaction-solidification.png
+		// Builds a UTXO-DAG with `txChains` spending outputs from the corresponding chain.
+		// All value objects reference the previous value object, effectively creating a chain.
+		// The test attaches the prepared value objects concurrently in reverse order.
+		//name, err := ioutil.TempDir("", "example")
+		//require.NoError(t, err)
+		//bDB, err := database.NewDB(name)
+		//require.NoError(t, err)
+		//
+		//tangle := New(bDB.NewStore())
+		tangle := New(mapdb.NewMapDB())
+
+		tangle.Events.Error.Attach(events.NewClosure(func(err error) {
+			fmt.Println(err)
+		}))
 
-	// initialize tangle with genesis block
-	outputs := make(map[address.Address][]*balance.Balance)
-	for i := 0; i < txChains; i++ {
-		outputs[address.Random()] = []*balance.Balance{
-			balance.New(balance.ColorIOTA, 1),
-		}
-	}
-	inputIDs := loadSnapshotFromOutputs(tangle, outputs)
+		tangle.Events.TransactionInvalid.Attach(events.NewClosure(func(_ *transaction.CachedTransaction, _ *CachedTransactionMetadata, err error) {
+			panic(err)
+		}))
+		tangle.Events.PayloadInvalid.Attach(events.NewClosure(func(_ *payload.CachedPayload, _ *CachedPayloadMetadata, err error) {
+			panic(err)
+		}))
 
-	transactions := make([]*transaction.Transaction, countTotal)
-	valueObjects := make([]*payload.Payload, countTotal)
+		tipManager := tipmanager.New()
 
-	// create chains of transactions
-	for i := 0; i < count*threads; i++ {
-		for j := 0; j < txChains; j++ {
-			var tx *transaction.Transaction
+		txChains := 1
+		count := 10
+		threads := 2
+		countTotal := txChains * threads * count
 
-			// transferring from genesis
-			if i == 0 {
-				tx = transaction.New(
-					transaction.NewInputs(inputIDs[j]),
-					transaction.NewOutputs(
-						map[address.Address][]*balance.Balance{
-							address.Random(): {
-								balance.New(balance.ColorIOTA, 1),
-							},
-						}),
-				)
-			} else {
-				// create chains in UTXO dag
-				tx = transaction.New(
-					getTxOutputsAsInputs(transactions[i*txChains-txChains+j]),
-					transaction.NewOutputs(
-						map[address.Address][]*balance.Balance{
-							address.Random(): {
-								balance.New(balance.ColorIOTA, 1),
-							},
-						}),
-				)
+		// initialize tangle with genesis block
+		outputs := make(map[address.Address][]*balance.Balance)
+		for i := 0; i < txChains; i++ {
+			outputs[address.Random()] = []*balance.Balance{
+				balance.New(balance.ColorIOTA, 1),
 			}
-
-			transactions[i*txChains+j] = tx
 		}
-	}
-
-	// prepare value objects (simple chain)
-	for i := 0; i < countTotal; i++ {
-		parent1, parent2 := tipManager.Tips()
-		valueObject := payload.New(parent1, parent2, transactions[i])
-
-		tipManager.AddTip(valueObject)
-		valueObjects[i] = valueObject
-	}
-
-	//TODO:
-	//for i := 0; i < countTotal; i++ {
-	//	fmt.Println(i, "ValueObject", valueObjects[i].ID())
-	//	fmt.Println(i, "Transaction", valueObjects[i].Transaction().ID())
-	//}
-
-	// attach value objects in reverse order
-	var wg sync.WaitGroup
-	for thread := 0; thread < threads; thread++ {
-		wg.Add(1)
-		go func(threadNo int) {
-			defer wg.Done()
+		inputIDs := loadSnapshotFromOutputs(tangle, outputs)
+
+		transactions := make([]*transaction.Transaction, countTotal)
+		valueObjects := make([]*payload.Payload, countTotal)
+
+		// create chains of transactions
+		for i := 0; i < count*threads; i++ {
+			for j := 0; j < txChains; j++ {
+				var tx *transaction.Transaction
+
+				// transferring from genesis
+				if i == 0 {
+					tx = transaction.New(
+						transaction.NewInputs(inputIDs[j]),
+						transaction.NewOutputs(
+							map[address.Address][]*balance.Balance{
+								address.Random(): {
+									balance.New(balance.ColorIOTA, 1),
+								},
+							}),
+					)
+				} else {
+					// create chains in UTXO dag
+					tx = transaction.New(
+						getTxOutputsAsInputs(transactions[i*txChains-txChains+j]),
+						transaction.NewOutputs(
+							map[address.Address][]*balance.Balance{
+								address.Random(): {
+									balance.New(balance.ColorIOTA, 1),
+								},
+							}),
+					)
+				}
 
-			for i := countTotal - 1 - threadNo; i >= 0; i -= threads {
-				valueObject := valueObjects[i]
-				tangle.AttachPayloadSync(valueObject)
+				transactions[i*txChains+j] = tx
 			}
-		}(thread)
-	}
-	wg.Wait()
+		}
 
-	fmt.Println("finished attaching")
+		// prepare value objects (simple chain)
+		for i := 0; i < countTotal; i++ {
+			parent1, parent2 := tipManager.Tips()
+			valueObject := payload.New(parent1, parent2, transactions[i])
 
-	// verify correctness
-	for i := 0; i < countTotal; i++ {
-		// check if transaction metadata is found in database
-		require.Truef(t, tangle.TransactionMetadata(transactions[i].ID()).Consume(func(transactionMetadata *TransactionMetadata) {
-			assert.Truef(t, transactionMetadata.Solid(), "the transaction %s is not solid", transactions[i].ID().String())
-			assert.Equalf(t, branchmanager.MasterBranchID, transactionMetadata.BranchID(), "the transaction was booked into the wrong branch")
-		}), "transaction metadata %s not found in database", transactions[i].ID())
-
-		// check if value object metadata is found in database
-		require.Truef(t, tangle.PayloadMetadata(valueObjects[i].ID()).Consume(func(payloadMetadata *PayloadMetadata) {
-			assert.Truef(t, payloadMetadata.IsSolid(), "the payload %s is not solid", valueObjects[i].ID())
-			assert.Equalf(t, branchmanager.MasterBranchID, payloadMetadata.BranchID(), "the payload was booked into the wrong branch")
-		}), "value object metadata %s not found in database", valueObjects[i].ID())
+			tipManager.AddTip(valueObject)
+			valueObjects[i] = valueObject
+		}
 
-		// check if outputs are found in database
-		transactions[i].Outputs().ForEach(func(address address.Address, balances []*balance.Balance) bool {
-			cachedOutput := tangle.TransactionOutput(transaction.NewOutputID(address, transactions[i].ID()))
-			assert.Truef(t, cachedOutput.Consume(func(output *Output) {
-				// only the last outputs in chain should not be spent
-				if i+txChains >= countTotal {
-					assert.Equalf(t, 0, output.ConsumerCount(), "the output should not be spent")
-				} else {
-					assert.Equalf(t, 1, output.ConsumerCount(), "the output should be spent")
+		//TODO:
+		//for i := 0; i < countTotal; i++ {
+		//	fmt.Println(i, "ValueObject", valueObjects[i].ID())
+		//	fmt.Println(i, "Transaction", valueObjects[i].Transaction().ID())
+		//}
+
+		// attach value objects in reverse order
+		var wg sync.WaitGroup
+		for thread := 0; thread < threads; thread++ {
+			wg.Add(1)
+			go func(threadNo int) {
+				defer wg.Done()
+
+				for i := countTotal - 1 - threadNo; i >= 0; i -= threads {
+					valueObject := valueObjects[i]
+					tangle.AttachPayloadSync(valueObject)
 				}
-				assert.Equal(t, []*balance.Balance{balance.New(balance.ColorIOTA, 1)}, output.Balances())
-				assert.Equalf(t, branchmanager.MasterBranchID, output.BranchID(), "the output was booked into the wrong branch")
-				assert.Truef(t, output.Solid(), "the output is not solid")
-			}), "output not found in database for tx %s", transactions[i])
-			return true
-		})
+			}(thread)
+		}
+		wg.Wait()
+
+		// verify correctness
+		for i := 0; i < countTotal; i++ {
+			// check if transaction metadata is found in database
+			require.Truef(t, tangle.TransactionMetadata(transactions[i].ID()).Consume(func(transactionMetadata *TransactionMetadata) {
+				require.Truef(t, transactionMetadata.Solid(), "the transaction %s is not solid", transactions[i].ID().String())
+				require.Equalf(t, branchmanager.MasterBranchID, transactionMetadata.BranchID(), "the transaction was booked into the wrong branch")
+			}), "transaction metadata %s not found in database", transactions[i].ID())
+
+			// check if value object metadata is found in database
+			require.Truef(t, tangle.PayloadMetadata(valueObjects[i].ID()).Consume(func(payloadMetadata *PayloadMetadata) {
+				require.Truef(t, payloadMetadata.IsSolid(), "the payload %s is not solid", valueObjects[i].ID())
+				require.Equalf(t, branchmanager.MasterBranchID, payloadMetadata.BranchID(), "the payload was booked into the wrong branch")
+			}), "value object metadata %s not found in database", valueObjects[i].ID())
+
+			// check if outputs are found in database
+			transactions[i].Outputs().ForEach(func(address address.Address, balances []*balance.Balance) bool {
+				cachedOutput := tangle.TransactionOutput(transaction.NewOutputID(address, transactions[i].ID()))
+				assert.Truef(t, cachedOutput.Consume(func(output *Output) {
+					// only the last outputs in chain should not be spent
+					if i+txChains >= countTotal {
+						assert.Equalf(t, 0, output.ConsumerCount(), "the output should not be spent")
+					} else {
+						assert.Equalf(t, 1, output.ConsumerCount(), "the output should be spent")
+					}
+					assert.Equal(t, []*balance.Balance{balance.New(balance.ColorIOTA, 1)}, output.Balances())
+					assert.Equalf(t, branchmanager.MasterBranchID, output.BranchID(), "the output was booked into the wrong branch")
+					assert.Truef(t, output.Solid(), "the output is not solid")
+				}), "output not found in database for tx %s", transactions[i])
+				return true
+			})
+		}
 	}
 }
 

dapps/valuetransfers/packages/tangle/tangle_test.go

@@ -1248,8 +1248,8 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata.SetBranchID(branchmanager.MasterBranchID)
 
 		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
-		assert.True(t, solid)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
+		assert.False(t, solid)
 		assert.NoError(t, err)
 	}
 
@@ -1259,7 +1259,7 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata := NewPayloadMetadata(valueObject.ID())
 
 		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
 		assert.True(t, solid)
 		assert.NoError(t, err)
 	}
@@ -1275,7 +1275,7 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata := NewPayloadMetadata(valueObject.ID())
 
 		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
 		assert.True(t, solid)
 		assert.NoError(t, err)
 	}
@@ -1290,22 +1290,7 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata := NewPayloadMetadata(valueObject.ID())
 
 		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
-		assert.False(t, solid)
-		assert.NoError(t, err)
-	}
-
-	// check with non-solid parents but branch set -> should not be solid
-	{
-		setParent := func(payloadMetadata *PayloadMetadata) {
-			payloadMetadata.SetBranchID(branchmanager.MasterBranchID)
-		}
-
-		valueObject := payload.New(storeParentPayloadWithMetadataFunc(t, tangle, setParent), storeParentPayloadWithMetadataFunc(t, tangle, setParent), createDummyTransaction())
-		metadata := NewPayloadMetadata(valueObject.ID())
-
-		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
 		assert.False(t, solid)
 		assert.NoError(t, err)
 	}
@@ -1330,7 +1315,7 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata := NewPayloadMetadata(valueObject.ID())
 
 		transactionBranches := []branchmanager.BranchID{branchmanager.MasterBranchID}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
 		assert.False(t, solid)
 		assert.Error(t, err)
 	}
@@ -1355,7 +1340,7 @@ func TestCheckPayloadSolidity(t *testing.T) {
 		metadata := NewPayloadMetadata(valueObject.ID())
 
 		transactionBranches := []branchmanager.BranchID{{2}}
-		solid, err := tangle.checkPayloadSolidity(valueObject, metadata, transactionBranches)
+		solid, err := tangle.payloadBecameNewlySolid(valueObject, metadata, transactionBranches)
 		assert.False(t, solid)
 		assert.Error(t, err)
 	}