refactor(core/txpool): don't inject lazy resolved transactions into the container #28917#2133
refactor(core/txpool): don't inject lazy resolved transactions into the container #28917#2133gzliudan wants to merge 1 commit intoXinFinOrg:dev-upgradefrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors LazyTransaction.Resolve() in the txpool subpool API to avoid persisting (and thus retaining) fully-resolved transactions when they are fetched lazily, addressing the “don’t inject lazy resolved transactions into the container” goal from ethereum#28917.
Changes:
- Updates
LazyTransaction.Resolve()to return an already-presentltx.Tximmediately, otherwise fetch vialtx.Pool.Get(ltx.Hash)without caching it back onto theLazyTransaction. - Adds an explanatory comment describing the motivation (avoiding memory bloat from retaining large transactions).
Comments suppressed due to low confidence (1)
core/txpool/subpool.go:55
- The added comment says Resolve will not cache the retrieved tx “if the original pool has not cached it”, but the implementation never stores the result of Pool.Get into ltx.Tx. Please reword the comment to match the actual behavior (i.e., Resolve does not update ltx.Tx), or if conditional caching was intended, implement that explicitly.
// Note, the method will *not* cache the retrieved transaction if the original
// pool has not cached it. The idea being, that if the tx was too big to insert
// originally, silently saving it will cause more trouble down the line (and
// indeed seems to have caused a memory bloat in the original implementation
// which did just that).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…he container ethereum#28917
Proposed changes
Ref: ethereum#28917
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that