Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tests transaction receipts for reverted evm execution #184

Merged
merged 9 commits into from
Nov 3, 2020
Merged

Tests transaction receipts for reverted evm execution #184

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0d125a2
lots of debugging lines
JoshOrndorff Oct 29, 2020
e8a253b
Add integration test to demonstrate the issue
JoshOrndorff Oct 30, 2020
f7c56f2
Revert "lots of debugging lines"
JoshOrndorff Oct 30, 2020
1ba3103
One possible fix for the bug
JoshOrndorff Oct 30, 2020
872f8f9
Fix constants in tests
JoshOrndorff Oct 30, 2020
69fa678
Fix warning
JoshOrndorff Oct 30, 2020
1b9eaaf
Fix runtime test
JoshOrndorff Nov 2, 2020
c376d51
revert my attempt to fix
JoshOrndorff Nov 3, 2020
36d96c9
master.into() // get @sorpass's fix from master
JoshOrndorff Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions frame/ethereum/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use sp_runtime::{
};
use evm::{ExitError, ExitRevert, ExitFatal, ExitReason};
use sp_evm::CallOrCreateInfo;
use pallet_evm::{Runner, ExecutionInfo};
use pallet_evm::Runner;
use sha3::{Digest, Keccak256};
use codec::Encode;
use frontier_consensus_primitives::{FRONTIER_ENGINE_ID, ConsensusLog};
Expand Down Expand Up @@ -176,7 +176,7 @@ decl_module! {
transaction.action,
)?;

let (status, used_gas) = match info {
let (status, used_gas, exit_reason) = match info {
CallOrCreateInfo::Call(info) => {
(TransactionStatus {
transaction_hash,
Expand All @@ -186,7 +186,7 @@ decl_module! {
contract_address: None,
logs: info.logs,
logs_bloom: Bloom::default(), // TODO: feed in bloom.
}, info.used_gas)
}, info.used_gas, info.exit_reason)
},
CallOrCreateInfo::Create(info) => {
(TransactionStatus {
Expand All @@ -197,7 +197,7 @@ decl_module! {
contract_address: Some(info.value),
logs: info.logs,
logs_bloom: Bloom::default(), // TODO: feed in bloom.
}, info.used_gas)
}, info.used_gas, info.exit_reason)
},
};

Expand All @@ -210,6 +210,8 @@ decl_module! {

Pending::append((transaction, status, receipt));

Self::handle_exec(exit_reason)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually still cannot return error here. It must succeed, because at this stage the runtime state has already changed.


Self::deposit_event(Event::Executed(source, transaction_hash));
}

Expand Down Expand Up @@ -385,7 +387,7 @@ impl<T: Trait> Module<T> {
) -> Result<(Option<H160>, CallOrCreateInfo), DispatchError> {
match action {
ethereum::TransactionAction::Call(target) => {
Ok((Some(target), CallOrCreateInfo::Call(Self::handle_exec(
Ok((Some(target), CallOrCreateInfo::Call(
T::Runner::call(
from,
target,
Expand All @@ -395,10 +397,10 @@ impl<T: Trait> Module<T> {
gas_price,
nonce,
).map_err(Into::into)?
)?)))
)))
},
ethereum::TransactionAction::Create => {
Ok((None, CallOrCreateInfo::Create(Self::handle_exec(
Ok((None, CallOrCreateInfo::Create(
T::Runner::create(
from,
input.clone(),
Expand All @@ -407,14 +409,14 @@ impl<T: Trait> Module<T> {
gas_price,
nonce,
).map_err(Into::into)?
)?)))
)))
},
}
}

fn handle_exec<R>(res: ExecutionInfo<R>) -> Result<ExecutionInfo<R>, Error<T>> {
match res.exit_reason {
ExitReason::Succeed(_s) => Ok(res),
fn handle_exec(reason: ExitReason) -> Result<(), Error<T>> {
match reason {
ExitReason::Succeed(_s) => Ok(()),
ExitReason::Error(e) => Err(Self::parse_exit_error(e)),
ExitReason::Revert(e) => {
match e {
Expand Down
11 changes: 8 additions & 3 deletions frame/ethereum/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,16 +295,21 @@ fn call_should_handle_errors() {
}

// calling bar will revert
let err = Ethereum::execute(
let (_, info) = Ethereum::execute(
alice.address,
bar,
U256::zero(),
U256::from(1048576),
Some(U256::from(1)),
Some(U256::from(2)),
TransactionAction::Call(H160::from_slice(&contract_address))
).err().unwrap();
).ok().unwrap();

let exit_reason : ExitReason = match info {
CallOrCreateInfo::Create(_) => panic!("Should have gotten a Call variant"),
CallOrCreateInfo::Call(info) => info.exit_reason,
};

assert_eq!(err, Error::<Test>::Reverted.into());
assert_eq!(exit_reason, ExitReason::Revert(ExitRevert::Reverted));
});
}
118 changes: 118 additions & 0 deletions ts-tests/tests/test-revert-receipt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { expect } from "chai";

import { createAndFinalizeBlock, customRequest, describeWithFrontier } from "./util";

describeWithFrontier("Frontier RPC (Constructor Revert)", `simple-specs.json`, (context) => {
const GENESIS_ACCOUNT = "0x6be02d1d3665660d22ff9624b7be0551ee1ac91b";
const GENESIS_ACCOUNT_PRIVATE_KEY =
"0x99B3C12287537E38C90A9219D4CB074A89A16E9CDB20BF85728EBD97C343E342";

// ```
// pragma solidity >=0.4.22 <0.7.0;
//
// contract WillFail {
// constructor() public {
// require(false);
// }
// }
// ```
const FAIL_BYTECODE = '6080604052348015600f57600080fd5b506000601a57600080fd5b603f8060276000396000f3fe6080604052600080fdfea26469706673582212209f2bb2a4cf155a0e7b26bd34bb01e9b645a92c82e55c5dbdb4b37f8c326edbee64736f6c63430006060033';
const GOOD_BYTECODE = '6080604052348015600f57600080fd5b506001601a57600080fd5b603f8060276000396000f3fe6080604052600080fdfea2646970667358221220c70bc8b03cdfdf57b5f6c4131b836f9c2c4df01b8202f530555333f2a00e4b8364736f6c63430006060033';

it("should provide a tx receipt after successful deployment", async function () {
this.timeout(15000);
const GOOD_TX_HASH = '0xae813c533aac0719fbca4db6e3bb05cfb5859bdeaaa7dc5c9dbd24083301be8d';

const tx = await context.web3.eth.accounts.signTransaction(
{
from: GENESIS_ACCOUNT,
data: GOOD_BYTECODE,
value: "0x00",
gasPrice: "0x01",
gas: "0x100000",
},
GENESIS_ACCOUNT_PRIVATE_KEY
);

expect(
await customRequest(context.web3, "eth_sendRawTransaction", [tx.rawTransaction])
).to.deep.equal({
id: 1,
jsonrpc: "2.0",
result: GOOD_TX_HASH,
});

// Verify the receipt exists after the block is created
//TODO Actually, why doesn't this receipt have a status in it?
// I guess because the RPC handler in eth.rs sets `status_code: None`??
await createAndFinalizeBlock(context.web3);
expect(
await customRequest(context.web3, "eth_getTransactionReceipt", [GOOD_TX_HASH])
).to.deep.equal({
id: 1,
jsonrpc: "2.0",
result: {
"blockHash": "0xf543706adc989641d066cf0831374a8b33aad1dd91dc1f18ee75786d50d478e2",
"blockNumber": "0x1",
"contractAddress": "0xc2bf5f29a4384b1ab0c063e1c666f02121b6084a",
"cumulativeGasUsed": "0x1069f",
"from": "0x6be02d1d3665660d22ff9624b7be0551ee1ac91b",
"gasUsed": "0x1069f",
"logs": [],
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"root": "0x0000000000000000000000000000000000000000000000000000000000000000",
"to": null,
"transactionHash": GOOD_TX_HASH,
"transactionIndex": "0x0",
}
});
});

it("should provide a tx receipt after failed deployment", async function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, those tests should be split in 2:

  • should provide a tx receipt after failed deployment (verify the result is valid)
  • should store the tx after reverted (verify querying the receipt returns the valid data)

this.timeout(15000);
// Transaction hash depends on which nonce we're using
//const FAIL_TX_HASH = '0x89a956c4631822f407b3af11f9251796c276655860c892919f848699ed570a8d'; //nonce 1
const FAIL_TX_HASH = '0x640df9deb183d565addc45bdc8f95b30c7c03ce7e69df49456be9929352e4347'; //nonce 2

const tx = await context.web3.eth.accounts.signTransaction(
{
from: GENESIS_ACCOUNT,
data: FAIL_BYTECODE,
value: "0x00",
gasPrice: "0x01",
gas: "0x100000",
},
GENESIS_ACCOUNT_PRIVATE_KEY
);

expect(
await customRequest(context.web3, "eth_sendRawTransaction", [tx.rawTransaction])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use context.web3.eth.sendRawTransaction ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the style of the existing tests.

).to.deep.equal({
id: 1,
jsonrpc: "2.0",
result: FAIL_TX_HASH,
});

await createAndFinalizeBlock(context.web3);
expect(
await customRequest(context.web3, "eth_getTransactionReceipt", [FAIL_TX_HASH])
).to.deep.equal({
id: 1,
jsonrpc: "2.0",
result: {
"blockHash": "0x74233c510529ffb8a740223748ed0df1b30b86e7c31039f7e645138332a0dfb5",
"blockNumber": "0x2",
"contractAddress": "0x5c4242beb94de30b922f57241f1d02f36e906915",
"cumulativeGasUsed": "0xd548",
"from": "0x6be02d1d3665660d22ff9624b7be0551ee1ac91b",
"gasUsed": "0xd548",
"logs": [],
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"root": "0x0000000000000000000000000000000000000000000000000000000000000000",
"to": null,
"transactionHash": FAIL_TX_HASH,
"transactionIndex": "0x0",
}
});
});
});