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

How to bring back Native execution on my chain? #2891

Closed
tdelabro opened this issue Jan 9, 2024 · 13 comments
Closed

How to bring back Native execution on my chain? #2891

tdelabro opened this issue Jan 9, 2024 · 13 comments
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@tdelabro
Copy link

tdelabro commented Jan 9, 2024

Hey,

I know native execution has been removed in a recent update. We can only execute in Wasm, which has a clear impact on my chain TPS.

I want to be able to execute in Native again. How can I have it back?

I think it is still there, somewhere in the codebase, because pallet's tests are still run in Native. So, is it as simple as tweaking a config or changing a bool somewhere, or will it require me to fork Substrate? If I have to fork, could you point me to where is the code I have to change?

Also, I would like to know the exact PR that removed Native execution. It will probably be helpful in my endeavors.

Best regards

@github-actions github-actions bot added the I10-unconfirmed Issue might be valid, but it's not yet known. label Jan 9, 2024
@ggwpez
Copy link
Member

ggwpez commented Jan 9, 2024

The problem is that native is not guaranteed to be deterministic like WASM. I think there were a few MRs about this from @michalkucharczyk.

What kind of performance degradation do you see? Did you compile with lto and codegen-units = 1 to get the most perf?

@bkchr
Copy link
Member

bkchr commented Jan 9, 2024

I know native execution has been removed in a recent update. We can only execute in Wasm, which has a clear impact on my chain TPS.

Yeah, please provide some numbers. For sure wasm is slower, but it should be not that much slower.

@tdelabro
Copy link
Author

tdelabro commented Jan 9, 2024

Here is a portion of my Cargo.toml

[profile.production]
inherits = "release"
codegen-units = 1    # Setting this to 1 allows for more optimizations at the cost of slower compile time
lto = true           # Enables Link Time Optimization, enabling more aggressive optimizations across the entire codebase
opt-level = 3        # Optimize for speed regardless of binary size or compile time
rpath = false        # Disables adding rpath to the binary

lto is enable, codegen-units too.

We are around 40 TPS in Wasm and if we revert back to git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.43" we are at 70TPS.

We use Substrate to run a VM whose execution is provable and deterministic. So we already have this guarantee.
And the Substrate run is just for sequencing, we do another run asynchronously that is provable.
So, for the Substrate run we really just aim for raw performances.

@michalkucharczyk
Copy link
Contributor

michalkucharczyk commented Jan 9, 2024

Here is the list of relevant PRs I am aware of:

We are around 40 TPS in Native..

You meant wasm?

@bkchr
Copy link
Member

bkchr commented Jan 9, 2024

We use Substrate to run a VM whose execution is provable and deterministic.

What kind of VM? Maybe you could just run the VM in a host function.

@tdelabro
Copy link
Author

tdelabro commented Jan 9, 2024

Here is the VM: https://github.com/lambdaclass/cairo-vm
We call the library wrapping it rather than the VM directly: https://github.com/starkware-libs/blockifier
Here is the repo: https://github.com/keep-starknet-strange/madara

Host functions could be an idea. But the Blockifier has to access the storage too.
It injects the storages values into calls to the VM

@tdelabro
Copy link
Author

tdelabro commented Jan 9, 2024

Thanks for the links @michalkucharczyk !

@burdges
Copy link

burdges commented Jan 9, 2024

If you're doing zk starks, then shouldn't you be using all custom storage, except for system paramaters, etc?

@tdelabro
Copy link
Author

tdelabro commented Jan 9, 2024

@burdges

Sorry, I don't understand the implications of your message. Can you explain a bit more what you mean?

@bkchr
Copy link
Member

bkchr commented Jan 9, 2024

Here is the VM: https://github.com/lambdaclass/cairo-vm
We call the library wrapping it rather than the VM directly: https://github.com/starkware-libs/blockifier
Here is the repo: https://github.com/keep-starknet-strange/madara

Yeah, then create some host function for it?

@tdelabro
Copy link
Author

tdelabro commented Jan 9, 2024

Yeah, then create some host function for it?

@bkchr Can you share an example of something similar being achieved?

Is it possible to write to the key value DB from such a host function? Is is written to the DB directly or to the cache?

@bkchr
Copy link
Member

bkchr commented Jan 19, 2024

Depends how you implement it.

Here is an example:

#[runtime_interface]
pub trait StatementStore {
/// Submit a new new statement. The statement will be broadcast to the network.
/// This is meant to be used by the offchain worker.
fn submit_statement(&mut self, statement: Statement) -> SubmitResult {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
match store.submit(statement, StatementSource::Chain) {
crate::SubmitResult::New(_) => SubmitResult::OkNew,
crate::SubmitResult::Known => SubmitResult::OkKnown,
crate::SubmitResult::Ignored => SubmitResult::Full,
// This should not happen for `StatementSource::Chain`. An existing statement will
// be overwritten.
crate::SubmitResult::KnownExpired => SubmitResult::Bad,
crate::SubmitResult::Bad(_) => SubmitResult::Bad,
crate::SubmitResult::InternalError(_) => SubmitResult::Bad,
}
} else {
SubmitResult::NotAvailable
}
}
/// Return all statements.
fn statements(&mut self) -> Vec<(Hash, Statement)> {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
store.statements().unwrap_or_default()
} else {
Vec::default()
}
}
/// Return the data of all known statements which include all topics and have no `DecryptionKey`
/// field.
fn broadcasts(&mut self, match_all_topics: &[Topic]) -> Vec<Vec<u8>> {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
store.broadcasts(match_all_topics).unwrap_or_default()
} else {
Vec::default()
}
}
/// Return the data of all known statements whose decryption key is identified as `dest` (this
/// will generally be the public key or a hash thereof for symmetric ciphers, or a hash of the
/// private key for symmetric ciphers).
fn posted(&mut self, match_all_topics: &[Topic], dest: [u8; 32]) -> Vec<Vec<u8>> {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
store.posted(match_all_topics, dest).unwrap_or_default()
} else {
Vec::default()
}
}
/// Return the decrypted data of all known statements whose decryption key is identified as
/// `dest`. The key must be available to the client.
fn posted_clear(&mut self, match_all_topics: &[Topic], dest: [u8; 32]) -> Vec<Vec<u8>> {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
store.posted_clear(match_all_topics, dest).unwrap_or_default()
} else {
Vec::default()
}
}
/// Remove a statement from the store by hash.
fn remove(&mut self, hash: &Hash) {
if let Some(StatementStoreExt(store)) = self.extension::<StatementStoreExt>() {
store.remove(hash).unwrap_or_default()
}
}
}

@atenjin
Copy link
Contributor

atenjin commented Jan 20, 2024

in fact for now you can still use NativeExecutor for now, only need to specify the NativeElseWasmExecutor in service.rs

pub struct Executor;
impl NativeExecutionDispatch for Executor {
    type ExtendHostFunctions = ();

    fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
        node_template_runtime::api::dispatch(method, data)
    }

    fn native_version() -> NativeVersion {
        node_template_runtime::native_version()
    }
}

pub fn new_native_or_wasm_executor<D: NativeExecutionDispatch>(
    config: &Configuration,
) -> NativeElseWasmExecutor<D> {
    NativeElseWasmExecutor::new_with_wasm_executor(new_wasm_executor(config))
}

this pr show more info for this ##2358, which comes from our team

@bkchr bkchr closed this as completed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Development

No branches or pull requests

6 participants