Framework for improved RPC unit tests #1141
Conversation
|
|
||
| let res_after_pending = r#"{"jsonrpc":"2.0","result":"0x01","id":18}"#; | ||
|
|
||
| assert_eq!(&tester.handler.handle_request(&req_after_pending).unwrap(), res_after_pending); |
There was a problem hiding this comment.
Right now, this really only tests the TestMinerService's ability to update transactions.
It would probably be useful to integrate this with the actual Miner, although I worry that this could lead to race conditions in tests like these, where we want to send the "pending" RPC request after the miner has placed the transaction in a pending block, but before the block is sealed.
There was a problem hiding this comment.
I spoke to @tomusdrw about this in person, apparently this is not an issue.
rphmeier
added
A0-pleasereview 🤓
| Vec<Result<TransactionImportResult, Error>> | ||
| where T: Fn(&Address) -> AccountDetails { | ||
| // lets assume that all txs are valid | ||
| self.imported_transactions.lock().unwrap().extend_from_slice(&transactions); | ||
|
|
||
| for transaction in &transactions { |
There was a problem hiding this comment.
transactions.filter_map(|t| t.sender()) ?
| for transaction in &transactions { | ||
| if let Ok(ref sender) = transaction.sender() { | ||
| let nonce = self.last_nonce(sender).unwrap_or(fetch_account(sender).nonce); | ||
| self.last_nonces.write().unwrap().insert(sender.clone(), nonce + U256::from(1)); |
There was a problem hiding this comment.
let nonce = self.last_nonce(sender).map(|n| n + U256::from(1)).unwrap_or(fetch_account(sender).nonce);
In state next valid nonce is stored, last_nonce returns last used nonce afair - so you should increment only in the first case.
Same thing in L144
There was a problem hiding this comment.
(resolved in person, not an issue with TestMinerService)
also adds an EthTester struct for more test flexibility.
rphmeier
changed the title
rphmeier
added
A0-pleasereview 🤓
This lays the framework for RPC unit tests which use parity's actual
BlockChainClientandMiner. TheMinerhas not been integrated yet, since the changes were growing pretty large.