Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: storage override #1425

Merged

Conversation

koushiro
Copy link
Collaborator

@koushiro koushiro commented May 12, 2024

Description

This PR provides a new StorageOverrideHandler type like old OverrideHandle to support different ethereum schema and a fallback (runtime API) implementation.
But it will automatically determine the ethereum schema version of the runtime based on block_hash, and then call the methods of StorageOverride implementation corresponding to the schema version.

Before :

let schema = fc_storage::onchain_storage_schema(client.as_ref(), block_hash);
let handler = overrides
	.schemas
	.get(&schema)
	.unwrap_or(&overrides.fallback);
let block = handler.current_block(block_hash);
let receipts = handler.current_receipts(block_hash);

After:

let block = storage_override.current_block(hash);
let receipts = storage_override.current_receipts(hash);

Addition

This PR uses Option::<EthereumStorageSchema>::None instead of EthereumStorageSchema::Undefined.

/// The schema version for Pallet Ethereum's storage
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Encode, Decode)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum EthereumStorageSchema {
-	// Undefined,
+	// deprecated
+	// #[codec(index = 0)]
+	// Undefined,
+	#[codec(index = 1)]
	V1,
+	#[codec(index = 2)]
	V2,
+	#[codec(index = 3)]
	V3,
}

IMO, this is more consistent with the current semantics: use the storage override corresponding to the schema version as the preference. If there is no ethereum schema in the state, use the runtime api as the fallback implementation, like:

fn current_block(&self, at: B::Hash) -> Option<BlockV2> {
	match self.querier.storage_schema(at) {
		Some(EthereumStorageSchema::V1) => {
			SchemaV1StorageOverrideRef::new(&self.querier).current_block(at)
		}
		Some(EthereumStorageSchema::V2) => {
			SchemaV2StorageOverrideRef::new(&self.querier).current_block(at)
		}
		Some(EthereumStorageSchema::V3) => {
			SchemaV3StorageOverrideRef::new(&self.querier).current_block(at)
		}
		None => self.fallback.current_block(at),
	}
}

@koushiro koushiro requested a review from sorpaas as a code owner May 12, 2024 15:51
@koushiro koushiro requested a review from boundless-forest May 13, 2024 11:56
@koushiro
Copy link
Collaborator Author

cc @nanocryk @librelois

Copy link
Collaborator

@boundless-forest boundless-forest left a comment

Choose a reason for hiding this comment

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

Much clearer than before, LGTM

@koushiro koushiro merged commit 77d7157 into polkadot-evm:master May 15, 2024
4 checks passed
@koushiro koushiro deleted the refactor-storage-override branch May 15, 2024 03:48
boundless-forest pushed a commit to darwinia-network/frontier that referenced this pull request Sep 23, 2024
This PR provides a new `StorageOverrideHandler` type like old `OverrideHandle` to support different ethereum schema and a fallback (runtime API) implementation.
But it will automatically determine the ethereum schema version of the runtime based on `block_hash`, and then call the methods of `StorageOverride` implementation corresponding to the schema version.

Before :

```rust
let schema = fc_storage::onchain_storage_schema(client.as_ref(), block_hash);
let handler = overrides
	.schemas
	.get(&schema)
	.unwrap_or(&overrides.fallback);
let block = handler.current_block(block_hash);
let receipts = handler.current_receipts(block_hash);
```

After:

```rust
let block = storage_override.current_block(hash);
let receipts = storage_override.current_receipts(hash);
```

This PR uses `Option::<EthereumStorageSchema>::None` instead of `EthereumStorageSchema::Undefined`.

```diff
/// The schema version for Pallet Ethereum's storage
pub enum EthereumStorageSchema {
-	// Undefined,
+	// deprecated
+	// #[codec(index = 0)]
+	// Undefined,
+	#[codec(index = 1)]
	V1,
+	#[codec(index = 2)]
	V2,
+	#[codec(index = 3)]
	V3,
}
```

IMO, this is more consistent with the current semantics: use the storage override corresponding to the schema version as the preference. If there is no ethereum schema in the state, use the runtime api as the fallback implementation, like:

```rust
fn current_block(&self, at: B::Hash) -> Option<BlockV2> {
	match self.querier.storage_schema(at) {
		Some(EthereumStorageSchema::V1) => {
			SchemaV1StorageOverrideRef::new(&self.querier).current_block(at)
		}
		Some(EthereumStorageSchema::V2) => {
			SchemaV2StorageOverrideRef::new(&self.querier).current_block(at)
		}
		Some(EthereumStorageSchema::V3) => {
			SchemaV3StorageOverrideRef::new(&self.querier).current_block(at)
		}
		None => self.fallback.current_block(at),
	}
}
```
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants