Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Prioritize lower-compute-unit-limit transactions #28791

Closed

Conversation

apfitzge
Copy link
Contributor

Problem

#28751. Promote transactions with lower requested computer units.

Summary of Changes

Update Ord for ImmutableDeserializedPacket to prioritize lower compute-unit-limit packets after priority.

Fixes #

@apfitzge apfitzge requested a review from tao-stones November 11, 2022 17:47
@apfitzge
Copy link
Contributor Author

This doesn't strictly bump txs with requested CU, but those with lower CU limits. I think in most cases this will result in the same thing, however does not if a tx actually needs higher CUs than the default of 200k.

fn cmp(&self, other: &Self) -> Ordering {
self.priority().cmp(&other.priority())
match self.priority().cmp(&other.priority()) {
Ordering::Equal => other.compute_unit_limit().cmp(&self.compute_unit_limit()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not fully convinced if it's a good idea to prioritize txs requested less CUs. Maybe better to incentivize user to adapt requested CU with economic means. But if we really want to using prioritization as tool to encourage user to requested CU, then maybe only by a boolean flag of requested vs non-requested. (perhaps better move the discussion to issue itself)

Copy link
Contributor

Choose a reason for hiding this comment

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

Does reversing the comparison result in better block packing?

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Dec 29, 2022
@apfitzge
Copy link
Contributor Author

apfitzge commented Jan 2, 2023

See comments above, not the direction we want to go.

@apfitzge apfitzge closed this Jan 2, 2023
@apfitzge apfitzge deleted the feature/promote_tx_requested_cu branch January 2, 2023 18:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants