Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core, trie: prealloc capacity for maps #30437

Merged
merged 3 commits into from
Sep 16, 2024
+7 −7
Merged

core, trie: prealloc capacity for maps #30437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0476172
avoid posible zero index panic
mask-pp Sep 13, 2024
114fc6b
temporary code
mask-pp Sep 14, 2024
d77bc61
merge master branch and fix conflict
mask-pp Sep 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions core/stateless/witness.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ func (w *Witness) AddState(nodes map[string]struct{}) {
w.lock.Lock()
defer w.lock.Unlock()

for node := range nodes {
w.State[node] = struct{}{}
}
maps.Copy(w.State, nodes)
}

// Copy deep-copies the witness object. Witness.Block isn't deep-copied as it
Expand Down
6 changes: 4 additions & 2 deletions core/txpool/blobpool/blobpool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ func (p *BlobPool) reorg(oldHead, newHead *types.Header) (map[common.Address][]*
}
// Generate the set of transactions per address to pull back into the pool,
// also updating the rest along the way
reinject := make(map[common.Address][]*types.Transaction)
reinject := make(map[common.Address][]*types.Transaction, len(transactors))
for addr := range transactors {
// Generate the set that was lost to reinject into the pool
lost := make([]*types.Transaction, 0, len(discarded[addr]))
Expand All @@ -949,7 +949,9 @@ func (p *BlobPool) reorg(oldHead, newHead *types.Header) (map[common.Address][]*
lost = append(lost, tx)
}
}
reinject[addr] = lost
if len(lost) > 0 {
reinject[addr] = lost
}
Comment on lines +952 to +954
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I failed to take note of this at the time, but now I am unsure if this was correct. Previously, reinject would contains an empty list for the address addr.

The blobpool Reset function would then call p.recheck on that addr

	if reinject, inclusions := p.reorg(oldHead, newHead); reinject != nil {
		var adds []*types.Transaction
		for addr, txs := range reinject {
			// Blindly push all the lost transactions back into the pool
			for _, tx := range txs {
				if err := p.reinject(addr, tx.Hash()); err == nil {
					adds = append(adds, tx.WithoutBlobTxSidecar())
				}
			}
			// Recheck the account's pooled transactions to drop included and
			// invalidated ones
			p.recheck(addr, inclusions)
		}
		if len(adds) > 0 {
			p.insertFeed.Send(core.NewTxsEvent{Txs: adds})
		}
	}

I think the new code is wrong: it may very well happen that we reorg out a transaction, but it is not eligible for re-injection, because the filtering fails:

			if p.Filter(tx) {
				lost = append(lost, tx)
			}
// Filter returns whether the given transaction can be consumed by the blob pool.
func (p *BlobPool) Filter(tx *types.Transaction) bool {
	return tx.Type() == types.BlobTxType
}

...
That is: we reorg-out a regular transaction, it is not eligible for re-injection (in the blob pool), and thus the blob pool becomes off: it's should have invoked `recheck` on the account. 


@karalabe PTAL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inclusions from reorg is always not null, and if txs is null it won't touch reinject func and in recheck function inclusions != nil && txs == nil check is true and then return. So I think this PR is right.


// Update the set that was already reincluded to track the blocks in limbo
for _, tx := range types.TxDifference(included[addr], discarded[addr]) {
Expand Down
2 changes: 1 addition & 1 deletion core/types/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func (s Transactions) EncodeIndex(i int, w *bytes.Buffer) {
func TxDifference(a, b Transactions) Transactions {
keep := make(Transactions, 0, len(a))

remove := make(map[common.Hash]struct{})
remove := make(map[common.Hash]struct{}, b.Len())
for _, tx := range b {
remove[tx.Hash()] = struct{}{}
}
Expand Down
2 changes: 1 addition & 1 deletion trie/trie.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ func (t *Trie) Witness() map[string]struct{} {
if len(t.tracer.accessList) == 0 {
return nil
}
witness := make(map[string]struct{})
witness := make(map[string]struct{}, len(t.tracer.accessList))
for _, node := range t.tracer.accessList {
witness[string(node)] = struct{}{}
}
Expand Down