-
Notifications
You must be signed in to change notification settings - Fork 8
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
trivial extrinsic stalled the parachain #84
Comments
I don't know if it is related but I observed this in the logs: |
we could purge the pending extrinsic with rpc ah. js/apps is so kind to tell us the hash if you go to Network -> Node Info -> Pending extrinsics. execute this on all collators, banning the evil extrinsic:
|
Tested a simple balance transfer which should be filtered anyway because of our extrinsic filter in the shell runtime. stalls the parachain as well
hash: 0xdfbb8bff0d46a56f2ab4c766141833914111053e72576e6b1e9fed84f90f4ade |
Forget this, coming from somewhere else, already fixed. |
after discussing the setup: we may need to repeat because the SetCurrentHead set the head back to the same genesis, not to a new one, which isn't exactly the same setup. edit: the above is not true. still we played it through with the new binary 1.5.13 but really can't reproduce #77 (comment) |
I believe I found a problem: The Kusama relay chain tells me for paraid 2015:
which is the hash of our runtime V2 Our collator thinks we're on runtime V3 queried on our api node wss://kusama.api.integritee.network
which would hash to
I was actually wondering why our runtime jumped to V3 just because we changed It could be related to this error message in the collator log:
the above refers to a relaychain block hash: |
I was able to reproduce our Kusama parachian issue on rococo-local. Follow this precisely: using release binaries:
save binaries in runtimes (just for reference. no need to download for this test):
polkadot-launch script procedure
now edit the spec and set
new-head.state:
Ok, so now we have the same problem like on Kusama. diagnoseThe relaychain thinks we're still running V2, but the collator derive from the newly generated genesis that it has to be V3
why didn't we see this when we tested our fix with forceSetCurrentHead?
fix optionsWe can fix it by relaychain governance: then extrinsics will be accepted again. But we'd like to avoid annoying Kusama governance again.
|
@brenzi congrats, good catch! |
solution idea 1Basically, we need to perform a runtime upgrade with a sudo call on our parachain. If that succeeds, everything is back to normal. a simple sudo extrinsic will not be accepted by relay because the signed extra is different for V3 (which the para uses now) than V2 (which the relay validators use). signed extra includes spec_version and therefore, the PoV will include an extrinsic whose signature will be wrong if verifed against runtime V2 We could patch substrate where it verifies extrinsic signatures and hard-code runtime_version to 2 there. That patch would be used to build a temporary collator binary which only serves to do the runtime upgrade we would need to patch substrate-api-client too, to compose a fake spec-version extrinsic for |
Here are the relaychain logs for the rococo-local case: It seems the suspicion above was right
later we also see this, but that may be because I let my laptop sleep in between (check times in log)
|
We successfully updated our parachain runtime from version v3 to v3 with a To make this happen, we had to tweak the following:
diff --git a/compose-macros/src/lib.rs b/compose-macros/src/lib.rs
index 5ef9ade..ddea999 100644
--- a/compose-macros/src/lib.rs
+++ b/compose-macros/src/lib.rs
@@ -133,7 +133,7 @@ macro_rules! compose_extrinsic {
Era::Immortal,
$api.genesis_hash,
$api.genesis_hash,
- $api.runtime_version.spec_version,
+ 2,
$api.runtime_version.transaction_version
)
} else {
diff --git a/examples/example_setcode_extrinsic.rs b/examples/example_setcode_extrinsic.rs
new file mode 100644
index 0000000..5346832
--- /dev/null
+++ b/examples/example_setcode_extrinsic.rs
@@ -0,0 +1,76 @@
+/*
+ Copyright 2019 Supercomputing Systems AG
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+ http://www.apache.org/licenses/LICENSE-2.0
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+///! Example that shows how to compose a setCode extrinsic.
+/// Upgrade from node 1.05 to 1.0.6: use wasm integritee_node_runtime-v6.compact.wasm
+use clap::{load_yaml, App};
+use sp_core::crypto::Ss58Codec;
+use sp_core::{crypto::Pair, sr25519};
+use support::weights::Weight;
+
+use substrate_api_client::rpc::WsRpcClient;
+use substrate_api_client::{compose_call, compose_extrinsic, Api, UncheckedExtrinsicV4, XtStatus};
+
+fn main() {
+ env_logger::init();
+ let url = get_node_url_from_cli();
+
+ // Alice's seed: subkey inspect //Alice
+ let from: sr25519::Pair = Pair::from_string(
+ "0xe5be9a5092b81bca64be81d212e7f2f9eba183bb7a90954f7b76361f6edb5c0a",
+ None,
+ )
+ .unwrap();
+ println!("signer account: {}", from.public().to_ss58check());
+
+ let client = WsRpcClient::new(&url);
+ let api = Api::new(client)
+ .map(|api| api.set_signer(from.clone()))
+ .unwrap();
+
+ //let new_wasm: &[u8] = include_bytes!("integritee_node_runtime-v6.compact.wasm");
+ let new_wasm: &[u8] = include_bytes!("shell_runtime-v3.compact.compressed.wasm");
+
+ // this call can only be called by sudo
+ #[allow(clippy::redundant_clone)]
+ let call = compose_call!(
+ api.metadata.clone(),
+ "System",
+ "set_code_without_checks",
+ new_wasm.to_vec()
+ );
+
+ let weight: Weight = 0;
+
+ #[allow(clippy::redundant_clone)]
+ let xt: UncheckedExtrinsicV4<_> =
+ compose_extrinsic!(api.clone(), "Sudo", "sudo_unchecked_weight", call, weight);
+
+ // send and watch extrinsic until finalized
+ let tx_hash = api
+ .send_extrinsic(xt.hex_encode(), XtStatus::InBlock)
+ .unwrap();
+
+ println!("[+] Transaction got included. Hash: {:?}", tx_hash);
+}
+
+pub fn get_node_url_from_cli() -> String {
+ let yml = load_yaml!("cli.yml");
+ let matches = App::from_yaml(yml).get_matches();
+
+ let node_ip = matches.value_of("node-server").unwrap_or("ws://127.0.0.1");
+ let node_port = matches.value_of("node-port").unwrap_or("9944");
+ let url = format!("{}:{}", node_ip, node_port);
+ println!("Interacting with node on {}\n", url);
+ url
+}
diff --git a/examples/shell_runtime-v3.compact.compressed.wasm b/examples/shell_runtime-v3.compact.compressed.wasm
new file mode 100644
index 0000000..66283c5
Binary files /dev/null and b/examples/shell_runtime-v3.compact.compressed.wasm differ
diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs
index a677f418be..6c191057c8 100644
--- a/primitives/io/src/lib.rs
+++ b/primitives/io/src/lib.rs
@@ -654,7 +654,7 @@ pub trait Crypto {
///
/// Returns `true` when the verification was successful.
fn ed25519_verify(sig: &ed25519::Signature, msg: &[u8], pub_key: &ed2551 9::Public) -> bool {
- ed25519::Pair::verify(sig, msg, pub_key)
+ true
}
/// Register a `ed25519` signature for batch verification.
@@ -671,9 +671,7 @@ pub trait Crypto {
msg: &[u8],
pub_key: &ed25519::Public,
) -> bool {
- self.extension::<VerificationExt>()
- .map(|extension| extension.push_ed25519(sig.clone(), pub _key.clone(), msg.to_vec()))
- .unwrap_or_else(|| ed25519_verify(sig, msg, pub_key))
+ true
}
/// Verify `sr25519` signature.
@@ -681,7 +679,7 @@ pub trait Crypto {
/// Returns `true` when the verification was successful.
#[version(2)]
fn sr25519_verify(sig: &sr25519::Signature, msg: &[u8], pub_key: &sr2551 9::Public) -> bool {
- sr25519::Pair::verify(sig, msg, pub_key)
+ true
}
/// Register a `sr25519` signature for batch verification.
@@ -698,9 +696,7 @@ pub trait Crypto {
msg: &[u8],
pub_key: &sr25519::Public,
) -> bool {
- self.extension::<VerificationExt>()
- .map(|extension| extension.push_sr25519(sig.clone(), pub _key.clone(), msg.to_vec()))
- .unwrap_or_else(|| sr25519_verify(sig, msg, pub_key))
+ true
}
/// Start verification extension.
@@ -721,15 +717,7 @@ pub trait Crypto {
///
/// Will panic if no `VerificationExt` is registered (`start_batch_verif y` was not called).
fn finish_batch_verify(&mut self) -> bool {
- let result = self
- .extension::<VerificationExt>()
- .expect("`finish_batch_verify` should only be called aft er `start_batch_verify`")
- .verify_and_clear();
-
- self.deregister_extension::<VerificationExt>()
- .expect("No verification extension in current context!") ;
-
- result
+ true
}
/// Returns all `sr25519` public keys for the given key id from the keys tore.
@@ -779,7 +767,7 @@ pub trait Crypto {
/// Returns `true` when the verification in successful regardless of
/// signature version.
fn sr25519_verify(sig: &sr25519::Signature, msg: &[u8], pubkey: &sr25519 :
/// Will panic if no `VerificationExt` is registered (`start_batch_verify` was not called).
fn finish_batch_verify(&mut self) -> bool {
- let result = self
- .extension::<VerificationExt>()
- .expect("`finish_batch_verify` should only be called after `start_batch_verify`")
- .verify_and_clear();
-
- self.deregister_extension::<VerificationExt>()
- .expect("No verification extension in current context!");
-
- result
+ true
}
/// Returns all `sr25519` public keys for the given key id from the keystore.
@@ -779,7 +767,7 @@ pub trait Crypto {
/// Returns `true` when the verification in successful regardless of
/// signature version.
fn sr25519_verify(sig: &sr25519::Signature, msg: &[u8], pubkey: &sr25519::Public) -> bool {
- sr25519::Pair::verify_deprecated(sig, msg, pubkey)
+ true
}
/// Returns all `ecdsa` public keys for the given key id from the keystore.
@@ -843,7 +831,7 @@ pub trait Crypto {
///
/// Returns `true` when the verification was successful.
fn ecdsa_verify(sig: &ecdsa::Signature, msg: &[u8], pub_key: &ecdsa::Public) -> bool {
- ecdsa::Pair::verify_deprecated(sig, msg, pub_key)
+ true
}
/// Verify `ecdsa` signature.
@@ -851,7 +839,7 @@ pub trait Crypto {
/// Returns `true` when the verification was successful.
#[version(2)]
fn ecdsa_verify(sig: &ecdsa::Signature, msg: &[u8], pub_key: &ecdsa::Public) -> bool {
- ecdsa::Pair::verify(sig, msg, pub_key)
+ true
}
/// Verify `ecdsa` signature with pre-hashed `msg`.
@@ -862,7 +850,7 @@ pub trait Crypto {
msg: &[u8; 32],
pub_key: &ecdsa::Public,
) -> bool {
- ecdsa::Pair::verify_prehashed(sig, msg, pub_key)
+ true
}
/// Register a `ecdsa` signature for batch verification.
@@ -879,9 +867,7 @@ pub trait Crypto {
msg: &[u8],
pub_key: &ecdsa::Public,
) -> bool {
- self.extension::<VerificationExt>()
- .map(|extension| extension.push_ecdsa(sig.clone(), pub_key.clone(), msg.to_vec()))
- .unwrap_or_else(|| ecdsa_verify(sig, msg, pub_key))
+ true
} |
the consequence of this fix - however - is that we now have an invalid signature in our collator parachain history and syncing a collator from scratch will stall at the block where we sent the code update. This can be fixed by applying the patched collator binary to go beyond that block and once synched, every "proper" collator binary will work again |
@mosonyi please add artifacts to release
https://github.com/integritee-network/parachain/releases/tag/1.5.15 then we can close this issue |
@brenzi done |
@mosonyi I added instructions to https://github.com/integritee-network/parachain/releases/tag/1.5.15
I'd like these release notes to be comprehensive documentation of how to sync our parachain. Otherwise we'll have to explain 100x |
For traceability, I have pushed two new branches: Tweaked substrate version: https://github.com/integritee-network/substrate/tree/polkadot-v0.9.20-skip-signature-verification The collator binaries can be found here: https://github.com/integritee-network/parachain/actions/runs/2343727441 |
The documentation is in wiki. |
sending a simple
sudo.sudo(balances.setBalance())
extrinsic on a new account bricked our parachain (again)The tx has not yet been included in a finalized block, so it seemns this extrinsic somehow breaks our PoV
The text was updated successfully, but these errors were encountered: