From efa7343615cac31ee4059b67107fd6cff84a31b0 Mon Sep 17 00:00:00 2001 From: jonastheis <4181434+jonastheis@users.noreply.github.com> Date: Fri, 22 Mar 2024 19:46:11 +0800 Subject: [PATCH 1/3] Add children as a set to blocks.Block as otherwise we're copying and merging distinct slices for each children type over and over when propagating witness weight --- pkg/protocol/engine/blocks/block.go | 85 +++++++------------ .../scheduler/drr/scheduler.go | 4 +- .../thresholdblockgadget/gadget.go | 10 ++- 3 files changed, 39 insertions(+), 60 deletions(-) diff --git a/pkg/protocol/engine/blocks/block.go b/pkg/protocol/engine/blocks/block.go index 0d204e660..de425a5b3 100644 --- a/pkg/protocol/engine/blocks/block.go +++ b/pkg/protocol/engine/blocks/block.go @@ -6,7 +6,6 @@ import ( "github.com/iotaledger/hive.go/ds" "github.com/iotaledger/hive.go/ds/reactive" - "github.com/iotaledger/hive.go/ds/types" "github.com/iotaledger/hive.go/lo" "github.com/iotaledger/hive.go/runtime/syncutils" "github.com/iotaledger/hive.go/stringify" @@ -24,6 +23,7 @@ type Block struct { strongChildren []*Block weakChildren []*Block shallowLikeChildren []*Block + allChildren ds.Set[*Block] // Booker block booked reactive.Variable[bool] @@ -74,48 +74,43 @@ func (r *rootBlock) String() string { return builder.String() } -// NewBlock creates a new Block with the given options. -func NewBlock(modelBlock *model.Block) *Block { +func newEmptyBlock() *Block { return &Block{ witnesses: ds.NewSet[account.SeatIndex](), spenderIDs: ds.NewSet[iotago.TransactionID](), payloadSpenderIDs: ds.NewSet[iotago.TransactionID](), acceptanceRatifiers: ds.NewSet[account.SeatIndex](), confirmationRatifiers: ds.NewSet[account.SeatIndex](), - modelBlock: modelBlock, solid: reactive.NewVariable[bool](), invalid: reactive.NewVariable[bool](), booked: reactive.NewVariable[bool](), accepted: reactive.NewVariable[bool](), weightPropagated: reactive.NewVariable[bool](), notarized: reactive.NewEvent(), - workScore: modelBlock.WorkScore(), + allChildren: ds.NewSet[*Block](), } } -func NewRootBlock(blockID iotago.BlockID, commitmentID iotago.CommitmentID, issuingTime time.Time) *Block { - b := &Block{ - witnesses: ds.NewSet[account.SeatIndex](), - spenderIDs: ds.NewSet[iotago.TransactionID](), - payloadSpenderIDs: ds.NewSet[iotago.TransactionID](), - acceptanceRatifiers: ds.NewSet[account.SeatIndex](), - confirmationRatifiers: ds.NewSet[account.SeatIndex](), +// NewBlock creates a new Block with the given options. +func NewBlock(modelBlock *model.Block) *Block { + b := newEmptyBlock() + b.modelBlock = modelBlock + b.workScore = modelBlock.WorkScore() - rootBlock: &rootBlock{ - blockID: blockID, - commitmentID: commitmentID, - issuingTime: issuingTime, - }, - solid: reactive.NewVariable[bool](), - invalid: reactive.NewVariable[bool](), - booked: reactive.NewVariable[bool](), - preAccepted: true, - accepted: reactive.NewVariable[bool](), - weightPropagated: reactive.NewVariable[bool](), - notarized: reactive.NewEvent(), - scheduled: true, + return b +} + +func NewRootBlock(blockID iotago.BlockID, commitmentID iotago.CommitmentID, issuingTime time.Time) *Block { + b := newEmptyBlock() + b.rootBlock = &rootBlock{ + blockID: blockID, + commitmentID: commitmentID, + issuingTime: issuingTime, } + b.preAccepted = true + b.scheduled = true + // This should be true since we commit and evict on acceptance. b.solid.Set(true) b.booked.Set(true) @@ -127,21 +122,11 @@ func NewRootBlock(blockID iotago.BlockID, commitmentID iotago.CommitmentID, issu } func NewMissingBlock(blockID iotago.BlockID) *Block { - return &Block{ - missing: true, - missingBlockID: blockID, - witnesses: ds.NewSet[account.SeatIndex](), - spenderIDs: ds.NewSet[iotago.TransactionID](), - payloadSpenderIDs: ds.NewSet[iotago.TransactionID](), - acceptanceRatifiers: ds.NewSet[account.SeatIndex](), - confirmationRatifiers: ds.NewSet[account.SeatIndex](), - solid: reactive.NewVariable[bool](), - invalid: reactive.NewVariable[bool](), - booked: reactive.NewVariable[bool](), - accepted: reactive.NewVariable[bool](), - weightPropagated: reactive.NewVariable[bool](), - notarized: reactive.NewEvent(), - } + b := newEmptyBlock() + b.missing = true + b.missingBlockID = blockID + + return b } func (b *Block) ProtocolBlock() *iotago.Block { @@ -294,25 +279,11 @@ func (b *Block) SetInvalid() (wasUpdated bool) { } // Children returns the children of the Block. -func (b *Block) Children() (children []*Block) { +func (b *Block) Children() ds.Set[*Block] { b.mutex.RLock() defer b.mutex.RUnlock() - seenBlockIDs := make(map[iotago.BlockID]types.Empty) - for _, parentsByType := range [][]*Block{ - b.strongChildren, - b.weakChildren, - b.shallowLikeChildren, - } { - for _, childMetadata := range parentsByType { - if _, exists := seenBlockIDs[childMetadata.ID()]; !exists { - children = append(children, childMetadata) - seenBlockIDs[childMetadata.ID()] = types.Void - } - } - } - - return children + return b.allChildren.Clone() } func (b *Block) StrongChildren() []*Block { @@ -348,6 +319,8 @@ func (b *Block) AppendChild(child *Block, childType iotago.ParentsType) { case iotago.ShallowLikeParentType: b.shallowLikeChildren = append(b.shallowLikeChildren, child) } + + b.allChildren.Add(child) } // Update publishes the given Block data to the underlying Block and marks it as no longer missing. diff --git a/pkg/protocol/engine/congestioncontrol/scheduler/drr/scheduler.go b/pkg/protocol/engine/congestioncontrol/scheduler/drr/scheduler.go index d2abba0e3..bc37cdfb4 100644 --- a/pkg/protocol/engine/congestioncontrol/scheduler/drr/scheduler.go +++ b/pkg/protocol/engine/congestioncontrol/scheduler/drr/scheduler.go @@ -709,7 +709,7 @@ func (s *Scheduler) updateChildrenWithLocking(block *blocks.Block) { // updateChildrenWithoutLocking iterates over the direct children of the given blockID and // tries to mark them as ready. func (s *Scheduler) updateChildrenWithoutLocking(block *blocks.Block) { - for _, childBlock := range block.Children() { + block.Children().Range(func(childBlock *blocks.Block) { if _, childBlockExists := s.blockCache.Block(childBlock.ID()); childBlockExists && childBlock.IsEnqueued() { if _, isBasic := childBlock.BasicBlock(); isBasic { s.tryReady(childBlock) @@ -719,7 +719,7 @@ func (s *Scheduler) updateChildrenWithoutLocking(block *blocks.Block) { panic("invalid block type") } } - } + }) } func (s *Scheduler) maxDeficit() Deficit { diff --git a/pkg/protocol/engine/consensus/blockgadget/thresholdblockgadget/gadget.go b/pkg/protocol/engine/consensus/blockgadget/thresholdblockgadget/gadget.go index 5d497c9ce..b4da4f5ee 100644 --- a/pkg/protocol/engine/consensus/blockgadget/thresholdblockgadget/gadget.go +++ b/pkg/protocol/engine/consensus/blockgadget/thresholdblockgadget/gadget.go @@ -111,10 +111,16 @@ func (g *Gadget) isCommitteeValidationBlock(block *blocks.Block) (seat account.S } func anyChildInSet(block *blocks.Block, set ds.Set[iotago.BlockID]) bool { - for _, child := range block.Children() { + if err := block.Children().ForEach(func(child *blocks.Block) error { if set.Has(child.ID()) { - return true + return ierrors.New("stop iteration: child in set") } + + // We continue the iteration if the child is not in the set. + return nil + }); err != nil { + // There can only be the stop iteration error -> there's a child in the set. + return true } return false From 9c3de811504746829e2aedd573781a03345e5144 Mon Sep 17 00:00:00 2001 From: jonastheis <4181434+jonastheis@users.noreply.github.com> Date: Mon, 25 Mar 2024 15:17:41 +0800 Subject: [PATCH 2/3] Fix test issuing too fast before slots can be committed leading to a forced commitment --- pkg/tests/upgrade_signaling_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/tests/upgrade_signaling_test.go b/pkg/tests/upgrade_signaling_test.go index 05de3d917..faea67324 100644 --- a/pkg/tests/upgrade_signaling_test.go +++ b/pkg/tests/upgrade_signaling_test.go @@ -400,7 +400,8 @@ func Test_Upgrade_Signaling(t *testing.T) { // Check that issuing still produces the same commitments on the nodes that upgraded. The nodes that did not upgrade // should not be able to issue and process blocks with the new version. ts.IssueBlocksAtSlots("", []iotago.SlotIndex{64, 65}, 4, "63.3", ts.Nodes("nodeB", "nodeC"), false, false) - ts.IssueBlocksAtSlots("", []iotago.SlotIndex{66, 67, 68, 69, 70, 71}, 4, "65.3", ts.Nodes("nodeB", "nodeC"), true, true) + ts.Wait() + ts.IssueBlocksAtSlots("", []iotago.SlotIndex{65, 66, 67, 68, 69, 70, 71}, 4, "65.3", ts.Nodes("nodeB", "nodeC"), true, false) // Nodes that did not set up the new protocol parameters are not able to process blocks with the new version. ts.AssertNodeState(ts.Nodes("nodeA", "nodeD", "nodeF", "nodeG"), From 456948a5685165442d20e24515457228e5754bf3 Mon Sep 17 00:00:00 2001 From: jonastheis <4181434+jonastheis@users.noreply.github.com> Date: Mon, 25 Mar 2024 16:09:34 +0800 Subject: [PATCH 3/3] Change Children() return parameter to be a readable set to avoid copying over when accessing field --- pkg/protocol/engine/blocks/block.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/protocol/engine/blocks/block.go b/pkg/protocol/engine/blocks/block.go index de425a5b3..5271936a2 100644 --- a/pkg/protocol/engine/blocks/block.go +++ b/pkg/protocol/engine/blocks/block.go @@ -279,11 +279,11 @@ func (b *Block) SetInvalid() (wasUpdated bool) { } // Children returns the children of the Block. -func (b *Block) Children() ds.Set[*Block] { +func (b *Block) Children() ds.ReadableSet[*Block] { b.mutex.RLock() defer b.mutex.RUnlock() - return b.allChildren.Clone() + return b.allChildren } func (b *Block) StrongChildren() []*Block { @@ -642,6 +642,10 @@ func (b *Block) String() string { builder.AddField(stringify.NewStructField(fmt.Sprintf("shallowLikeChildren%d", index), child.ID().String())) } + for index, child := range b.allChildren.ToSlice() { + builder.AddField(stringify.NewStructField(fmt.Sprintf("allChildren%d", index), child.ID().String())) + } + if b.rootBlock != nil { builder.AddField(stringify.NewStructField("RootBlock", b.rootBlock.String())) }