Skip to content

Commit

Permalink
Merge pull request #29 from etclabscore/feat/rustfmt
Browse files Browse the repository at this point in the history
feat: common code formatting rules with rustfmt
  • Loading branch information
Mike Lubinets committed Mar 26, 2019
2 parents ad38978 + 22fa140 commit 6a60714
Show file tree
Hide file tree
Showing 54 changed files with 2,534 additions and 1,710 deletions.
5 changes: 5 additions & 0 deletions .travis.yml
@@ -1,4 +1,5 @@
language: rust
cache: cargo

rust:
- stable
Expand All @@ -9,9 +10,13 @@ os:
- linux
- osx

before_script:
- if [ "$TRAVIS_RUST_VERSION" == "stable" ]; then rustup component add rustfmt; fi

script:
- cargo build --release --all --verbose
- cargo test --release --all --verbose
- if [ "$TRAVIS_RUST_VERSION" == "stable" ]; then cargo fmt -- --check; fi
- if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then cargo build --no-default-features --features rust-secp256k1; fi

matrix:
Expand Down
88 changes: 88 additions & 0 deletions FORMATTING.md
@@ -0,0 +1,88 @@
### Formatting
This repository incorporates a certain formatting style derived from the [rustfmt defaults](https://github.com/rust-lang/rustfmt/blob/master/Configurations.md) with a few changes, which are:
- Maximum line width is 120 characters
- Newline separator style is always Unix (`\n` as opposed to Windows `\n\r`)
- `try!` macro would automatically convert into a `?` question-mark operator expression

Any other configuration is the **stable** `rustfmt` default.

#### Running the formatter
`evm-rs` is a complex project with a lot of sub-crates, so `cargo fmt` should be invoked with an `--all` argument.
```bash
cargo fmt --all
```
More info can be found at the [project page](https://github.com/rust-lang/rustfmt)

#### Manual formatting overrides
The formatting constraint is checked within Travis CI and Jenkins continuous integration pipelines, hence any pull-request should be formatted before it may be merged.

Though, as most of the tooling generally is, `rustfmt` isn’t perfect and sometimes one would requite to force the manual formatting, as, for instance, it’s required for the `Patch` interface in `evm-rs`
##### Original code
```rust
pub struct EmbeddedAccountPatch;
impl AccountPatch for EmbeddedAccountPatch {
fn initial_nonce() -> U256 { U256::zero() }
fn initial_create_nonce() -> U256 { Self::initial_nonce() }
fn empty_considered_exists() -> bool { true }
}
```
##### `Rustfmt` formatted code
```rust
pub struct EmbeddedAccountPatch;
impl AccountPatch for EmbeddedAccountPatch {
fn initial_nonce() -> U256 {
U256::zero()
}
fn initial_create_nonce() -> U256 {
Self::initial_nonce()
}
fn empty_considered_exists() -> bool {
true
}
}
```
Depending on the case, the readability of the code may decrease, like here (IMHO) since the expanded function body brings none value.
While this would be possible to fix with **nightly** `rustfmt`, nightly version is still too unstable, so it’s encouraged to use manual formatting overrides where it is justified.
##### Manual override
```rust
pub struct EmbeddedAccountPatch;
#[rustfmt::skip]
impl AccountPatch for EmbeddedAccountPatch {
fn initial_nonce() -> U256 { U256::zero() }
fn initial_create_nonce() -> U256 { Self::initial_nonce() }
fn empty_considered_exists() -> bool { true }
}
```
#### Automation
To ensure none commits are misformatted, it’s required to manually run rustfmt before commiting the code, though that might be irritating.
Fortunately, formatting (and formatting checks) may be automated, and here are ways to do that:
##### 1. Formatting on save
Most of the editors have a feature of pre-save hooks that can execute arbitrary commands before persisting the file contents.
* [Setup for JetBrains IDEs (Clion, Intellij Idea, …)](https://codurance.com/2017/11/26/rusting-IntelliJ/)
* [Setup for VSCode](https://github.com/editor-rs/vscode-rust/blob/master/doc/format.md)

If the editor doesn’t support the on-save hook, one could automate formatting through [cargo watch](https://github.com/passcod/cargo-watch):
```bash
cargo watch -s “cargo fmt --all”
```
##### 2. Git pre-commit hook
Create a file `.git/hooks/pre-commit` with the following contents:
```
#!/bin/sh
# Put in your Rust repository’s .git/hooks/pre-commit to ensure you never
# breaks rustfmt.
#
# WARNING: rustfmt is a fast moving target so ensure you have the version that
# all contributors have.
for FILE in `git diff --cached --name-only`; do
if [[ $FILE == *.rs ]] && ! rustfmt --check $FILE; then
echo “Commit rejected due to invalid formatting of \”$FILE\" file.”
exit 1
fi
done
```
This hook will reject ill-formatted code before the commit.

Synergy of these two automation techniques should allow one to ensure formatting correctness while not being forced to run `rustfmt` manually.


146 changes: 85 additions & 61 deletions cli/src/bin/main.rs
Expand Up @@ -2,16 +2,19 @@ mod profiler;
use self::profiler::Profiler;

use std::fs::File;
use std::ops::DerefMut;
use std::rc::Rc;
use std::str::FromStr;

use bigint::{Gas, Address, U256, M256, H256};
use bigint::{Address, Gas, H256, M256, U256};
use hexutil::read_hex;
use evm::{HeaderParams, Context, SeqTransactionVM, ValidTransaction, VM,
AccountCommitment, RequireError, TransactionAction, VMStatus, SeqContextVM};
use evm_network_classic::{MainnetFrontierPatch, MainnetHomesteadPatch, MainnetEIP150Patch, MainnetEIP160Patch};

use evm::{
AccountCommitment, Context, HeaderParams, RequireError, SeqContextVM, SeqTransactionVM, TransactionAction,
VMStatus, ValidTransaction, VM,
};
use evm_network_classic::{MainnetEIP150Patch, MainnetEIP160Patch, MainnetFrontierPatch, MainnetHomesteadPatch};
use gethrpc::{GethRPCClient, NormalGethRPCClient, RPCBlock};
use std::str::FromStr;
use std::ops::DerefMut;
use std::rc::Rc;

fn from_rpc_block(block: &RPCBlock) -> HeaderParams {
HeaderParams {
Expand All @@ -25,23 +28,24 @@ fn from_rpc_block(block: &RPCBlock) -> HeaderParams {

fn handle_step_without_rpc(vm: &mut VM) {
match vm.step() {
Ok(()) => {},
Ok(()) => {}
Err(RequireError::Account(address)) => {
vm.commit_account(AccountCommitment::Nonexist(address)).unwrap();
},
}
Err(RequireError::AccountStorage(address, index)) => {
vm.commit_account(AccountCommitment::Storage {
address,
index,
value: M256::zero(),
}).unwrap();
},
})
.unwrap();
}
Err(RequireError::AccountCode(address)) => {
vm.commit_account(AccountCommitment::Nonexist(address)).unwrap();
},
}
Err(RequireError::Blockhash(number)) => {
vm.commit_blockhash(number, H256::default()).unwrap();
},
}
}
}

Expand All @@ -50,12 +54,9 @@ fn profile_fire_without_rpc(vm: &mut VM) {
match vm.status() {
VMStatus::Running => {
let opcode = vm.peek_opcode();
flame::span_of(format!("{:?}", opcode), || {
handle_step_without_rpc(vm)
});
},
VMStatus::ExitedOk | VMStatus::ExitedErr(_) |
VMStatus::ExitedNotSupported(_) => return,
flame::span_of(format!("{:?}", opcode), || handle_step_without_rpc(vm));
}
VMStatus::ExitedOk | VMStatus::ExitedErr(_) | VMStatus::ExitedNotSupported(_) => return,
}
}
}
Expand All @@ -66,20 +67,21 @@ fn handle_fire_without_rpc(vm: &mut VM) {
Ok(()) => break,
Err(RequireError::Account(address)) => {
vm.commit_account(AccountCommitment::Nonexist(address)).unwrap();
},
}
Err(RequireError::AccountStorage(address, index)) => {
vm.commit_account(AccountCommitment::Storage {
address,
index,
value: M256::zero(),
}).unwrap();
},
})
.unwrap();
}
Err(RequireError::AccountCode(address)) => {
vm.commit_account(AccountCommitment::Nonexist(address)).unwrap();
},
}
Err(RequireError::Blockhash(number)) => {
vm.commit_blockhash(number, H256::default()).unwrap();
},
}
}
}
}
Expand All @@ -89,48 +91,54 @@ fn handle_fire_with_rpc<T: GethRPCClient>(client: &mut T, vm: &mut VM, block_num
match vm.fire() {
Ok(()) => break,
Err(RequireError::Account(address)) => {
let nonce = U256::from_str(&client.get_transaction_count(&format!("0x{:x}", address),
&block_number)).unwrap();
let balance = U256::from_str(&client.get_balance(&format!("0x{:x}", address),
&block_number)).unwrap();
let code = read_hex(&client.get_code(&format!("0x{:x}", address),
&block_number)).unwrap();
if !client.account_exist(&format!("0x{:x}", address), U256::from_str(&block_number).unwrap().as_usize()) {
let nonce =
U256::from_str(&client.get_transaction_count(&format!("0x{:x}", address), &block_number)).unwrap();
let balance = U256::from_str(&client.get_balance(&format!("0x{:x}", address), &block_number)).unwrap();
let code = read_hex(&client.get_code(&format!("0x{:x}", address), &block_number)).unwrap();
if !client.account_exist(
&format!("0x{:x}", address),
U256::from_str(&block_number).unwrap().as_usize(),
) {
vm.commit_account(AccountCommitment::Nonexist(address)).unwrap();
} else {
vm.commit_account(AccountCommitment::Full {
nonce,
address,
balance,
code: Rc::new(code),
}).unwrap();
})
.unwrap();
}
},
}
Err(RequireError::AccountStorage(address, index)) => {
let value = M256::from_str(&client.get_storage_at(&format!("0x{:x}", address),
&format!("0x{:x}", index),
&block_number)).unwrap();
vm.commit_account(AccountCommitment::Storage {
address,
index,
value,
}).unwrap();
},
let value = M256::from_str(&client.get_storage_at(
&format!("0x{:x}", address),
&format!("0x{:x}", index),
&block_number,
))
.unwrap();
vm.commit_account(AccountCommitment::Storage { address, index, value })
.unwrap();
}
Err(RequireError::AccountCode(address)) => {
let code = read_hex(&client.get_code(&format!("0x{:x}", address),
&block_number)).unwrap();
let code = read_hex(&client.get_code(&format!("0x{:x}", address), &block_number)).unwrap();
vm.commit_account(AccountCommitment::Code {
address,
code: Rc::new(code),
}).unwrap();
},
})
.unwrap();
}
Err(RequireError::Blockhash(number)) => {
let hash = H256::from_str(&client.get_block_by_number(&format!("0x{:x}", number))
.expect("block not found")
.hash
.expect("block has no hash")).unwrap();
let hash = H256::from_str(
&client
.get_block_by_number(&format!("0x{:x}", number))
.expect("block not found")
.hash
.expect("block has no hash"),
)
.unwrap();
vm.commit_blockhash(number, hash).unwrap();
},
}
}
}
}
Expand All @@ -155,12 +163,23 @@ fn main() {
(@arg CALLER: --caller +takes_value "Caller of the transaction.")
(@arg ADDRESS: --address +takes_value "Address of the transaction.")
(@arg VALUE: --value +takes_value "Value of the transaction.")
).get_matches();
)
.get_matches();

