-
Notifications
You must be signed in to change notification settings - Fork 177
SIMD-0295: Relax CU limit overage #295
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?
SIMD-0295: Relax CU limit overage #295
Conversation
- Treat the block as committed but stateless — no accounts are modified, no | ||
fee collected, no nonce advanced if apply. | ||
- Maintain consensus liveness and ledger continuity. | ||
|
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.
how does succeeding block poh work with a skipped block parent? am i treating this the same as a skipped slot and basing/ticking of the skipped block's parent or is there some new behavior?
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.
Great question! I don't have good answer. Scanning through code for skip slot logic, I feel same can be applied to Skipped block - its PoH Hashes should also be skipped, its child slot should based on Skipped block's parent. wdyt @MaxResnick ?
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 feature is to be activated after alpenglow, at which point, PoH is no more.
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.
IIRC the idea is that this is treated the same from PoH perspective as a normal (not skipped) slot. PoH isn't rolled back, only the state changes are.
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.
Agreed, this should be treated as a normal block wrt PoH otherwise we need to modify tick verification for child blocks.
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.
Ok, added "to advance poh and tick count".
How about slot_history
sysvar? stateless block
does not committing any account state changes, means its slot is not added to slot_history
. Will this impact vote or fork choice?
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.
I would expect we still populate the minimal account changes in new_from_parent
, namely the sysvars / feature flags / rewards etc. If the SlotHistory
sysvar is not populated then this will break TowerBFT voting in subsequent slots.
What I would expect in the implementation is the bank is dropped once the CU limit is hit, we create a new empty bank as if we are planning to replay, and then immediately set the tick height to max (assuming tick verify is successful).
## Backwards Compatibility | ||
|
||
This proposal changes validator replay logic, should be controlled behind a | ||
feature gate, activated only when async execution is enabled. |
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.
so this activates with asynce execution, not before?
does that make alpenglow a prereq?
if not, how does voting on a skipped block work? presumably i should be locked out of voting similarly to voting down a wrong fork
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.
Current plan is to get implementation ready behind feature gate, but wait for alpenglow to go out first, then flip the async flag.
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 feature flag and the remove bankhash from block id (not yet finished) should be activated after alpenglow rolls out. The other SIMDS:
- relax nonce
- relax fee payer
- relax ALT
- include bank hash verification in block header
Should all be flipped whenever they are done (before alpenglow ideally).
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 there a specific reason to block this by alpenglow? Without async there's no benefit to this, but there's also afaict not a downside except the risk of activation, which is there regardless of when we activate.
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.
I think the reason to wait would be to avoid handling the complexity of POH interactions
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.
We don't need to do anything for poh though, right?
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.
It should be fine to turn this on before alpenglow, voting will treat this as a regular block, the only difference is that replay views this as an empty 0 transaction bank.
There should be no change to PoH, the ticks from this empty bank should still be verified as normal, it is still a part of the finalized ledger.
Can we change the name to Relax CU limit overage to match the other SIMD title formats. |
A replayer can know whether a block's total requested CUs exceeds the block limits without executing the transactions. What about using that instead? A block is marked as dead if its total requested CUs exceeds the block limits. Assuming #172 does what it's supposed to do, this shouldn't be as big of a deal. |
I would love to get to requested CU as well. The only issue that @apfitzge raised with ALT is it would be difficult to check the per account CU limits. This change works well for per account CU limits and It would be a shame to lose the throughput of used CU limit if it doesn't work for both block level and per account CU limits. |
It's not so much about difficult to check the per account CU limits; it's impossible without loading some account state. To validate block N we wouldn't need to execute block N, but we'd need to have executed up to block |
But still the static check requires us to load all ALTs before voting which seems not ideal. |
- Treat the block as committed but stateless — no accounts are modified, no | ||
fee collected, no nonce advanced if apply. | ||
- Maintain consensus liveness and ledger continuity. | ||
|
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.
Agreed, this should be treated as a normal block wrt PoH otherwise we need to modify tick verification for child blocks.
## Backwards Compatibility | ||
|
||
This proposal changes validator replay logic, should be controlled behind a | ||
feature gate, activated only when async execution is enabled. |
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.
It should be fine to turn this on before alpenglow, voting will treat this as a regular block, the only difference is that replay views this as an empty 0 transaction bank.
There should be no change to PoH, the ticks from this empty bank should still be verified as normal, it is still a part of the finalized ledger.
Change the block CU validation logic during replay as follows: | ||
|
||
- If a block exceeds the Block CU limit during replay execution: | ||
- Stop execution of remaining transactions in the block, and rollback all |
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.
Alpenglow has a similar construct where a block can become stateless midway (and later restarted off a different parent). I don't think this validation logic will interfere, just something to note.
No description provided.