From bf5820328a630fd21fa1064d268fb8057482314a Mon Sep 17 00:00:00 2001 From: Jared Flatow Date: Fri, 25 Jun 2021 16:33:55 -0700 Subject: [PATCH] Prevent the extra block fetch from worker + small fixes + bump m16 (#387) --- ethereum-client/src/lib.rs | 9 ++--- pallets/cash/fuzz/Cargo.lock | 1 + pallets/cash/src/chains.rs | 8 +++++ pallets/cash/src/internal/events.rs | 52 +++++++++++++++++++++-------- pallets/cash/src/tests/mod.rs | 4 +-- pallets/cash/src/tests/testdata.rs | 30 ++++++++--------- pallets/cash/src/tests/worker.rs | 22 ++++++------ runtime/src/lib.rs | 2 +- 8 files changed, 81 insertions(+), 47 deletions(-) diff --git a/ethereum-client/src/lib.rs b/ethereum-client/src/lib.rs index 5f499edb9..7bde3c492 100644 --- a/ethereum-client/src/lib.rs +++ b/ethereum-client/src/lib.rs @@ -242,12 +242,13 @@ pub fn get_block( } } + // note these error messages are imperfect as they don't show the broken data + // but also should never happen and not worth fixing for now Ok(EthereumBlock { - hash: parse_word(block_obj.hash).ok_or_else(|| parse_error(&get_logs_response_str[..]))?, + hash: parse_word(block_obj.hash).ok_or_else(|| parse_error("bad hash"))?, parent_hash: parse_word(block_obj.parentHash) - .ok_or_else(|| parse_error(&get_logs_response_str[..]))?, - number: parse_u64(block_obj.number) - .ok_or_else(|| parse_error(&get_logs_response_str[..]))?, + .ok_or_else(|| parse_error("bad parent hash"))?, + number: parse_u64(block_obj.number).ok_or_else(|| parse_error("bad block number"))?, events, }) } diff --git a/pallets/cash/fuzz/Cargo.lock b/pallets/cash/fuzz/Cargo.lock index a20847d2e..e7e9112b2 100644 --- a/pallets/cash/fuzz/Cargo.lock +++ b/pallets/cash/fuzz/Cargo.lock @@ -3717,6 +3717,7 @@ version = "0.1.0" dependencies = [ "hex", "logos", + "our-std", ] [[package]] diff --git a/pallets/cash/src/chains.rs b/pallets/cash/src/chains.rs index 05b68d92c..bbefd307d 100644 --- a/pallets/cash/src/chains.rs +++ b/pallets/cash/src/chains.rs @@ -383,6 +383,14 @@ impl ChainBlock { .map(move |e| ChainBlockEvent::Eth(block.number, e.clone())), } } + + pub fn concat(self, chain_blocks: ChainBlocks) -> Result { + match (self, chain_blocks) { + (ChainBlock::Eth(block), ChainBlocks::Eth(blocks)) => { + Ok(ChainBlocks::Eth([vec![block], blocks].concat())) + } + } + } } /// Type for describing a set of blocks coming from an underlying chain. diff --git a/pallets/cash/src/internal/events.rs b/pallets/cash/src/internal/events.rs index ddf398a8a..19ef602fa 100644 --- a/pallets/cash/src/internal/events.rs +++ b/pallets/cash/src/internal/events.rs @@ -102,29 +102,41 @@ pub fn track_chain_events_on(chain_id: ChainId) -> Result<(), Reason> let starport = get_starport::(chain_id)?; let me = get_current_validator::()?; let last_block = get_last_block::(chain_id)?; - let true_block = fetch_chain_block(chain_id, last_block.number(), starport)?; - if last_block.hash() == true_block.hash() { + let next_block_number = last_block + .number() + .checked_add(1) + .ok_or(MathError::Overflow)?; + let next_block = fetch_chain_block(chain_id, next_block_number, starport)?; + if last_block.hash() == next_block.parent_hash() { debug!( - "Worker sees the same fork: true={:?} last={:?}", - true_block, last_block + "Worker sees the same fork: next={:?} last={:?}", + next_block, last_block ); let pending_blocks = PendingChainBlocks::get(chain_id); let event_queue = get_event_queue::(chain_id)?; let slack = queue_slack(&event_queue) as u64; - let blocks = fetch_chain_blocks( - chain_id, - last_block.number() + 1, - last_block.number() + 1 + slack, - starport, - )? - .filter_already_supported(&me.substrate_id, pending_blocks); + let blocks = next_block + .concat(fetch_chain_blocks( + chain_id, + next_block_number + .checked_add(1) + .ok_or(MathError::Overflow)?, + next_block_number + .checked_add(1) + .ok_or(MathError::Overflow)? + .checked_add(slack) + .ok_or(MathError::Overflow)?, + starport, + )?)? + .filter_already_supported(&me.substrate_id, pending_blocks); memorize_chain_blocks::(&blocks)?; submit_chain_blocks::(&blocks) } else { debug!( - "Worker sees a different fork: true={:?} last={:?}", - true_block, last_block + "Worker sees a different fork: next={:?} last={:?}", + next_block, last_block ); + let true_block = fetch_chain_block(chain_id, last_block.number(), starport)?; let pending_reorgs = PendingChainReorgs::get(chain_id); let reorg = formulate_reorg::(chain_id, &last_block, &true_block)?; if !reorg.is_already_signed(&me.substrate_id, pending_reorgs) { @@ -522,7 +534,19 @@ mod tests { let last_block = old_chain.last().unwrap().clone(); let true_block = new_chain.last().unwrap().clone(); - let mut fetched_blocks = new_chain[0..9].iter().rev().cloned().collect::>(); + let mut fetched_blocks = vec![EthereumBlock { + hash: [10u8; 32], + parent_hash: true_block.hash, + number: 10, + events: vec![], + }]; + fetched_blocks.extend(new_chain[0..9].iter().rev().cloned().collect::>()); + fetched_blocks.push(EthereumBlock { + hash: [10u8; 32], + parent_hash: last_block.hash, + number: 10, + events: vec![], + }); fetched_blocks.extend(old_chain[1..10].iter().rev().cloned()); let calls = gen_mock_calls(&fetched_blocks, ETH_STARPORT_ADDR); let (mut t, _, _) = new_test_ext_with_http_calls(calls); diff --git a/pallets/cash/src/tests/mod.rs b/pallets/cash/src/tests/mod.rs index eef569700..e0baec8c1 100644 --- a/pallets/cash/src/tests/mod.rs +++ b/pallets/cash/src/tests/mod.rs @@ -215,6 +215,7 @@ pub fn gen_mock_calls( let mut calls = vec![]; for block in blocks { let block_str = encode_block_hex(block.number); + let block_hash_str = format!("0x{}", hex::encode(block.hash)); let get_block_params: Vec = vec![block_str.clone().into(), false.into()]; @@ -245,8 +246,7 @@ pub fn gen_mock_calls( let get_logs_params = vec![serde_json::json!({ "address": format!("0x{}", ::hex::encode(&starport_address[..])), - "fromBlock": &block_str, - "toBlock": &block_str, + "blockHash": block_hash_str, })]; let get_logs_data = serde_json::json!({ diff --git a/pallets/cash/src/tests/testdata.rs b/pallets/cash/src/tests/testdata.rs index 6befb33d3..c497193f0 100644 --- a/pallets/cash/src/tests/testdata.rs +++ b/pallets/cash/src/tests/testdata.rs @@ -5,7 +5,7 @@ pub mod json_responses { "result": null }"#; - pub const GET_BLOCK_BY_NUMBER_1: &[u8] = br#"{ + pub const GET_BLOCK_BY_NUMBER_2: &[u8] = br#"{ "jsonrpc":"2.0", "id":1, "result":{ @@ -13,13 +13,13 @@ pub mod json_responses { "extraData":"0x65746865726d696e652d657531", "gasLimit":"0x7a121d", "gasUsed":"0x781503", - "hash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", + "hash":"0x50314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", "logsBloom":"0x044410ea904e1020440110008000902200168801c81010301489212010002008080b0010004001b006040222c42004b001200408400500901889c908212040401020008d300010100198d10800100080027900254120000000530141030808140c299400162c0000d200204080008838240009002c020010400010101000481660200420a884b8020282204a00141ce10805004810800190180114180001b0001b1000020ac8040007000320b0480004018240891882a20080010281002c00000010102e0184210003010100438004202003080401000806204010000a42200104110100201200008081005001104002410140114a002010808c00200894c0c0", "miner":"0xea674fdde714fd979de3edf0f56aa9716b898ec8", "mixHash":"0xd733e12126a2155f0278c3987777eaca558a274b42d0396306dffb8fa6d21e76", "nonce":"0x56a66f3802150748", - "number":"0x1", - "parentHash":"0x062e77dced431eb671a56839f96da912f68d841024665748d38cd3d6795961ea", + "number":"0x2", + "parentHash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", "receiptsRoot":"0x19ad317358916207491d4b64340153b924f4dda88fa8ef5dcb49090f234c00e7", "sha3Uncles":"0xd21bed33f01dac18a3ee5538d1607ff2709d742eb4e13877cf66dcbed6c980f2", "size":"0x5f50", @@ -30,7 +30,7 @@ pub mod json_responses { "uncles":["0x5e7dde2e3811b5881a062c8b2ff7fd14687d79745e2384965d73a9df3fb0b4a8"]} }"#; - pub const GET_BLOCK_BY_NUMBER_2: &[u8] = br#"{ + pub const GET_BLOCK_BY_NUMBER_3: &[u8] = br#"{ "jsonrpc":"2.0", "id":1, "result":{ @@ -38,12 +38,12 @@ pub mod json_responses { "extraData":"0x65746865726d696e652d657531", "gasLimit":"0x7a121d", "gasUsed":"0x781503", - "hash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", + "hash":"0x72314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", "logsBloom":"0x044410ea904e1020440110008000902200168801c81010301489212010002008080b0010004001b006040222c42004b001200408400500901889c908212040401020008d300010100198d10800100080027900254120000000530141030808140c299400162c0000d200204080008838240009002c020010400010101000481660200420a884b8020282204a00141ce10805004810800190180114180001b0001b1000020ac8040007000320b0480004018240891882a20080010281002c00000010102e0184210003010100438004202003080401000806204010000a42200104110100201200008081005001104002410140114a002010808c00200894c0c0", "miner":"0xea674fdde714fd979de3edf0f56aa9716b898ec8", "mixHash":"0xd733e12126a2155f0278c3987777eaca558a274b42d0396306dffb8fa6d21e76", "nonce":"0x56a66f3802150748", - "number":"0x1", + "number":"0x3", "parentHash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", "receiptsRoot":"0x19ad317358916207491d4b64340153b924f4dda88fa8ef5dcb49090f234c00e7", "sha3Uncles":"0xd21bed33f01dac18a3ee5538d1607ff2709d742eb4e13877cf66dcbed6c980f2", @@ -55,20 +55,20 @@ pub mod json_responses { "uncles":["0x5e7dde2e3811b5881a062c8b2ff7fd14687d79745e2384965d73a9df3fb0b4a8"]} }"#; - pub const GET_LOGS_1: &[u8] = br#"{ + pub const GET_LOGS_2: &[u8] = br#"{ "jsonrpc":"2.0", "id":1, "result": [] }"#; - pub const GET_LOGS_2: &[u8] = br#"{ + pub const GET_LOGS_3: &[u8] = br#"{ "jsonrpc":"2.0", "id":1, "result": [ { "address":"0xbbde1662bc3ed16aa8c618c9833c801f3543b587", - "blockHash":"0xc1c0eb37b56923ad9e20fdb31ca882988d5217f7ca24b6297ca6ed700811cf23", - "blockNumber":"0x3adf2f", + "blockHash":"0x72314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", + "blockNumber":"0x3", "data":"0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000034554480000000000000000000000000000000000000000000000000000000000", "logIndex":"0x0", "removed":false, @@ -83,8 +83,8 @@ pub mod json_responses { }, { "address":"0xbbde1662bc3ed16aa8c618c9833c801f3543b587", - "blockHash":"0xa5c8024e699a5c30eb965e47b5157c06c76f3b726bff377a0a5333a561f25648", - "blockNumber":"0x3c02e1", + "blockHash":"0x72314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", + "blockNumber":"0x3", "data":"0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000034554480000000000000000000000000000000000000000000000000000000000", "logIndex":"0x1", "removed":false, @@ -99,8 +99,8 @@ pub mod json_responses { }, { "address":"0xbbde1662bc3ed16aa8c618c9833c801f3543b587", - "blockHash":"0xa4a96e957718e3a30b77a667f93978d8f438bdcd56ff03545f08c833d9a26687", - "blockNumber":"0x3c030b", + "blockHash":"0x72314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a", + "blockNumber":"0x3", "data":"0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000de0b6b3a764000000000000000000000000000000000000000000000000000000000000000000034554480000000000000000000000000000000000000000000000000000000000", "logIndex":"0xe", "removed":false, diff --git a/pallets/cash/src/tests/worker.rs b/pallets/cash/src/tests/worker.rs index d0b7b746b..8108fd25d 100644 --- a/pallets/cash/src/tests/worker.rs +++ b/pallets/cash/src/tests/worker.rs @@ -10,8 +10,8 @@ fn test_offchain_worker() { method: "POST".into(), uri: "https://ropsten-eth.compound.finance".to_string(), headers: vec![("Content-Type".to_owned(), "application/json".to_owned())], - body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x1",false],"id":1}"#.to_vec(), - response: Some(tests::testdata::json_responses::GET_BLOCK_BY_NUMBER_1.to_vec()), + body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x2",false],"id":1}"#.to_vec(), + response: Some(tests::testdata::json_responses::GET_BLOCK_BY_NUMBER_2.to_vec()), sent: true, ..Default::default() }, @@ -19,8 +19,8 @@ fn test_offchain_worker() { method: "POST".into(), uri: "https://ropsten-eth.compound.finance".to_string(), headers: vec![("Content-Type".to_owned(), "application/json".to_owned())], - body: br#"{"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":"0x7777777777777777777777777777777777777777","blockHash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a"}],"id":1}"#.to_vec(), - response: Some(testdata::json_responses::GET_LOGS_1.to_vec()), + body: br#"{"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":"0x7777777777777777777777777777777777777777","blockHash":"0x50314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a"}],"id":1}"#.to_vec(), + response: Some(testdata::json_responses::GET_LOGS_2.to_vec()), sent: true, ..Default::default() }, @@ -28,8 +28,8 @@ fn test_offchain_worker() { method: "POST".into(), uri: "https://ropsten-eth.compound.finance".to_string(), headers: vec![("Content-Type".to_owned(), "application/json".to_owned())], - body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x2",false],"id":1}"#.to_vec(), - response: Some(tests::testdata::json_responses::GET_BLOCK_BY_NUMBER_2.to_vec()), + body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x3",false],"id":1}"#.to_vec(), + response: Some(tests::testdata::json_responses::GET_BLOCK_BY_NUMBER_3.to_vec()), sent: true, ..Default::default() }, @@ -37,8 +37,8 @@ fn test_offchain_worker() { method: "POST".into(), uri: "https://ropsten-eth.compound.finance".to_string(), headers: vec![("Content-Type".to_owned(), "application/json".to_owned())], - body: br#"{"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":"0x7777777777777777777777777777777777777777","blockHash":"0x61314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a"}],"id":1}"#.to_vec(), - response: Some(testdata::json_responses::GET_LOGS_2.to_vec()), + body: br#"{"jsonrpc":"2.0","method":"eth_getLogs","params":[{"address":"0x7777777777777777777777777777777777777777","blockHash":"0x72314c1c6837e15e60c5b6732f092118dd25e3ec681f5e089b3a9ad2374e5a8a"}],"id":1}"#.to_vec(), + response: Some(testdata::json_responses::GET_LOGS_3.to_vec()), sent: true, ..Default::default() }, @@ -46,7 +46,7 @@ fn test_offchain_worker() { method: "POST".into(), uri: "https://ropsten-eth.compound.finance".to_string(), headers: vec![("Content-Type".to_owned(), "application/json".to_owned())], - body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x3",false],"id":1}"#.to_vec(), + body: br#"{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x4",false],"id":1}"#.to_vec(), response: Some(tests::testdata::json_responses::NO_RESULT.to_vec()), sent: true, ..Default::default() @@ -71,10 +71,10 @@ fn test_offchain_worker() { if let mock::Call::Cash(crate::Call::receive_chain_blocks(blocks, _signature)) = ex1.call { assert_eq!(blocks.chain_id(), ChainId::Eth); - assert_eq!(blocks.len(), 1); + assert_eq!(blocks.len(), 2); match blocks { ChainBlocks::Eth(blocks) => { - let block = &blocks[0]; + let block = &blocks[1]; assert_eq!(block.events.len(), 3); let event = &block.events[1]; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 4c8a4c503..d134eefec 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -116,7 +116,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("gateway"), impl_name: create_runtime_str!("gateway"), authoring_version: 1, - spec_version: 15, + spec_version: 16, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1,