Merge pull request #915 from iotaledger/fix/memleak-mempool

Fix: Memleak (MemPool + SpendDAG)

pkg/protocol/engine/blocks/block.go

@@ -23,9 +23,6 @@ type Block struct {
 	// PayloadDependenciesAvailable is triggered when the dependencies of the block's payload are available.
 	PayloadDependenciesAvailable reactive.Event
 
-	// SignedTransactionMetadata contains the signed transaction metadata of the block.
-	SignedTransactionMetadata reactive.Variable[mempool.SignedTransactionMetadata]
-
 	// BlockDAG block
 	missing             bool
 	missingBlockID      iotago.BlockID
@@ -89,7 +86,6 @@ func newEmptyBlock() *Block {
 	return &Block{
 		ParentsBooked:                reactive.NewEvent(),
 		PayloadDependenciesAvailable: reactive.NewEvent(),
-		SignedTransactionMetadata:    reactive.NewVariable[mempool.SignedTransactionMetadata](),
 
 		witnesses:             ds.NewSet[account.SeatIndex](),
 		spenderIDs:            ds.NewSet[iotago.TransactionID](),

pkg/protocol/engine/booker/inmemorybooker/booker.go

@@ -97,13 +97,12 @@ func (b *Booker) Init(ledger ledger.Ledger, loadBlockFromStorage func(iotago.Blo
 func (b *Booker) Queue(block *blocks.Block) error {
 	signedTransactionMetadata, containsTransaction := b.ledger.AttachTransaction(block)
 	if !containsTransaction {
-		b.setupBlock(block)
+		b.setupBlock(block, nil)
 
 		return nil
 	} else if signedTransactionMetadata == nil {
 		return ierrors.Errorf("transaction in block %s was not attached", block.ID())
 	}
-	block.SignedTransactionMetadata.Set(signedTransactionMetadata)
 
 	// Based on the assumption that we always fork and the UTXO and Tangle past cones are always fully known.
 	signedTransactionMetadata.OnSignaturesValid(func() {
@@ -117,16 +116,16 @@ func (b *Booker) Queue(block *blocks.Block) error {
 
 		transactionMetadata.OnBooked(func() {
 			block.SetPayloadSpenderIDs(transactionMetadata.SpenderIDs())
-			b.setupBlock(block)
+			b.setupBlock(block, signedTransactionMetadata)
 		})
 
 		transactionMetadata.OnInvalid(func(_ error) {
-			b.setupBlock(block)
+			b.setupBlock(block, signedTransactionMetadata)
 		})
 	})
 
 	signedTransactionMetadata.OnSignaturesInvalid(func(_ error) {
-		b.setupBlock(block)
+		b.setupBlock(block, signedTransactionMetadata)
 	})
 
 	return nil
@@ -135,9 +134,10 @@ func (b *Booker) Queue(block *blocks.Block) error {
 // Reset resets the component to a clean state as if it was created at the last commitment.
 func (b *Booker) Reset() { /* nothing to reset but comply with interface */ }
 
-func (b *Booker) setupBlock(block *blocks.Block) {
+func (b *Booker) setupBlock(block *blocks.Block, signedTransactionMetadata mempool.SignedTransactionMetadata) {
 	var payloadDependencies, directlyReferencedPayloadDependencies ds.Set[mempool.StateMetadata]
-	if signedTransactionMetadata := block.SignedTransactionMetadata.Get(); signedTransactionMetadata != nil && signedTransactionMetadata.SignaturesInvalid() == nil && !signedTransactionMetadata.TransactionMetadata().IsInvalid() {
+
+	if signedTransactionMetadata != nil && signedTransactionMetadata.SignaturesInvalid() == nil && !signedTransactionMetadata.TransactionMetadata().IsInvalid() {
 		payloadDependencies = signedTransactionMetadata.TransactionMetadata().Inputs()
 		directlyReferencedPayloadDependencies = ds.NewSet[mempool.StateMetadata]()
 	}
@@ -155,8 +155,10 @@ func (b *Booker) setupBlock(block *blocks.Block) {
 
 		parentBlock.Booked().OnUpdateOnce(func(_ bool, _ bool) {
 			if directlyReferencedPayloadDependencies != nil {
-				if parentTransactionMetadata := parentBlock.SignedTransactionMetadata.Get(); parentTransactionMetadata != nil {
-					directlyReferencedPayloadDependencies.AddAll(parentTransactionMetadata.TransactionMetadata().Outputs())
+				if signedTx, hasTx := parentBlock.SignedTransaction(); hasTx {
+					if parentTransactionMetadata, exists := b.ledger.TransactionMetadata(signedTx.Transaction.MustID()); exists {
+						directlyReferencedPayloadDependencies.AddAll(parentTransactionMetadata.Outputs())
+					}
 				}
 			}
 

pkg/protocol/engine/mempool/spenddag/spenddagv1/spenddag.go

@@ -412,13 +412,12 @@ func (c *SpendDAG[SpenderID, ResourceID, VoteRank]) evictSpender(spenderID Spend
 	// remove the spenders from the spenddag dictionary
 	for _, evictedSpenderID := range evictedSpenderIDs {
 		c.spendersByID.Delete(evictedSpenderID)
-	}
-
-	// unhook the spend events and remove the unhook method from the storage
-	unhookFunc, unhookExists := c.spendUnhooks.Get(spenderID)
-	if unhookExists {
-		unhookFunc()
-		c.spendUnhooks.Delete(spenderID)
+		// unhook the spend events and remove the unhook method from the storage
+		unhookFunc, unhookExists := c.spendUnhooks.Get(spenderID)
+		if unhookExists {
+			unhookFunc()
+			c.spendUnhooks.Delete(spenderID)
+		}
 	}
 
 	return evictedSpenderIDs

pkg/protocol/engine/mempool/state_metadata.go

@@ -6,8 +6,6 @@ import (
 )
 
 type StateMetadata interface {
-	CreatingTransaction() TransactionMetadata
-
 	State() State
 
 	SpenderIDs() reactive.Set[iotago.TransactionID]

pkg/protocol/engine/mempool/v1/state_metadata.go

@@ -12,8 +12,7 @@ import (
 )
 
 type StateMetadata struct {
-	state  mempool.State
-	source *TransactionMetadata
+	state mempool.State
 
 	// lifecycle
 	spenderCount       uint64
@@ -31,8 +30,7 @@ type StateMetadata struct {
 
 func NewStateMetadata(state mempool.State, optSource ...*TransactionMetadata) *StateMetadata {
 	return (&StateMetadata{
-		state:  state,
-		source: lo.First(optSource),
+		state: state,
 
 		spent:              promise.NewEvent(),
 		doubleSpent:        promise.NewEvent(),
@@ -73,14 +71,6 @@ func (s *StateMetadata) setup(optSource ...*TransactionMetadata) *StateMetadata
 	return s
 }
 
-func (s *StateMetadata) CreatingTransaction() mempool.TransactionMetadata {
-	if s.source == nil {
-		return nil
-	}
-
-	return s.source
-}
-
 func (s *StateMetadata) State() mempool.State {
 	return s.state
 }