This repository has been archived by the owner on Sep 23, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Txpool 4844 upgrades Part 2 #1125
Txpool 4844 upgrades Part 2 #1125
Changes from 20 commits
101b29d
109504c
95c1313
969e427
54a5d23
2b62b55
0101e2c
73b24db
a310392
60e80d1
a4f8130
8c3610a
dfac277
9c91ded
38fc5b4
056b5a8
e5deeda
55ae513
4b4492d
0b073c2
fb3b618
c01aa0d
485e52c
778be3a
c71bfc3
a726ee9
c54dc75
5bf4f1d
87c22fb
ac899d3
e606c62
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“Read then write” on atomic - it’s race-condition. Please use CompareAndSwap operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the only place of writing that variable tho, but there is a bit of unnecessary code here. Removing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uint256 has method Less
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to native uint64 comparison
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to using uint256's
CmpUint64()
andLtUint64()
methodsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid “NewInt” inside loop - for optimization. Because we are also inside global mutex of txpool - and better unlock it as fast as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to native uint64 conversion and then comparison
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it in “wei”? Can it have uint64 overflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually comes from the submitted transaction, theoretically it could have an overflow. So could the block's baseFee. I will make the worst assumptions here and rather use uint256 everywhere.
Practically, it may not matter, but for expanding support to other chains later, with much lower native token values, it could be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to using uint256's
CmpUint64()
andLtUint64()
methodsLarge diffs are not rendered by default.