From 7c7976dea1496296951ee0111a8ac6680fd0b19a Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 16:42:28 +0900 Subject: [PATCH 01/10] EXPERIMENTAL: Bypass signature verification in lightclient We implement the light client only for dynamic validator. Because the static validator will be removed. Currently, the static validator is not removed yet. Only for this experimental branch, skip the verification of validator set. --- core/src/consensus/light_client/verification.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/src/consensus/light_client/verification.rs b/core/src/consensus/light_client/verification.rs index fa2840db4a..55c2ac910c 100644 --- a/core/src/consensus/light_client/verification.rs +++ b/core/src/consensus/light_client/verification.rs @@ -66,6 +66,13 @@ pub fn verify_header(client_state: &ClientState, proposal: &UpdateHeader) -> boo if client_state.next_validator_set_hash != proposal.validator_set.hash() { return false } + + // FIXME: We should remove this if statement when the static validator is removed. + let bypass_verification = std::env::var("BYPASS_VERIFICATION_IN_STATIC_VALIDATOR"); + if bypass_verification.is_ok() { + return true + } + if !verify_signature(proposal.hash, &proposal.validator_set, &proposal.seal) { return false } From b67cb6de187d2f26b9a4c379faf41750338f1386 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 17:34:55 +0900 Subject: [PATCH 02/10] EXPERIMENTAL: Wait a second before creating a block In the IBC scenario, a chain can creates blocks faster than 1 block/sec. Since Foundry does not allow blocks with the same timestamp, we should make the chain slow. This commit should not be included in the master branch --- core/src/miner/miner.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 2f7f2df0a3..770dcc4da8 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -328,6 +328,14 @@ impl Miner { }; assert!(self.engine.seals_internally(), "If a signer is not prepared, prepare_block should not be called"); + + // FIXME this sleeping is for the experimental branch. This should not be included in the master branch. + let wait = std::env::var("WAIT_1_SEC_BEFORE_CREATING_A_BLOCK"); + if wait.is_ok() { + cinfo!(MINER, "Wait 1 second before generating a block"); + std::thread::sleep(std::time::Duration::from_secs(1)); + } + let seal = self.engine.generate_seal(None, &parent_header.decode()); if let Some(seal_bytes) = seal.seal_fields() { open_block.seal(self.engine.borrow(), seal_bytes).expect("Sealing always success"); From 5a77ac19b79151c647677e6a5fcdb76c87d16956 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Thu, 5 Mar 2020 12:23:53 +0900 Subject: [PATCH 03/10] Use special environment variables in the IBC test scenario * WAIT_1_SEC_BEFORE_CREATING_A_BLOCK It will make Tendermint spend more than a second to create a block. * BYPASS_VERIFICATION_IN_STATIC_VALIDATOR It will make the light client skip signature verification. Since we didn't make verification logic for the static validator set, we can't validate it now. The static validator set will be removed soon. --- ibc.ts/src/scenario/runChains.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ibc.ts/src/scenario/runChains.ts b/ibc.ts/src/scenario/runChains.ts index cf5cde5e31..b188a6205d 100644 --- a/ibc.ts/src/scenario/runChains.ts +++ b/ibc.ts/src/scenario/runChains.ts @@ -53,6 +53,11 @@ async function runChainA() { "../../target/debug/foundry", ["-c", "./chainA.schem.json", "--config", "./chainA.config.toml"], { + env: { + ...process.env, + WAIT_1_SEC_BEFORE_CREATING_A_BLOCK: "true", + BYPASS_VERIFICATION_IN_STATIC_VALIDATOR: "true" + }, cwd: "./chainA" } ); @@ -64,6 +69,11 @@ async function runChainB() { "../../target/debug/foundry", ["-c", "./chainB.schem.json", "--config", "./chainB.config.toml"], { + env: { + ...process.env, + WAIT_1_SEC_BEFORE_CREATING_A_BLOCK: "true", + BYPASS_VERIFICATION_IN_STATIC_VALIDATOR: "true" + }, cwd: "./chainB" } ); From a2a581d0618f962631e7deff961d5b59670b9acc Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Tue, 3 Mar 2020 19:05:36 +0900 Subject: [PATCH 04/10] Make the UpdateClient function use the correct method name in log --- core/src/ibc/transaction_handler/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/ibc/transaction_handler/mod.rs b/core/src/ibc/transaction_handler/mod.rs index 2489838b64..e5a30f1f05 100644 --- a/core/src/ibc/transaction_handler/mod.rs +++ b/core/src/ibc/transaction_handler/mod.rs @@ -59,7 +59,7 @@ pub fn execute( header, } => { let mut client_manager = ibc_client::Manager::new(&mut context); - client_manager.update(&id, header).map_err(|err| RuntimeError::IBC(format!("CreateClient: {:?}", err)))?; + client_manager.update(&id, header).map_err(|err| RuntimeError::IBC(format!("UpdateClient: {:?}", err)))?; Ok(()) } Datagram::ConnOpenInit { From 02095c0f57c351b38f87d91a998598747c83562d Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 16:12:51 +0900 Subject: [PATCH 05/10] Add IBC and Lightclient log --- Cargo.lock | 1 + core/Cargo.toml | 1 + .../consensus/light_client/verification.rs | 31 +++++++++++++++++-- core/src/ibc/client_02/manager.rs | 1 + core/src/ibc/transaction_handler/mod.rs | 2 ++ util/logger/src/macros.rs | 6 ++++ 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e93e759535..8bd32b144b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -344,6 +344,7 @@ dependencies = [ "rlp", "rlp_compress", "rlp_derive", + "rustc-hex 1.0.0", "snap", "table", ] diff --git a/core/Cargo.toml b/core/Cargo.toml index 833fca2259..923c7fc651 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -30,6 +30,7 @@ num-rational = "0.2.1" parking_lot = "0.6.0" primitives = { git = "https://github.com/CodeChain-io/rust-codechain-primitives.git", version = "0.4" } rand = "0.6.1" +rustc-hex = "1.0" rlp = { git = "https://github.com/CodeChain-io/rlp.git", version = "0.4" } rlp_compress = { git = "https://github.com/CodeChain-io/rlp.git", version = "0.2" } rlp_derive = { git = "https://github.com/CodeChain-io/rlp.git", version = "0.2" } diff --git a/core/src/consensus/light_client/verification.rs b/core/src/consensus/light_client/verification.rs index 55c2ac910c..b3ccd99a83 100644 --- a/core/src/consensus/light_client/verification.rs +++ b/core/src/consensus/light_client/verification.rs @@ -26,18 +26,31 @@ pub fn verify_signature(block_hash: H256, vset: &CompactValidatorSet, seal: &Sea let seal_view = TendermintSealView::new(&seal.raw).signatures(); if seal_view.is_err() { + cdebug!(LIGHT_CLIENT, "verify_signature: seal is not a valis tendermint seal"); return false } let seal_dec = seal_view.unwrap(); for (index, sign) in &seal_dec { if *index >= n { + cdebug!( + LIGHT_CLIENT, + "verify_signature: index {} is equal or greater than validator set size {}", + index, + n + ); return false } match verify_schnorr(&vset[*index].public_key, &sign, &block_hash) { Ok(true) => (), - Ok(false) => return false, - _ => return false, + Ok(false) => { + cdebug!(LIGHT_CLIENT, "verify_signature: verify_schnorr verified that signature is not correct"); + return false + } + _ => { + cdebug!(LIGHT_CLIENT, "verify_signature: verify_schnorr failed"); + return false + } }; } true @@ -61,9 +74,21 @@ pub fn verify_quorum(vset: &CompactValidatorSet, seal: &Seal) -> bool { pub fn verify_header(client_state: &ClientState, proposal: &UpdateHeader) -> bool { if client_state.number + 1 != proposal.number { + ctrace!( + IBC, + "verify_header: The number in the header does not match. expted: {} given: {}", + client_state.number + 1, + proposal.number + ); return false } if client_state.next_validator_set_hash != proposal.validator_set.hash() { + ctrace!( + IBC, + "verify_header: Next validator set hash does not match. expected: {} given: {}", + client_state.next_validator_set_hash, + proposal.validator_set.hash() + ); return false } @@ -74,9 +99,11 @@ pub fn verify_header(client_state: &ClientState, proposal: &UpdateHeader) -> boo } if !verify_signature(proposal.hash, &proposal.validator_set, &proposal.seal) { + ctrace!(IBC, "verify_header: Signature verification of seal failed"); return false } if !verify_quorum(&proposal.validator_set, &proposal.seal) { + ctrace!(IBC, "verify_header: Qurom not met"); return false } true diff --git a/core/src/ibc/client_02/manager.rs b/core/src/ibc/client_02/manager.rs index babff3e898..0638305d26 100644 --- a/core/src/ibc/client_02/manager.rs +++ b/core/src/ibc/client_02/manager.rs @@ -57,6 +57,7 @@ impl<'a> Manager<'a> { pub fn update(&mut self, id: IdentifierSlice, header: Bytes) -> Result<(), String> { let header_dec: Header = rlp::decode(&header).map_err(|_| "Failed to decode IBC header")?; + cdebug!(IBC, "Decoded header in update: {:?}", header_dec); let client_state = self.query(id)?; let (new_client_state, new_consensus_state) = super::client::check_validity_and_update_state(&client_state, &header_dec)?; diff --git a/core/src/ibc/transaction_handler/mod.rs b/core/src/ibc/transaction_handler/mod.rs index e5a30f1f05..446477800a 100644 --- a/core/src/ibc/transaction_handler/mod.rs +++ b/core/src/ibc/transaction_handler/mod.rs @@ -27,6 +27,7 @@ use ibc::client_02 as ibc_client; use ibc::connection_03 as ibc_connection; use ibc::context as ibc_context; use rlp::{Decodable, Rlp}; +use rustc_hex::ToHex; pub fn execute( bytes: &[u8], @@ -58,6 +59,7 @@ pub fn execute( id, header, } => { + cdebug!(IBC, "Update client {} {}", id, header.to_hex()); let mut client_manager = ibc_client::Manager::new(&mut context); client_manager.update(&id, header).map_err(|err| RuntimeError::IBC(format!("UpdateClient: {:?}", err)))?; Ok(()) diff --git a/util/logger/src/macros.rs b/util/logger/src/macros.rs index 9340e212de..a99a04747e 100644 --- a/util/logger/src/macros.rs +++ b/util/logger/src/macros.rs @@ -97,6 +97,12 @@ macro_rules! log_target { (TX) => { "tx" }; + (IBC) => { + "ibc" + }; + (LIGHT_CLIENT) => { + "light_client" + }; } #[macro_export] From fa51eb3f98edbde555d0fae460a025e71b48c3ca Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 16:52:14 +0900 Subject: [PATCH 06/10] Make chain slow in the IBC scenario Relayer should run harder to update light clients if the chain creates block too fast. This commit will not affect consensus. This commit makes running the IBC scenario convenient. --- ibc.ts/chainA/chainA.schem.json | 2 +- ibc.ts/chainB/chainB.schem.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ibc.ts/chainA/chainA.schem.json b/ibc.ts/chainA/chainA.schem.json index 33bcaac6bb..930507087e 100644 --- a/ibc.ts/chainA/chainA.schem.json +++ b/ibc.ts/chainA/chainA.schem.json @@ -6,7 +6,7 @@ "validators": [ "0xc7db8f558a87a45d3b4d2e7422abb48a27e6eeee00a0f884ca244d1d783d6ead2ca445e9ac8f08b311d78b4b4b69d48a9daf0c98d2a19e87e482f8060138ba88" ], - "timeoutPropose": 3000, + "timeoutPropose": 10000, "timeoutProposeDelta": 500, "timeoutPrevote": 1000, "timeoutPrevoteDelta": 500, diff --git a/ibc.ts/chainB/chainB.schem.json b/ibc.ts/chainB/chainB.schem.json index 6c70132af3..da9ae5b140 100644 --- a/ibc.ts/chainB/chainB.schem.json +++ b/ibc.ts/chainB/chainB.schem.json @@ -6,7 +6,7 @@ "validators": [ "0x521d103d95b5b684f9be089ccc5cffd6b4ff2997ce535a7053e44d811b84f38abf2f35ebfeb5458e86a0b7d7293e71bcb3a59eddeb8b63801c2c4a8081fc67ab" ], - "timeoutPropose": 3000, + "timeoutPropose": 10000, "timeoutProposeDelta": 500, "timeoutPrevote": 1000, "timeoutPrevoteDelta": 500, From 004f660d0167af4f46371b3f6c56d1b207ef58d3 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 16:13:12 +0900 Subject: [PATCH 07/10] Make runChains be able to run with pre-existing DB Removing pending transactions helps to run chains with a reliable state --- ibc.ts/src/scenario/runChains.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ibc.ts/src/scenario/runChains.ts b/ibc.ts/src/scenario/runChains.ts index b188a6205d..51f04003e8 100644 --- a/ibc.ts/src/scenario/runChains.ts +++ b/ibc.ts/src/scenario/runChains.ts @@ -101,6 +101,11 @@ async function checkChainAAndBAreRunning() { debug("Send ping to B"); await sdkB.rpc.node.ping(); + debug("Delete pending Txs in A"); + await sdkA.rpc.sendRpcRequest("mempool_deleteAllPendingTransactions", []); + debug("Delete pending Txs in B"); + await sdkB.rpc.sendRpcRequest("mempool_deleteAllPendingTransactions", []); + await sendPayTx({ sdk: sdkA, from: "accqym7qmn5yj29cdl405xlmx6awd3f3yz07g7vq2c9", @@ -131,7 +136,7 @@ async function sendPayTx({ account: from, passphrase: "", fee: 1000, - seq: 0 + seq: await sdk.rpc.chain.getSeq(from) }); debug(`Send payTx to ${chainName}`); const txhash = await sdk.rpc.chain.sendSignedTransaction(signedPay); From c41a974ceefb2b692e0cc77294512bfcb2d8d693 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 17:35:31 +0900 Subject: [PATCH 08/10] Send transactions together If we send transactions without waiting for inclusion, Foundry can create a block that has more than one transaction. --- ibc.ts/src/common/chain.ts | 33 +++++++++++++++++++++++++-------- ibc.ts/src/relayer/index.ts | 8 ++------ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/ibc.ts/src/common/chain.ts b/ibc.ts/src/common/chain.ts index a830bc9ec5..e2354ba36c 100644 --- a/ibc.ts/src/common/chain.ts +++ b/ibc.ts/src/common/chain.ts @@ -54,17 +54,34 @@ export class Chain { } public async submitDatagram(datagram: Datagram): Promise { - const ibcAction = new IBC(this.sdk.networkId, datagram.rlpBytes()); + await this.submitDatagrams([datagram]); + } + public async submitDatagrams(datagrams: Datagram[]): Promise { + const txHashes = []; const seq = await this.sdk.rpc.chain.getSeq(this.faucetAddress); - const signedTx = await this.sdk.key.signTransaction(ibcAction, { - account: this.faucetAddress, - fee: 100, - seq - }); - const txHash = await this.sdk.rpc.chain.sendSignedTransaction(signedTx); - await waitForTx(this.sdk, txHash); + for (let i = 0; i < datagrams.length; i += 1) { + const datagram = datagrams[i]; + const ibcAction = new IBC(this.sdk.networkId, datagram.rlpBytes()); + + const signedTx = await this.sdk.key.signTransaction(ibcAction, { + account: this.faucetAddress, + fee: 100, + seq: seq + i + }); + + debug(`Send tx with seq ${seq + i}`); + const txHash = await this.sdk.rpc.chain.sendSignedTransaction( + signedTx + ); + txHashes.push(txHash); + } + + for (const txHash of txHashes) { + debug(`Wait for tx ${txHash}`); + await waitForTx(this.sdk, txHash); + } } public async latestHeight(): Promise { diff --git a/ibc.ts/src/relayer/index.ts b/ibc.ts/src/relayer/index.ts index 6442e3af85..89c488fdd0 100644 --- a/ibc.ts/src/relayer/index.ts +++ b/ibc.ts/src/relayer/index.ts @@ -61,13 +61,9 @@ async function relayFromTo({ counterpartyChain }); - for (const localDiagram of localDatagrams) { - await chain.submitDatagram(localDiagram); - } + await chain.submitDatagrams(localDatagrams); - for (const counterpartyDatagram of counterpartyDatagrams) { - await counterpartyChain.submitDatagram(counterpartyDatagram); - } + await counterpartyChain.submitDatagrams(counterpartyDatagrams); } async function pendingDatagrams({ From 3692890fa0a3ce9a5ad840e9ffcc85c51d0159c8 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 17:36:29 +0900 Subject: [PATCH 09/10] Add an assertion that composedHeader should not be null --- ibc.ts/src/relayer/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ibc.ts/src/relayer/index.ts b/ibc.ts/src/relayer/index.ts index 89c488fdd0..5ee550e297 100644 --- a/ibc.ts/src/relayer/index.ts +++ b/ibc.ts/src/relayer/index.ts @@ -5,6 +5,7 @@ import { delay } from "../common/util"; import { getConfig } from "../common/config"; import { PlatformAddress } from "codechain-primitives/lib"; import { UpdateClientDatagram } from "../common/datagram/updateClient"; +import { strict as assert } from "assert"; require("dotenv").config(); @@ -123,6 +124,7 @@ async function updateLightClient({ const header = (await counterpartyChain.queryIBCHeader( currentBlockNumber + 1 ))!; + assert.notEqual(header, null, "Composed header should not be null"); datagrams.push( new UpdateClientDatagram({ id: chain.counterpartyIdentifiers.client, From 8831e61c33dc8fae63362c48dca3ab4778ae6976 Mon Sep 17 00:00:00 2001 From: Park Juhyung Date: Wed, 4 Mar 2020 17:36:45 +0900 Subject: [PATCH 10/10] Get the correct counterpartyChainHeight --- ibc.ts/src/relayer/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc.ts/src/relayer/index.ts b/ibc.ts/src/relayer/index.ts index 5ee550e297..85b3180dbf 100644 --- a/ibc.ts/src/relayer/index.ts +++ b/ibc.ts/src/relayer/index.ts @@ -75,7 +75,7 @@ async function pendingDatagrams({ counterpartyChain: Chain; }): Promise<{ localDatagrams: Datagram[]; counterpartyDatagrams: Datagram[] }> { const height = await chain.latestHeight(); - const counterpartyChainHeight = await chain.latestHeight(); + const counterpartyChainHeight = await counterpartyChain.latestHeight(); let localDatagrams: Datagram[] = []; let counterpartyDatagrams: Datagram[] = [];