let code = read_hex(matches.value_of("CODE").unwrap()).unwrap();
let data = read_hex(matches.value_of("DATA").unwrap_or("")).unwrap();
let caller = Address::from_str(matches.value_of("CALLER").unwrap_or("0x0000000000000000000000000000000000000000")).unwrap();
let address = Address::from_str(matches.value_of("ADDRESS").unwrap_or("0x0000000000000000000000000000000000000000")).unwrap();
let caller = Address::from_str(
matches
.value_of("CALLER")
.unwrap_or("0x0000000000000000000000000000000000000000"),
)
.unwrap();
let address = Address::from_str(
matches
.value_of("ADDRESS")
.unwrap_or("0x0000000000000000000000000000000000000000"),
)
.unwrap();
let value = U256::from_str(matches.value_of("VALUE").unwrap_or("0x0")).unwrap();
let gas_limit = Gas::from_str(matches.value_of("GAS_LIMIT").unwrap_or("0x2540be400")).unwrap();
let gas_price = Gas::from_str(matches.value_of("GAS_PRICE").unwrap_or("0x0")).unwrap();
Expand Down Expand Up @@ -188,7 +207,11 @@ fn main() {

let mut vm: Box<VM> = if matches.is_present("CODE") {
let context = Context {
address, caller, gas_limit, gas_price, value,
address,
caller,
gas_limit,
gas_price,
value,
code: Rc::new(code),
data: Rc::new(data),
origin: caller,
Expand All @@ -207,13 +230,14 @@ fn main() {
} else {
let transaction = ValidTransaction {
caller: Some(caller),
value, gas_limit, gas_price,
value,
gas_limit,
gas_price,
input: Rc::new(data),
nonce: match client {
Some(ref mut client) => {
U256::from_str(&client.get_transaction_count(&format!("0x{:x}", caller),
&block_number)).unwrap()
},
U256::from_str(&client.get_transaction_count(&format!("0x{:x}", caller), &block_number)).unwrap()
}
None => U256::zero(),
},
action: if is_create {
Expand All @@ -234,7 +258,7 @@ fn main() {
match client {
Some(ref mut client) => {
handle_fire_with_rpc(client, vm.deref_mut(), block_number);
},
}
None => {
if matches.is_present("PROFILE") {
profile_fire_without_rpc(vm.deref_mut());
Expand All @@ -249,7 +273,7 @@ fn main() {
} else {
handle_fire_without_rpc(vm.deref_mut());
}
},
}
}

println!("VM returned: {:?}", vm.status());
Expand Down

0 comments on commit 6a60714

Please sign in to comment.