Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tao-stones
Copy link
Contributor

No description provided.

@tao-stones tao-stones marked this pull request as ready for review June 4, 2025 17:04
- 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.

Copy link
Contributor

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?

Copy link
Contributor Author

@tao-stones tao-stones Jun 5, 2025

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 ?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.
Copy link
Contributor

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

Copy link
Contributor Author

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.

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:

  1. relax nonce
  2. relax fee payer
  3. relax ALT
  4. include bank hash verification in block header

Should all be flipped whenever they are done (before alpenglow ideally).

Copy link
Contributor

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.

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

Copy link
Contributor

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?

Copy link
Contributor

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.

@tao-stones tao-stones changed the title Block CU limits overage relaxation SIMD-0295: Block CU limits overage relaxation Jun 4, 2025
@MaxResnick
Copy link

MaxResnick commented Jun 4, 2025

Can we change the name to Relax CU limit overage to match the other SIMD title formats.

@tao-stones tao-stones changed the title SIMD-0295: Block CU limits overage relaxation SIMD-0295: Relax CU limit overage Jun 5, 2025
@ptaffet-jump
Copy link
Contributor

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.

@MaxResnick
Copy link

MaxResnick commented Jun 6, 2025

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.

@apfitzge
Copy link
Contributor

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 N-1 (or whatever parent is) in order to know the state of ALTs at the beginning of block N, since ALT lookups are "frozen" at state at the start of each slot.

@MaxResnick
Copy link

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 N-1 (or whatever parent is) in order to know the state of ALTs at the beginning of block N, since ALT lookups are "frozen" at state at the start of each slot.

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.

Copy link
Contributor

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.
Copy link
Contributor

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
Copy link
Contributor

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.

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.

6 participants