From 35cd14105a053d0a6aaa0c287fea74c7ebfa8990 Mon Sep 17 00:00:00 2001 From: Julian Kulesh Date: Wed, 10 Jul 2024 15:12:14 +0800 Subject: [PATCH 1/3] feat: panic on inconsistent block submission --- contract/src/lib.rs | 65 +++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/contract/src/lib.rs b/contract/src/lib.rs index a5ae4f7..9e25881 100644 --- a/contract/src/lib.rs +++ b/contract/src/lib.rs @@ -226,30 +226,24 @@ impl Contract { /// /// # Panics /// - Many cases - /// - /// # Errors - /// - No previous block recorded, so we cannot validate chain - #[handle_result] #[pause(except(roles(Role::UnrestrictedSubmitBlocks)))] - pub fn submit_block_header(&mut self, block_header: Header) -> Result<(), String> { + pub fn submit_block_header(&mut self, block_header: Header) { // Chainwork is validated inside block_header structure (other consistency checks too) let prev_blockhash = block_header.prev_blockhash.to_string(); - let prev_block_header = if let Some(header) = self.headers_pool.get(&prev_blockhash) { - header.clone() - } else { - // We do not have a previous block in the headers_pool, there is a high probability - //it means we are starting to receive a new fork, - // so what we do now is we are returning the error code - // to ask the relay to deploy the previous block. - // - // Offchain relay now, should submit blocks one by one in decreasing height order - // 80 -> 79 -> 78 -> ... - // And do it until we can accept the block. - // It means we found an initial fork position. - // We are starting to gather new fork from this initial position. - return Err(String::from("1")); - }; + // We do not have a previous block in the headers_pool, there is a high probability + //it means we are starting to receive a new fork, + // so what we do now is we are returning the error code + // to ask the relay to deploy the previous block. + // + // Offchain relay now, should submit blocks one by one in decreasing height order + // 80 -> 79 -> 78 -> ... + // And do it until we can accept the block. + // It means we found an initial fork position. + // We are starting to gather new fork from this initial position. + let prev_block_header = self.headers_pool + .get(&prev_blockhash) + .unwrap_or_else(|| env::panic_str("PrevBlockNotFound")); if self.enable_check { block_header @@ -296,8 +290,6 @@ impl Contract { self.reorg_chain(¤t_blockhash); } } - - Ok(()) } /// The most expensive operation which reorganizes the chain, based on fork weight @@ -607,7 +599,7 @@ mod tests { let header = fork_block_header_example(); let mut contract = Contract::new(genesis_block_header(), 0, true, 3); - contract.submit_block_header(header).unwrap(); + contract.submit_block_header(header); let received_header = contract.get_last_block_header(); @@ -630,7 +622,7 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - contract.submit_block_header(header).unwrap(); + contract.submit_block_header(header); let received_header = contract.get_last_block_header(); @@ -653,11 +645,10 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - contract.submit_block_header(header).unwrap(); + contract.submit_block_header(header); contract - .submit_block_header(fork_block_header_example()) - .unwrap(); + .submit_block_header(fork_block_header_example()); let received_header = contract.get_last_block_header(); @@ -680,8 +671,7 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); contract - .submit_block_header(block_header_example()) - .unwrap(); + .submit_block_header(block_header_example()); assert_eq!( contract.get_blockhash_by_height(0).unwrap(), @@ -698,8 +688,7 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); contract - .submit_block_header(block_header_example()) - .unwrap(); + .submit_block_header(block_header_example()); assert_eq!( contract @@ -720,15 +709,12 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); contract - .submit_block_header(block_header_example()) - .unwrap(); + .submit_block_header(block_header_example()); contract - .submit_block_header(fork_block_header_example()) - .unwrap(); + .submit_block_header(fork_block_header_example()); contract - .submit_block_header(fork_block_header_example_2()) - .unwrap(); + .submit_block_header(fork_block_header_example_2()); let received_header = contract.get_last_block_header(); @@ -746,11 +732,10 @@ mod tests { } #[test] + #[should_panic] fn test_getting_an_error_if_submitting_unattached_block() { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - let result = contract.submit_block_header(fork_block_header_example_2()); - assert!(result.is_err()); - assert!(result.is_err_and(|value| value == *"1")); + contract.submit_block_header(fork_block_header_example_2()); } } From 53f4ad1bc9afcda4d6de01105a35195a30c46806 Mon Sep 17 00:00:00 2001 From: Julian Kulesh Date: Wed, 10 Jul 2024 15:13:38 +0800 Subject: [PATCH 2/3] chore: fmt --- contract/src/lib.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/contract/src/lib.rs b/contract/src/lib.rs index 9e25881..1c28579 100644 --- a/contract/src/lib.rs +++ b/contract/src/lib.rs @@ -241,7 +241,8 @@ impl Contract { // And do it until we can accept the block. // It means we found an initial fork position. // We are starting to gather new fork from this initial position. - let prev_block_header = self.headers_pool + let prev_block_header = self + .headers_pool .get(&prev_blockhash) .unwrap_or_else(|| env::panic_str("PrevBlockNotFound")); @@ -647,8 +648,7 @@ mod tests { contract.submit_block_header(header); - contract - .submit_block_header(fork_block_header_example()); + contract.submit_block_header(fork_block_header_example()); let received_header = contract.get_last_block_header(); @@ -670,8 +670,7 @@ mod tests { fn test_getting_block_by_height() { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - contract - .submit_block_header(block_header_example()); + contract.submit_block_header(block_header_example()); assert_eq!( contract.get_blockhash_by_height(0).unwrap(), @@ -687,8 +686,7 @@ mod tests { fn test_getting_height_by_block() { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - contract - .submit_block_header(block_header_example()); + contract.submit_block_header(block_header_example()); assert_eq!( contract @@ -708,13 +706,10 @@ mod tests { fn test_submitting_existing_fork_block_header_and_promote_fork() { let mut contract = Contract::new(genesis_block_header(), 0, false, 3); - contract - .submit_block_header(block_header_example()); + contract.submit_block_header(block_header_example()); - contract - .submit_block_header(fork_block_header_example()); - contract - .submit_block_header(fork_block_header_example_2()); + contract.submit_block_header(fork_block_header_example()); + contract.submit_block_header(fork_block_header_example_2()); let received_header = contract.get_last_block_header(); From f35a581cb9925ea79beb830db3099e69df51a4fe Mon Sep 17 00:00:00 2001 From: Julian Kulesh Date: Wed, 10 Jul 2024 15:21:05 +0800 Subject: [PATCH 3/3] chore: other lints --- contract/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contract/src/lib.rs b/contract/src/lib.rs index 1c28579..1e77a13 100644 --- a/contract/src/lib.rs +++ b/contract/src/lib.rs @@ -592,7 +592,7 @@ mod tests { let mut contract = Contract::new(genesis_block_header(), 0, true, 3); - let _ = contract.submit_block_header(header); + contract.submit_block_header(header); } #[test] @@ -727,7 +727,7 @@ mod tests { } #[test] - #[should_panic] + #[should_panic(expected = "PrevBlockNotFound")] fn test_getting_an_error_if_submitting_unattached_block() { let mut contract = Contract::new(genesis_block_header(), 0, false, 3);