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.

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

Transaction aggregation in bucket_transactions is not weight aware #2456

Closed
antiochp opened this issue Jan 23, 2019 · 1 comment · Fixed by #2487
Closed

Transaction aggregation in bucket_transactions is not weight aware #2456

antiochp opened this issue Jan 23, 2019 · 1 comment · Fixed by #2487
Assignees
Labels
Milestone

Comments

@antiochp
Copy link
Member

When preparing mineable transactions via the txpool we add transactions to buckets and then aggregate each bucket. The buckets allow us to group dependent transactions (0-conf txs) together to maximize aggregation in the candidate block.

But - We do not take max block weight into account when building these buckets.
If we take a transaction at the limit of max block weight and then bucket it along with a 0-conf transaction that spends one of its outputs then we end up producing a transaction that exceed the max block weight.

Proposal:

Every time we add a transaction to an existing bucket (it spends an output that exists in a bucket) we need to construct the aggregate tx and determine its weight. If this exceeds the limit then we cannot proceed with the bucketing, so we should start a new bucket.
This will ensure all buckets are below the max block weight.

Then in the final step (as we do today) we then combine multiple buckets into the final set of aggregate transactions, ensuring we remain below the max block weight.

@antiochp antiochp added the bug label Jan 23, 2019
@antiochp antiochp added this to the 1.0.1 milestone Jan 23, 2019
@antiochp antiochp self-assigned this Jan 23, 2019
@ignopeverell
Copy link
Contributor

Sounds like a good improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants