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

Revertible in-memory store #160

Merged
merged 12 commits into from
Apr 24, 2024
Merged

Revertible in-memory store #160

merged 12 commits into from
Apr 24, 2024

Conversation

rnbguy
Copy link
Member

@rnbguy rnbguy commented Feb 22, 2024

Closes #159

Supports revertibility in in-memory store directly, without RevertibleStore wrapper.


Revert in RevertibleStore

The old RevertibleStore relies on maintaining a list of processed operations - write and delete. If it has to revert on a failed transaction, it tries to revert the previous operations.

  • if it was overwrite, overwrite with the old value
  • if it was delete, insert the old value
  • if it was insert, delete the current value

Problem

But this creates a problem with a deterministic Merkle root hash. overwrite-ing doesn't change the Merkle tree - but insert and delete may reorganize the Merkle tree. And we don't want to reorganize a Merkle tree on a failed transaction. That is, storage should have zero effect on a failed transaction.

Also, we don't have deletion support until #141 is merged. This is why we have this non-termination bug present #129.

Revert in new InMemoryStore

The new InMemoryStore has two copies of the current working store. These two copies are staged and pending.

Each transaction works on pending copy. When a transaction returns,

  • If it succeeds, the store applies the transaction changes by copying pending to staged.
  • If it fails, the store reverts the transaction changes by copying staged to pending.

When a block is committed, the staged copy is copied into the record of all committed stores.

Problem

This is not production-friendly. After each transaction, a whole store is copied from pending to staged or staged to pending.

basecoin/store/src/impls/in_memory.rs Show resolved Hide resolved
basecoin/store/src/impls/in_memory.rs Show resolved Hide resolved
basecoin/store/src/impls/in_memory.rs Outdated Show resolved Hide resolved
basecoin/store/src/impls/in_memory.rs Show resolved Hide resolved
basecoin/store/src/types/height.rs Show resolved Hide resolved
Co-authored-by: Sean Chen <seanchen11235@gmail.com>
Copy link
Contributor

@seanchen1991 seanchen1991 left a comment

Choose a reason for hiding this comment

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

Once we address the len method, I think this is good to go 👍

@seanchen1991
Copy link
Contributor

I like the expanded description of the problem you added to the PR description. I think the section on InMemoryStore should be added to the type's docstring.

@seanchen1991 seanchen1991 merged commit af920ef into main Apr 24, 2024
10 checks passed
@seanchen1991 seanchen1991 deleted the rano/store/imp-inmemory-store branch April 24, 2024 14:39
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.

Revertible in-memory store
2 participants