solana icon indicating copy to clipboard operation
solana copied to clipboard

Move accounts data size checks to end of ReplayStage

Open brooksprumo opened this issue 1 year ago • 2 comments

Problem

Accounts data size checks must be deterministic. Their current location is not, since batches are replayed in parallel, thus transaction processing order is non deterministic.

For more context, see https://github.com/solana-labs/solana/issues/26439

Summary of Changes

All the checks for accounts data size violations have been moved to the end of replay stage, once the bank is complete. At this point, we check the accounts data size to see if there were violations either within the block or for the total size.

This must happen deterministically. Are there any negative performance impacts these checks may have? I've tried to make the checks as small and infrequent as possible; can they be further reduced?

Fixes #26439 Feature Gate Issue: modifies #24135 and #25517

Builds on
  • [x] #26772
  • [x] #26776
  • [x] #26773

brooksprumo avatar Jul 22 '22 18:07 brooksprumo

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Jul 31 '22 06:07 stale[bot]

Hi folks, thanks in advance for taking a look to review this PR. For some additional context, here's why I've asked each of you:

@brennanwatt: You touched this code most recently but allowing concurrent replay. Would love your expertise on perf and soundness. @AshwinSekar: I ran these ideas by you, so wanted to make sure this looked right by you. @jstarry: You found the issue to begin with. @Lichtso: You wanted to make sure we still have test coverage for the accounts data size checks that were removed in other locations.

Also, please let me know if some parts need more explanation. Each person likely has a different level of familiarity with both this code and this issue, so happy to help in anyway.

brooksprumo avatar Aug 08 '22 17:08 brooksprumo

Closing this PR as it cannot be merged. It would introduce potential non-determinism. For more information, please refer to https://github.com/solana-labs/solana/issues/27029

brooksprumo avatar Feb 10 '23 22:02 brooksprumo