test(core/txpool/legacypool): fix flaky test TestAllowedTxSize #31836#2209
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 |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
This PR aims to fix flakiness in TestAllowedTxSize by making the helper that generates “large data” legacy transactions produce a deterministic encoded transaction size (avoiding variability from RLP-encoding shorter-than-32-byte ECDSA r/s values).
Changes:
- Updates
pricedDataTransactionto retry signing untilrandsare both exactly 32 bytes. - Adds explanatory comments about why signature component sizes can change encoded tx size.
- Adds inline comments to the max-size calculation in
TestAllowedTxSize.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 10 attempts is statistically sufficient since leading zeros in ECDSA signatures are rare and randomly distributed. | ||
| var retryTimes = 10 | ||
| for i := 0; i < retryTimes; i++ { | ||
| data := make([]byte, dataBytes) |
| crand.Read(data) | ||
|
|
||
| tx, _ = types.SignTx(types.NewTransaction(nonce, common.Address{}, big.NewInt(0), gaslimit, gasprice, data), types.HomesteadSigner{}, key) | ||
| _, r, s := tx.RawSignatureValues() | ||
| if len(r.Bytes()) == 32 && len(s.Bytes()) == 32 { |
| maxTxLengthWithoutData := txWithLargeData.Size() - largeDataLength // 103 bytes | ||
| maxTxDataLength := txMaxSize - maxTxLengthWithoutData // 131072 - 103 = 130969 bytes |
| break | ||
| } | ||
| } | ||
| return tx |
gzliudan
force-pushed
the
fix-flaky-test-TestAllowedTxSize
branch
from
d5e66de to
5a98af6
Compare
gzliudan
force-pushed
the
fix-flaky-test-TestAllowedTxSize
branch
from
5a98af6 to
cfd84f3
Compare
Proposed changes
Ref: ethereum#31836
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