-
Notifications
You must be signed in to change notification settings - Fork 253
Amend simd-0123 #300
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
base: main
Are you sure you want to change the base?
Amend simd-0123 #300
Conversation
a3dc7d2 to
1807d1e
Compare
1807d1e to
6c6cda1
Compare
|
@dimandotsol can you take a look now and see if the distribution section is easier to understand? |
Looks great! Thank you! |
|
I noticed that lamports will remain on the sysvar balance due to integer division. Maybe it’s worth explicitly mentioning this? In general, it’s fine, and how to handle it can be addressed later. But while reading, I had the following idea: And on each distribution block, increase |
|
The idea sounds good to me intuitively. However, there are a few more details that need to be fleshed out.
|
|
@dimandotsol Thanks for pointing this out, agreed on burning the excess, just amended!
@HaoranYi We recover this value from the epoch stakes cache. We already use the epoch stakes cache for recalculating partitioned inflation rewards after restart. In the future we can store the last few epochs of delegator rewards in the vote account state so that we don't need to rely on the epoch stakes cache.
Yes, have considered it. Basically it's just a few extra arithmetic operations to calculate the extra rewards.
@HaoranYi agreed. I don't think it's a good option and not necessary as I explained above. @dimandotsol please note that I didn't include this part in the amendment. |
@jstarry Wasn’t the validators array supposed to be stored in the sysvar data? Those are just the And that’s less than 100 KB for 2000 validators. |
| #### Delegator Rewards Completion | ||
|
|
||
| After distributing all partitioned delegator rewards, the epoch rewards sysvar balance | ||
| MUST be reset to its rent exemption balance and any surplus lamports are burned. |
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.
This approach will allow burning any amount of lamports that can be transferred to the sysvar address.
By simply additionally storing values in only two u64 fields (in fact, one is enough, because the second is just the sum of P over the array and computing it once isn’t a problem), I’m proposing to prevent burning of arbitrary amounts - only rounding burns would remain.
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.
You can't transfer to the sysvar in a transaction, it's reserved
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.
Ah, okay, I didn’t know that.
Nope, the calculated values do not to be stored onchain, this SIMD is designed so that those values can always be recalculated after restart. |
|
It seems to me that the complexity of introducing a new sysvar for block
|
|
I was confused by the comments and the actual SIMD. The SIMD didn't introduce a new sysvar. So my last comments should be like this.
t seems to me that the complexity of collecting rewards into epoch reward sysvar for block revenue payout in this spec may not be worth it for the following reasons. |
HaoranYi
left a comment
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.
Lgtm
| Note that unlike inflation rewards distribution, block revenue distribution will | ||
| not impact any internal epoch rewards sysvar state fields like `total_rewards` | ||
| or `distributed_rewards` since block revenue will instead be tracked via the | ||
| epoch rewards sysvar lamport balance. |
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.
This feels hacky and could lead to edge cases - how do we ensure that the sysvar account is rent-exempt, for example? Is it that bad to just add a field to the sysvar? Overloading the lamport balance feels like it could be confusing, is there a good reason for doing this?
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.
Since the reward lamports already exist onchain they need to live somewhere, unlike inflation rewards which are newly created for distribution. Adding another field might lead to more edge cases since we would need to track both the lamport balance and the field balance. The sysvar account already always exists so no need to worry about rent exempt balance here.
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.
The capatialization thing makes sense, thanks! It would be great to get a bit more clarification on how this interacts with the code for making sysvars always rent exempt.
When sysvars are saved, update_sysvar_account calls adjust_sysvar_balance_for_rent which ensures the balance is always rent-exempt. Are you proposing that the lamport balance of the epoch reward sysvar would be minimum_balance_for_rent_exemption + the reward lamports?
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.
Are you proposing that the lamport balance of the epoch reward sysvar would be minimum_balance_for_rent_exemption + the reward lamports?
Yes, that's correct. On all clusters the sysvar already exists with the rent exempt balance, so this SIMD doesn't detail anything about topping up the sysvar account balance.
Btw I talk about the sysvar balance a bit more a little lower in the SIMD
After distributing all partitioned delegator rewards, the epoch rewards sysvar balance MUST be reset to its rent exemption balance and any surplus lamports are burned.
|
@t-nelson @topointon-jump @buffalojoec @Benhawkins18 I think I spent too much energy thinking about the distribution mechanism and not enough about what we're actually sharing. As written, SIMD-0123 will allow validators to set a commission for ALL block revenue and the rest goes to delegators. This means that EVEN base fees will be shared with delegators. This feels wrong to me. We should only support sharing priority fees right? Base fees in theory pay for the base cost of tx compute? |
|
I lean towards keeping this simple: all block revenue (both base fees and priority fees) should flow through the same commission mechanism. Validators already have multiple potential revenue streams, and they can decide what to share (or not share) via their overall commission. Introducing a separate carve-out just for base fees adds complexity for both validators and delegators. Delegators would need to understand that “commission” applies differently depending on the source of the revenue, which feels like unnecessary nuance. If everything is bundled, the mental model is much cleaner: delegators know they’re sharing in all block rewards minus validator commission, and validators retain the flexibility to share other income streams however they see fit. So my preference is that SIMD-0123’s “block reward commission” applies uniformly across both base fees and prioritization fees. |
Conceptually I like the separation of base fee vs. priority fee from the revenue-sharing mechanism.
|
|
Isn't another motivation for SIMD-123 to allow operators to send fees to other keys besides the identity if they choose? It seems wrong to force them into sending it to the identity if they don't want to. |
|
do not discern priority fees. they need to die and we should make that as easy as possible |
Yes, that is something we are doing but it's technically part of #232. That SIMD does refer to the new
This is what I was thinking as well. But now I'm realizing this doesn't make sense. Each transaction is processed by all the validators and if base fees were truly meant to cover cluster operational costs, they would get divided evenly (regardless of stake) to all validators rather than just getting paid to the current leader. So now I'm thinking @Benhawkins18 and @t-nelson have the right idea that we shouldn't differentiate between base and priority fees here. It's all block revenue. We can always differentiate later if we really need to but I think it makes most sense to combine them for block revenue sharing with delegators. The whole point of a commission on block revenue (and collection to a specific collector address for bespoke solutions) is for validators to adjust how much revenue makes sense for their operational strategy rather than rely on base fees. |
|
Just throwing my two cents in here since I was tagged! I think the drawback of including both the base fees and the priority fees in the block commission is that ecosystem "standards" for evaluating a validator's commission could give larger validators a competitive edge. With the fees combined, imagine a scenario where potential stakers are evaluating which node to stake with: a small node and a large node.
To the uninformed retail staker, this might be grounds to stake with the larger node, to save that N%. Thus, the larger node can more effectively leverage low block commission rates to attract stake and increase their MEV profit. I think keeping base fees separate could help level the playing field a bit. The small node could safely rely on base fee revenue, while still advertising a 0% block commission just like the large node. |
|
Yeah but small stake nodes have small base fee revenue because you only earn base fees for blocks that you produce. Sure you will get compensated for executing transactions in your block, but that's not going to be enough to cover operational costs unless you set non-zero commission or have other revenue streams |
That's until you attract enough stake though. Base fee compensation would increase if you're competitively attracting new stake. It could be acceptable to operate at a diminishing loss for a bit as you grow your stake. To be fair, this may not be a strong enough argument for splitting them, since it would mean we'd have less flexibility to redesign block revenue characteristics and we'd be stuck with another lengthy protocol change if enough operators request the ability to distribute all block revenue, but it's worth at least considering IMO. |
Originally posted by @dimandotsol in #123 (comment)