Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
karimodm committed Oct 4, 2023
1 parent fac1e85 commit b6cf776
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 44 deletions.
4 changes: 2 additions & 2 deletions pkg/protocol/engine/booker/inmemorybooker/booker.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (b *Booker) Queue(block *blocks.Block) error {
signedTransactionMetadata.OnSignaturesValid(func() {
transactionMetadata := signedTransactionMetadata.TransactionMetadata()

if orphanedSlot, isOrphaned := transactionMetadata.IsOrphaned(); isOrphaned && orphanedSlot <= block.SlotCommitmentID().Slot() {
if orphanedSlot, isOrphaned := transactionMetadata.GetOrphanedSlot(); isOrphaned && orphanedSlot <= block.SlotCommitmentID().Slot() {
block.SetInvalid()

return
Expand Down Expand Up @@ -158,7 +158,7 @@ func (b *Booker) book(block *blocks.Block) error {
return ierrors.Errorf("failed to load transaction %s for block %s", conflictID.String(), block.ID())
}

if orphanedSlot, orphaned := txMetadata.IsOrphaned(); orphaned && orphanedSlot <= block.SlotCommitmentID().Slot() {
if orphanedSlot, orphaned := txMetadata.GetOrphanedSlot(); orphaned && orphanedSlot <= block.SlotCommitmentID().Slot() {
// Merge-to-master orphaned conflicts.
conflictsToInherit.Delete(conflictID)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/protocol/engine/ledger/ledger/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (l *Ledger) CommitSlot(slot iotago.SlotIndex) (stateRoot iotago.Identifier,

// Mark each transaction as committed so the mempool can evict it
stateDiff.ExecutedTransactions().ForEach(func(_ iotago.TransactionID, tx mempool.TransactionMetadata) bool {
tx.Commit(slot)
tx.Commit()
return true
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/protocol/engine/mempool/tests/testframework.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (t *TestFramework) CommitSlot(slot iotago.SlotIndex) {
})

stateDiff.ExecutedTransactions().ForEach(func(_ iotago.TransactionID, transaction mempool.TransactionMetadata) bool {
transaction.Commit(slot)
transaction.Commit()

return true
})
Expand Down
20 changes: 10 additions & 10 deletions pkg/protocol/engine/mempool/tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ func TestSetInclusionSlot(t *testing.T, tf *TestFramework) {
tf.CommitSlot(3)
tf.RequireTransactionsEvicted(transactionDeletionState)

require.False(t, lo.Return2(tx1Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx2Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx3Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx1Metadata.GetOrphanedSlot()))
require.False(t, lo.Return2(tx2Metadata.GetOrphanedSlot()))
require.False(t, lo.Return2(tx3Metadata.GetOrphanedSlot()))

tf.RequireAttachmentsEvicted(lo.MergeMaps(attachmentDeletionState, map[string]bool{"block1": true, "block2": true, "block3": false}))
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestSetTransactionOrphanage(t *testing.T, tf *TestFramework) {
// We only evict after MCA
tf.RequireTransactionsEvicted(map[string]bool{"tx1": false, "tx2": false, "tx3": false})

require.True(t, lo.Return2(tx1Metadata.IsOrphaned()))
require.True(t, lo.Return2(tx1Metadata.GetOrphanedSlot()))
require.True(t, tx2Metadata.IsPending())
require.True(t, tx3Metadata.IsPending())

Expand Down Expand Up @@ -312,19 +312,19 @@ func TestSetTxOrphanageMultipleAttachments(t *testing.T, tf *TestFramework) {
require.False(t, tx3Metadata.IsAccepted())

tf.Instance.Evict(1)
require.False(t, lo.Return2(tx1Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx2Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx3Metadata.IsOrphaned()))
require.False(t, lo.Return2(tx1Metadata.GetOrphanedSlot()))
require.False(t, lo.Return2(tx2Metadata.GetOrphanedSlot()))
require.False(t, lo.Return2(tx3Metadata.GetOrphanedSlot()))

require.True(t, lo.Return2(tf.ConflictDAG.ConflictSets(tf.TransactionID("tx1"))))
require.True(t, lo.Return2(tf.ConflictDAG.ConflictSets(tf.TransactionID("tx2"))))
require.True(t, lo.Return2(tf.ConflictDAG.ConflictSets(tf.TransactionID("tx3"))))

tf.Instance.Evict(2)

require.True(t, lo.Return2(tx1Metadata.IsOrphaned()))
require.True(t, lo.Return2(tx2Metadata.IsOrphaned()))
require.True(t, lo.Return2(tx3Metadata.IsOrphaned()))
require.True(t, lo.Return2(tx1Metadata.GetOrphanedSlot()))
require.True(t, lo.Return2(tx2Metadata.GetOrphanedSlot()))
require.True(t, lo.Return2(tx3Metadata.GetOrphanedSlot()))

// All conflicts still exist, as they are kept around until MCA
require.True(t, lo.Return2(tf.ConflictDAG.ConflictSets(tf.TransactionID("tx1"))))
Expand Down
6 changes: 3 additions & 3 deletions pkg/protocol/engine/mempool/transaction_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type TransactionMetadata interface {

ConflictIDs() reactive.Set[iotago.TransactionID]

Commit(slot iotago.SlotIndex)
Commit()

IsSolid() bool

Expand Down Expand Up @@ -57,15 +57,15 @@ type inclusionFlags interface {

OnAccepted(callback func())

IsCommitted() (slot iotago.SlotIndex, isCommitted bool)
GetCommittedSlot() (slot iotago.SlotIndex, isCommitted bool)

OnCommitted(callback func(slot iotago.SlotIndex))

IsRejected() bool

OnRejected(callback func())

IsOrphaned() (slot iotago.SlotIndex, isOrphaned bool)
GetOrphanedSlot() (slot iotago.SlotIndex, isOrphaned bool)

OnOrphaned(callback func(slot iotago.SlotIndex))
}
28 changes: 14 additions & 14 deletions pkg/protocol/engine/mempool/v1/inclusion_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,24 @@ type inclusionFlags struct {
// accepted gets triggered when the entity gets marked as accepted.
accepted reactive.Variable[bool]

// committed gets set when the entity gets marked as committed.
committed reactive.Variable[iotago.SlotIndex]
// committedOnSlot gets set to the slot in which the entity gets marked as committed.
committedOnSlot reactive.Variable[iotago.SlotIndex]

// rejected gets triggered when the entity gets marked as rejected.
rejected *promise.Event

// orphaned gets set when the entity gets marked as orphaned.
orphaned reactive.Variable[iotago.SlotIndex]
// orphanedOnSlot gets set to the slot in which the entity gets marked as orphaned.
orphanedOnSlot reactive.Variable[iotago.SlotIndex]
}

// newInclusionFlags creates a new inclusionFlags instance.
func newInclusionFlags() *inclusionFlags {
return &inclusionFlags{
accepted: reactive.NewVariable[bool](),
committed: reactive.NewVariable[iotago.SlotIndex](),
rejected: promise.NewEvent(),
accepted: reactive.NewVariable[bool](),
committedOnSlot: reactive.NewVariable[iotago.SlotIndex](),
rejected: promise.NewEvent(),
// Make sure the oldest orphaned index doesn't get overridden by newer TX spending the orphaned conflict further.
orphaned: reactive.NewVariable[iotago.SlotIndex](func(currentValue, newValue iotago.SlotIndex) iotago.SlotIndex {
orphanedOnSlot: reactive.NewVariable[iotago.SlotIndex](func(currentValue, newValue iotago.SlotIndex) iotago.SlotIndex {
if currentValue != 0 {
return currentValue
}
Expand Down Expand Up @@ -76,25 +76,25 @@ func (s *inclusionFlags) OnRejected(callback func()) {
}

// IsCommitted returns true if the entity was committed.
func (s *inclusionFlags) IsCommitted() (slot iotago.SlotIndex, isCommitted bool) {
return s.committed.Get(), s.committed.Get() != 0
func (s *inclusionFlags) GetCommittedSlot() (slot iotago.SlotIndex, isCommitted bool) {
return s.committedOnSlot.Get(), s.committedOnSlot.Get() != 0
}

// OnCommitted registers a callback that gets triggered when the entity gets committed.
func (s *inclusionFlags) OnCommitted(callback func(slot iotago.SlotIndex)) {
s.committed.OnUpdate(func(_, newValue iotago.SlotIndex) {
s.committedOnSlot.OnUpdate(func(_, newValue iotago.SlotIndex) {
callback(newValue)
})
}

// IsOrphaned returns true if the entity was orphaned.
func (s *inclusionFlags) IsOrphaned() (slot iotago.SlotIndex, isOrphaned bool) {
return s.orphaned.Get(), s.orphaned.Get() != 0
func (s *inclusionFlags) GetOrphanedSlot() (slot iotago.SlotIndex, isOrphaned bool) {
return s.orphanedOnSlot.Get(), s.orphanedOnSlot.Get() != 0
}

// OnOrphaned registers a callback that gets triggered when the entity gets orphaned.
func (s *inclusionFlags) OnOrphaned(callback func(slot iotago.SlotIndex)) {
s.orphaned.OnUpdate(func(_, newValue iotago.SlotIndex) {
s.orphanedOnSlot.OnUpdate(func(_, newValue iotago.SlotIndex) {
callback(newValue)
})
}
6 changes: 2 additions & 4 deletions pkg/protocol/engine/mempool/v1/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,7 @@ func (m *MemPool[VoteRank]) Evict(slot iotago.SlotIndex) {

if delayedOutputs, exists := m.delayedOutputStateEviction.Get(delayedEvictionSlot); exists {
delayedOutputs.ForEach(func(stateID iotago.Identifier, state *StateMetadata) bool {
if !m.cachedStateRequests.Delete(stateID, state.HasNoSpenders) && m.cachedStateRequests.Has(stateID) {
state.onAllSpendersRemoved(func() { m.cachedStateRequests.Delete(stateID, state.HasNoSpenders) })
}
m.cachedStateRequests.Delete(stateID, state.HasNoSpenders)

return true
})
Expand Down Expand Up @@ -333,7 +331,7 @@ func (m *MemPool[VoteRank]) forkTransaction(transactionMetadata *TransactionMeta

if err := m.conflictDAG.UpdateConflictingResources(transactionMetadata.ID(), resourceIDs); err != nil {
// this is a hack, as with a reactive.Variable we cannot set it to 0 and still check if it was orphaned.
transactionMetadata.orphaned.Set(1)
transactionMetadata.orphanedOnSlot.Set(1)

m.errorHandler(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/protocol/engine/mempool/v1/state_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func (s *StateMetadata) setup(optSource ...*TransactionMetadata) *StateMetadata
source.OnPending(func() { s.accepted.Set(false) })
source.OnAccepted(func() { s.accepted.Set(true) })
source.OnRejected(func() { s.rejected.Trigger() })
source.OnCommitted(lo.Void(s.committed.Set))
source.OnOrphaned(lo.Void(s.orphaned.Set))
source.OnCommitted(lo.Void(s.committedOnSlot.Set))
source.OnOrphaned(lo.Void(s.orphanedOnSlot.Set))

return s
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/protocol/engine/mempool/v1/transaction_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ func (t *TransactionMetadata) markInputSolid() (allInputsSolid bool) {
return false
}

func (t *TransactionMetadata) Commit(slot iotago.SlotIndex) {
t.committed.Set(slot)
func (t *TransactionMetadata) Commit() {
t.committedOnSlot.Set(t.earliestIncludedValidAttachment.Get().Slot())
}

func (t *TransactionMetadata) IsConflicting() bool {
Expand Down Expand Up @@ -251,7 +251,7 @@ func (t *TransactionMetadata) setupInput(input *StateMetadata) {

input.OnRejected(func() { t.rejected.Trigger() })
input.OnOrphaned(func(slot iotago.SlotIndex) {
t.orphaned.Set(slot)
t.orphanedOnSlot.Set(slot)
})
input.OnAccepted(func() {
if atomic.AddUint64(&t.unacceptedInputsCount, ^uint64(0)) == 0 {
Expand Down Expand Up @@ -279,7 +279,7 @@ func (t *TransactionMetadata) setupInput(input *StateMetadata) {
input.OnSpendCommitted(func(spender mempool.TransactionMetadata) {
if spender != t {
spender.OnCommitted(func(slot iotago.SlotIndex) {
t.orphaned.Set(slot)
t.orphanedOnSlot.Set(slot)
})
}
})
Expand All @@ -295,8 +295,8 @@ func (t *TransactionMetadata) setup() (self *TransactionMetadata) {
})

t.allValidAttachmentsEvicted.OnUpdate(func(_, slot iotago.SlotIndex) {
if !lo.Return2(t.IsCommitted()) {
t.orphaned.Set(slot)
if !lo.Return2(t.GetCommittedSlot()) {
t.orphanedOnSlot.Set(slot)
}
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/testsuite/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (t *TestSuite) AssertTransactionsInCachePending(expectedTransactions []*iot
}

func (t *TestSuite) AssertTransactionsInCacheOrphaned(expectedTransactions []*iotago.Transaction, expectedFlag bool, nodes ...*mock.Node) {
t.assertTransactionsInCacheWithFunc(expectedTransactions, expectedFlag, func(tm mempool.TransactionMetadata) bool { return lo.Return2(tm.IsOrphaned()) }, nodes...)
t.assertTransactionsInCacheWithFunc(expectedTransactions, expectedFlag, func(tm mempool.TransactionMetadata) bool { return lo.Return2(tm.GetOrphanedSlot()) }, nodes...)
}

func (t *TestSuite) AssertTransactionInCacheConflicts(transactionConflicts map[*iotago.Transaction][]string, nodes ...*mock.Node) {
Expand Down

0 comments on commit b6cf776

Please sign in to comment.