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

reward: Fix state regeneration with post-Kaia staking info #43

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

blukat29
Copy link
Contributor

@blukat29 blukat29 commented Jul 8, 2024

Proposed changes

  • Affects

    • debug_trace* APIs on full-mode nodes may have returned incorrect results if the traced transaction touches any validator reward address. e.g. interaction with PublicDelegation contracts. Archive-mode nodes are unaffected.
  • Background

    • State regeneration (or reexec) is a process of generating the world state at a given block when the state is not in the database. It works by re-executing the transactions (and post-transaction state transitions by Finalize()), starting from the nearest block in which the state exists in the database.
    • State regen is primarily used in debug trace APIs. In those APIs, a block or a transaction is executed against a historic block state. To support this API even in full nodes, the block state is regenerated if not exist in the database.
    • During state regen, the Finalize() queries the staking info for the block to calculate rewards. Without the staking info, we cannot regenerate the correct state.
    • Since Kaia hardfork, the staking info is calculated from the previous block's state.
  • Problem

  • Solution

    • During state regen, preload staking info from the ephemeral state.
      • Load state at 10. Preload staking info from state at 10.
      • Execute block 11 to regen state at 11. Use preloaded staking info. Preload staking info from state at 11.
      • Execute block 12 to regen state at 12. Use preloaded staking info. Preload staking info from state at 12.
      • Delete preloaded staking info because it's only for regen.
    • Because preloaded staking info can be concurrently used by multiple threads (e.g. concurrent debug API calls), it is implemented with a reference counting data structure.

Types of changes

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have read the CLA and signed by comment I have read the CLA Document and I hereby sign the CLA in first time contribute
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Related issues

Further comments

@blukat29 blukat29 changed the title wip Fix state regeneration with post-Kaia staking info Jul 8, 2024
@blukat29 blukat29 force-pushed the fix-reexec-stakinginfo branch from a912b3f to 38a6f51 Compare July 8, 2024 05:17
@blukat29 blukat29 force-pushed the fix-reexec-stakinginfo branch 2 times, most recently from 293b303 to e0e8f1f Compare July 8, 2024 14:32
@blukat29 blukat29 marked this pull request as ready for review July 8, 2024 14:53
@blukat29 blukat29 changed the title Fix state regeneration with post-Kaia staking info reward: Fix state regeneration with post-Kaia staking info Jul 9, 2024
@blukat29 blukat29 force-pushed the fix-reexec-stakinginfo branch from e0e8f1f to 31cb5d3 Compare July 9, 2024 01:54
@blukat29 blukat29 force-pushed the fix-reexec-stakinginfo branch from 31cb5d3 to c14f8c2 Compare July 9, 2024 03:17
@blukat29
Copy link
Contributor Author

blukat29 commented Jul 9, 2024

@yoomee1313 @hyeonLewis @hyunsooda PTAL. Tell me if you need further context.

@blukat29 blukat29 force-pushed the fix-reexec-stakinginfo branch from 4e065d1 to e51c56f Compare July 9, 2024 05:58
@hyunsooda
Copy link
Contributor

I think cache seems more appropriate rather than preload, which seems ambiguous. Can you please provide motivation for such a name?

@blukat29
Copy link
Contributor Author

blukat29 commented Jul 10, 2024

@hyunsooda Thanks for asking that. Preload is a strictly distinct feature from the cache. The difference is eviction.

  • Cache (stakingInfoCache) can be safely evicted because we can read from database upon cache miss. It is for optimization purpose and non-essential for correct operation.
  • Preload (preloadedInfo) must not be evicted because there's no way we can re-calculate from the state. It is for correct operation.

When I named the feature, I ruled out 'cache' because it implies it can be evicted. So I used the name 'preload'. I also considered 'prefetch' but didn't choose it because prefetch also implies optimization.

@blukat29 blukat29 mentioned this pull request Jul 10, 2024
22 tasks
@blukat29 blukat29 merged commit e8b8e10 into kaiachain:dev Jul 11, 2024
11 checks passed
@blukat29 blukat29 deleted the fix-reexec-stakinginfo branch July 11, 2024 03:34
@github-actions github-actions bot locked and limited conversation to collaborators Jul 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants