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

Offchain DB should be fork aware #1125

Closed
acatangiu opened this issue Apr 26, 2022 · 8 comments
Closed

Offchain DB should be fork aware #1125

acatangiu opened this issue Apr 26, 2022 · 8 comments

Comments

@acatangiu
Copy link
Contributor

acatangiu commented Apr 26, 2022

Pallets that are using data in offchain DB saved with keys derived from block numbers end up with data collisions caused by forks.

Working around this issue by using keys derived from fork-unique properties like block hashes can easily lead to complicated user-facing APIs. Example of such issue is the case of pallet-mmr.

The best future-looking solution would be to enhance the offchain DB to handle forks transparently. That way all pallets potentially facing this issue don't have to worry about it.

The proposal is to enhance the Indexing API with fork-specific overlays that get committed to permanent DB on finality. On finality. non-canonical overlays get pruned/discarded.

@cheme
Copy link
Contributor

cheme commented Apr 27, 2022

https://github.com/paritytech/substrate/blob/6e740bc89d097a38d922d5619b55e0c7e8664a6c/primitives/core/src/offchain/mod.rs#L75 is a placeholder for it (I did implement that in the past but code/design was too complex or just bad :) Today, if I wanted a quick implementation, I would just reuse the current merkelized state code (bad perf), to do properly I would probably target primary support at parity-db level (not trivial but related to a design I got in mind for faster state storage and no rocksdb support).

@acatangiu
Copy link
Contributor Author

I would just reuse the current merkelized state code (bad perf)

I don't think we need all the complexity of state db. Unlike state db, the data from offchain db doesn't have to keep state-per-block. I see it as single offchain global state.

For example say value[key] = 7 is assigned at block 5, and value[key] = 42 is overwritten at block 10; any subsequent (runtime) interrogations for value[key] will come back with 42 regardless of at: BlockNumber the runtime is running on.

wdyt?

@cheme
Copy link
Contributor

cheme commented Apr 27, 2022

yes you can have a offchain worker db that is synched with the head of the chain and do not allow querying historical states.
Sounds like a good direction, it simplify the storage design 👍
Remaining difficult part is managing reorgs until block is finalized, but there is code from client-state-db that does it for state and should work for this use case (cc @arkpar ).
Edit: storage of finalized data with sc-state-db over it for non canonical would still allows the to query specifically finalized state which can be good for offchain db api.

@arkpar
Copy link
Member

arkpar commented Apr 27, 2022

Remaining difficult part is managing reorgs until block is finalized, but there is code from client-state-db that does it for state and should work for this use case (cc @arkpar ).

I don't think it will, not as is at least. state-db was designed to manages trie node references, and to make sure the node is removed from the db when all states that reference it are discarded (because of re-orgs or pruning). It assumes that values are preimages. I.e. two different blocks can't insert different value (trie node) under the same key.

@cheme
Copy link
Contributor

cheme commented Apr 27, 2022

Indeed. That 's quite some impl then (similar to ethereum state snapshot).

@arkpar
Copy link
Member

arkpar commented Apr 27, 2022

One issue with maintaining a "global canonical" offchain DB state is that it needs to be synced with the canonical blockchain state. You don't want a race condition when the offchain DB view is updated a bit after the main DB imports or reorgs to a new block.

@acatangiu
Copy link
Contributor Author

No longer a priority (or maybe even necessary - no sure if should close). In MMR we fixed it through paritytech/substrate#11594

@EmmanuellNorbertTulbure EmmanuellNorbertTulbure transferred this issue from paritytech/substrate Aug 25, 2023
@acatangiu
Copy link
Contributor Author

closing this as not planned/required..

claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Jan 26, 2024
252df12a5a4 Adds runtime tests (#1114)
3b17d2a89d2 Refactoring Inbound fixture (paritytech#1125)

git-subtree-dir: bridges/snowbridge
git-subtree-split: 252df12a5a451a03df6fefeec44ac60ea9c7cb0b
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
…itytech#1125)

* add some missing tests to encode_call

* fix tests
bkchr pushed a commit that referenced this issue Apr 10, 2024
* add some missing tests to encode_call

* fix tests
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

No branches or pull requests

3 participants