SMT storage key hashing #1207
Merged
SMT storage key hashing #1207
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bvrooman
reviewed
Jun 12, 2023
xgreenx
added a commit
to FuelLabs/fuel-specs
that referenced
this pull request
Jun 12, 2023
We use SMT in two places for contract balances and contract state. While it is not a huge problem for balances SMT root(because `AssetId` is randomly derived from `sha256`), it is a massive problem for contract state root. Each leaf key is specified by the user/developer for the storage key-value pair. The SMT is a vast data structure that uses some optimization that helps to improve its performance and occupied storage. Based on the knowledge of how our SMT works inside, malicious users can manipulate the structure and make it work in a non-optimal way. We've already faced that in the beta3 testnet. [It is a snapshot](https://github.com/FuelLabs/fuel-core/blob/e4f5d65d471954b9cc1148ed067e9bb3f598bb7a/bin/e2e-test-client/src/tests/test_data/large_state/contract.json) of the state of the contract from the beta3 testnet. It has only 30k leafs but because those leafs are close to each other it produces 1.3m of nodes in the SMT. But if we [hash each leaf key](FuelLabs/fuel-core#1207) it reduces the number of the nodes from 1.3m to only 70k. Because of the randomness leafs are distributed in a better way preventing a huge number of empty side nodes. This PR proposes to hash each leaf key of any SMT to prevent any kind of manipulation.
bvrooman
reviewed
Jun 12, 2023
bvrooman
reviewed
Jun 12, 2023
Co-authored-by: Brandon Vrooman <brandon.vrooman@fuel.sh>
leviathanbeak
previously approved these changes
Jun 13, 2023
| .storage::<ContractsAssetsMerkleMetadata>() | ||
| .contains_key(contract_id)? | ||
| { | ||
| Err(anyhow::anyhow!( |
There was a problem hiding this comment.
nit: I'd prefer explicit return over ? but up to you
| .storage::<ContractsStateMerkleMetadata>() | ||
| .contains_key(contract_id)? | ||
| { | ||
| Err(anyhow::anyhow!("The contract state is already initialized"))?; |
crates/fuel-core/src/service.rs
Outdated
Comment on lines
199
to
202
| pub fn new(database: Database, mut config: Config) -> anyhow::Result<Task> { | ||
| // initialize state | ||
| genesis::maybe_initialize_state(&config, &database)?; | ||
| config.chain_conf.initial_state = None; |
There was a problem hiding this comment.
What's the initial_state before this line? Isn't there any other way for this to get initialized to desired state or it has to go from Some to None in here?
There was a problem hiding this comment.
I rollbacked this file because it should be handled separately in the #1209
leviathanbeak
approved these changes
Jun 13, 2023
Merged
xgreenx
added a commit
that referenced
this pull request
Jun 14, 2023
## What's Changed * version compatibility cleanup by @Voxelot in #1171 * Added example with custom query around the `fuel-core-client` by @xgreenx in #1175 * Update to fuel-vm 0.32 (including wideint gas profiling) by @Dentosal in #1173 * feat: Client primitives by @bvrooman in #1144 * Improve executor config by @Voxelot in #1185 * Added `contract_id` to the `ContractConfig` by @xgreenx in #1184 * fix windows file name error by @firedpeanut in #1176 * Make transaction status stream work by @freesig in #1108 * Added `submit_and_await` endpoint to not miss the notifications by @xgreenx in #1192 * feat: Use ID types in client api by @bvrooman in #1191 * Use `fuel-vm 0.33` with predicate estimation by @xgreenx in #1195 * Add transaction lifecycle diagram to the docs by @digorithm in #1201 * sync with peers before producing blocks by @leviathanbeak in #1169 * SMT storage key hashing by @xgreenx in #1207 ## New Contributors * @firedpeanut made their first contribution in #1176 **Full Changelog**: v0.18.1...v0.19.0 --------- Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR uses an upcoming release of the
fuel-vm 0.34.It brings a new important feature - hashing the leaf key for the SMT to prevent tree structure manipulation.
As an example: Hashed storage key decreases the number of nodes in the SMT from 1.3M to 70K for 30K leaves in the
run_contract_large_statee2e test. It improves the test time from 200 seconds to 13 seconds in Debug mode and to 1 second in the release(instead of 20 seconds).So it improves security and performance.
The change is breaking because it affects the
state_rootfield -> generatedContractId.Fixes #1143