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

CAP 62 #1576

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CAP 62 #1576

wants to merge 1 commit into from

Conversation

SirTyson
Copy link
Contributor

@SirTyson SirTyson commented Dec 2, 2024

This CAP introduces "partial state archival".

Instead of deleting evicted state initially in protocol 23, I'm proposing only implementing the "Hot Archive" initially. This will allow prioritization of live Soroban state (as in caching all live state in memory). See more context here. Full State Archival will still be necessary long term, but can be implemented in a later protocol release.

This CAP discusses this Hot Archive implementation and related changes to the storage fee system.

@SirTyson SirTyson requested a review from dmkozh December 2, 2024 21:03
Comment on lines +97 to +98
| [CAP-0057](cap-0057.md) | Smart Contract Standardized Asset (Stellar Asset Contract) Extension: Memo | Tomer Weller | Draft |
| [CAP-0057](cap-0057.md) | Soroban Live State Prioritization | Garand Tyson | Draft |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird upstream merge and wrong link in your CAP?


### XDR changes

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is usually rendered as a diff to make it easier to read 🤔


#### Hot Archive BucketList

The Hot Archive is a BucketList that stores evicted `PERSISTENT` entries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing period

Suggested change
The Hot Archive is a BucketList that stores evicted `PERSISTENT` entries
The Hot Archive is a BucketList that stores evicted `PERSISTENT` entries.

Comment on lines +200 to +203
Whenever an entry is restored via `RestoreFootprintOp`, the `LedgerEntry` being restored and its
associated TTL will be emitted as a `LedgerEntryChange` of type `LEDGER_ENTRY_RESTORE`. Note that
entries being restored may not have been evicted such that entries currently in the Live State BucketList
can see `LEDGER_ENTRY_RESTORE` events.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this

Note that entries being restored may not have been evicted such that entries currently in the Live State BucketList can see LEDGER_ENTRY_RESTORE events.

I'd like to make sure I understand it correctly: Are you saying that LEDGER_ENTRY_RESTORE does not necessarily mean restoration from eviction because live state may have an expired-yet-not-evicted entry that can be restored before it pops off the end of the BL?

If yes, I'd like to suggest a rewording:

Suggested change
Whenever an entry is restored via `RestoreFootprintOp`, the `LedgerEntry` being restored and its
associated TTL will be emitted as a `LedgerEntryChange` of type `LEDGER_ENTRY_RESTORE`. Note that
entries being restored may not have been evicted such that entries currently in the Live State BucketList
can see `LEDGER_ENTRY_RESTORE` events.
Whenever an entry is restored via `RestoreFootprintOp`, the `LedgerEntry` being restored and its
associated TTL will be emitted as a `LedgerEntryChange` of type `LEDGER_ENTRY_RESTORE`. Note that
entries can be restored from both the Live State and Hot Archive BucketList, because archived entries
continue to exist in Live State until they are evicted as part of natural BucketList growth. The emitted
meta does not distinguish between these.

The reasoning behind introducing CAP-0057 before it was strictly necessary was to prevent breakage of applications. If applications
and users were not prepared for State Archival, it would be very difficult to enable CAP-0057. However, because
[CAP-0046-12](./cap-0046-12.md) is implemented and State Archival semantics are already required, upgrading to CAP-0057 should not
be breaking, especially since much of the complexity, like proof generation, is handled automatically by RPC endpoints.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
be breaking, especially since much of the complexity, like proof generation, is handled automatically by RPC endpoints.
be breaking, especially since much of the complexity, like proof generation, will be handled automatically by new Stellar Core HTTP endpoints.

Is that correct? While RPC will be the facilitator via simulateTransaction, the proofs are actually generated by /getrestorationproof per stellar/go#5426, right?

Comment on lines +260 to +261
The rate of eviction (i.e. the rate at which state is added to the Hot Archive) can already be controlled
via network config settings. The rate at which state is added to the Live BucketList is also already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on this? I was under the impression that eviction is controlled by state TTL rather than network configurations. Or is this referencing the eviction of already-archived entries in the live BL?

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.

2 participants