From 4375120bb3f906aea7c2e5f8ef1235c5a6ccf4bd Mon Sep 17 00:00:00 2001 From: Seulgi Kim Date: Fri, 17 May 2019 14:29:37 +0900 Subject: [PATCH] Use the child of the best header to verify transactions In the previous implementation, the transaction expired after the parent block is added in the pool and rejected during execution. This patch makes mempool reject the transaction. So the expriation e2e tests are adjusted. --- core/src/block.rs | 10 +++------- core/src/header.rs | 11 +++++++++++ core/src/miner/miner.rs | 8 ++++---- test/src/e2e.long/expiration.test.ts | 4 ++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/core/src/block.rs b/core/src/block.rs index 789082227b..f6ab94bedf 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -93,9 +93,9 @@ pub struct ExecutedBlock { } impl ExecutedBlock { - fn new(state: TopLevelState) -> ExecutedBlock { + fn new(state: TopLevelState, parent: &Header) -> ExecutedBlock { ExecutedBlock { - header: Default::default(), + header: parent.generate_child(), state, transactions: Default::default(), invoices: Default::default(), @@ -132,17 +132,13 @@ impl<'x> OpenBlock<'x> { author: Address, extra_data: Bytes, ) -> Result { - let number = parent.number() + 1; let state = TopLevelState::from_existing(db, *parent.state_root()).map_err(StateError::from)?; let mut r = OpenBlock { - block: ExecutedBlock::new(state), + block: ExecutedBlock::new(state, parent), engine, }; - r.block.header.set_parent_hash(parent.hash()); - r.block.header.set_number(number); r.block.header.set_author(author); - r.block.header.set_timestamp_now(parent.timestamp()); r.block.header.set_extra_data(extra_data); r.block.header.note_dirty(); diff --git a/core/src/header.rs b/core/src/header.rs index 893dcf3680..8d20afc972 100644 --- a/core/src/header.rs +++ b/core/src/header.rs @@ -261,6 +261,17 @@ impl Header { pub fn rlp_blake(&self, with_seal: &Seal) -> H256 { blake256(&self.rlp(with_seal)) } + + pub fn generate_child(&self) -> Self { + let mut header = Header::default(); + + header.set_parent_hash(self.hash()); + header.set_number(self.number() + 1); + header.set_timestamp_now(self.timestamp()); + header.note_dirty(); + + header + } } impl Decodable for Header { diff --git a/core/src/miner/miner.rs b/core/src/miner/miner.rs index 4bcb12397c..902ae08d34 100644 --- a/core/src/miner/miner.rs +++ b/core/src/miner/miner.rs @@ -254,7 +254,7 @@ impl Miner { default_origin: TxOrigin, mem_pool: &mut MemPool, ) -> Vec> { - let best_block_header = client.best_block_header().decode(); + let fake_header = client.best_block_header().decode().generate_child(); let current_block_number = client.chain_info().best_block_number; let current_timestamp = client.chain_info().best_block_timestamp; let mut inserted = Vec::with_capacity(transactions.len()); @@ -274,8 +274,8 @@ impl Miner { } match self .engine - .verify_transaction_basic(&tx, &best_block_header) - .and_then(|_| self.engine.verify_transaction_unordered(tx, &best_block_header)) + .verify_transaction_basic(&tx, &fake_header) + .and_then(|_| self.engine.verify_transaction_unordered(tx, &fake_header)) { Err(e) => { cdebug!(MINER, "Rejected transaction {:?} with invalid signature: {:?}", hash, e); @@ -283,7 +283,7 @@ impl Miner { } Ok(tx) => { // This check goes here because verify_transaction takes SignedTransaction parameter - self.engine.machine().verify_transaction(&tx, &best_block_header, client, false)?; + self.engine.machine().verify_transaction(&tx, &fake_header, client, false)?; let origin = self .accounts diff --git a/test/src/e2e.long/expiration.test.ts b/test/src/e2e.long/expiration.test.ts index d5cc18ead9..1359322c96 100644 --- a/test/src/e2e.long/expiration.test.ts +++ b/test/src/e2e.long/expiration.test.ts @@ -51,7 +51,7 @@ describe("TransferAsset expiration test", function() { // 3. Send TransferAsset transactions (which should not processed) const seq = await node.sdk.rpc.chain.getSeq(faucetAddress); - startTime = Math.round(new Date().getTime() / 1000); + startTime = Math.round(new Date().getTime() / 1000) + 5; for (let i = 0; i < numTx; i++) { const recipient = await node.createP2PKHAddress(); const tx = node.sdk.core.createTransferAssetTransaction({ @@ -95,7 +95,7 @@ describe("TransferAsset expiration test", function() { const isExpired = bestBlockTimestamp > startTime + numTx - i; expect(isResultsEmpty).to.be.equal(isExpired); } - }).timeout(10_000); + }).timeout(15_000); }); afterEach(async function() {