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

Remove sender stakes from banking_stage buffer prioritization #26512

Merged

Conversation

tao-stones
Copy link
Contributor

Problem

It seems unnecessary to prioritize packets by sender stake on top of transaction's compute-unit price, due to:

  1. Quic stake weight part already giving priority to highly staked senders;
  2. When compute-unit price is widely adapted, due to micro-lamport as its unit, it is very unlikely enough transactions have same priority; makes adding sender stake into MinMaxHeap ordering unnecessary;

therefore it makes sense to remove sender stake from MinMaxHeap ordering to save some computing and simplify code a bit.

Summary of Changes

Remove sender-stake from MinMaxHeap ordering.

Fixes #

@tao-stones tao-stones requested a review from carllin July 8, 2022 18:28
@nikhayes
Copy link

nikhayes commented Jul 8, 2022

Agreed. You can see evidence that the few senders using fee priority are already using highly nuanced fees. There is incentive not to have the same priority as others and minutely tweak things... thus there will likely be a great variety of fee priorities observed.

Pays 0.000006001 Sol
https://explorer.solana.com/tx/3E5sjRJYV9E6vccWdjzRkxuYrZoTQ26NjcJhMkY8G3UWsEXY6eTad8sjebbRSUQ59TFc6vZHYrBAE4QT8VvBmQjh

Pays 0.000005002
https://explorer.solana.com/tx/2M4F8nKdmV8SqsWXKtEPWXhJxfbtACvMEbeGA45mLvWJ3fWTkPafZQQTRkyq2AGZLBS5uNj5bifTk3r3YNf3L17T

In summary, I don't think sorting by stake weight will accomplish much other than causing worry about stake centralization issues (i.e. the powerful get richer -- which I've seen people wonder already) and adding a fud angle.

@tao-stones tao-stones requested a review from sakridge July 8, 2022 19:16
carllin
carllin previously approved these changes Jul 11, 2022
core/src/unprocessed_packet_batches.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed carllin’s stale review July 11, 2022 14:48

Pull request has been modified.

@tao-stones tao-stones merged commit a3b0943 into solana-labs:master Jul 11, 2022
@tao-stones tao-stones deleted the remove_stake_from_prioritizing branch July 11, 2022 17:47
mergify bot pushed a commit that referenced this pull request Jul 11, 2022
* remove sender stakes from banking_stage buffer prioritization

(cherry picked from commit a3b0943)
mergify bot added a commit that referenced this pull request Jul 11, 2022
#26512) (#26562)

Remove sender stakes from banking_stage buffer prioritization (#26512)

* remove sender stakes from banking_stage buffer prioritization

(cherry picked from commit a3b0943)

Co-authored-by: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com>
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Apr 6, 2023
solana-labs#26512
removed sender_stake from banking-stage buffer prioritization and it is
not used any more.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Apr 6, 2023
solana-labs#26512
removed sender_stake from banking-stage buffer prioritization and it is
not used any more.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Apr 6, 2023
Packet Meta.sender_stake is unused since
solana-labs#26512
removed sender_stake from banking-stage buffer prioritization.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request Apr 6, 2023
Packet Meta.sender_stake is unused since
solana-labs#26512
removed sender_stake from banking-stage buffer prioritization.
behzadnouri added a commit that referenced this pull request Apr 6, 2023
…31077)

Packet Meta.sender_stake is unused since
#26512
removed sender_stake from banking-stage buffer prioritization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants