Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fixing evm-debug #2161

Merged
merged 3 commits into from
Sep 20, 2016
Merged

Fixing evm-debug #2161

merged 3 commits into from
Sep 20, 2016

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 18, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 85.255% when pulling 9bf29a5 on fixes into 7f7e748 on master.

@@ -113,7 +113,7 @@ impl<'a> Finalize for Result<GasLeft<'a>> {
}

/// Cost calculation type. For low-gas usage we calculate costs using usize instead of U256
pub trait CostType: ops::Mul<Output=Self> + ops::Div<Output=Self> + ops::Add<Output=Self> + ops::Sub<Output=Self> + ops::Shr<usize, Output=Self> + ops::Shl<usize, Output=Self> + cmp::Ord + Sized + From<usize> + Copy {
pub trait CostType: ops::Mul<Output=Self> + ops::Div<Output=Self> + ops::Add<Output=Self> + ops::Sub<Output=Self> + ops::Shr<usize, Output=Self> + ops::Shl<usize, Output=Self> + cmp::Ord + Sized + From<usize> + Copy + fmt::Debug {
Copy link
Contributor

Choose a reason for hiding this comment

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

this line should be broken up

@@ -126,7 +126,7 @@ impl<Cost: CostType> evm::Evm for Interpreter<Cost> {
gasometer.current_gas = gasometer.current_gas - gas_cost;

evm_debug!({
println!("[0x{:x}][{}(0x{:x}) Gas: {:x}\n Gas Before: {:x}",
println!("[0x{:x}][{}(0x{:x}) Gas: {:?}\n Gas Before: {:?}",
Copy link
Contributor

Choose a reason for hiding this comment

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

should be one of the logging related macros (probably just debug!?)

Copy link
Collaborator Author

@tomusdrw tomusdrw Sep 19, 2016

Choose a reason for hiding this comment

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

It's much easier to run/debug tests when there are printlns, cause logs are not initialized in tests. It's also a compilation feature, so when someone is compiling with it I guess you would expect the logs to be always printed.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A0-pleasereview 🤓 Pull request needs code review. A8-looksgood 🦄 Pull request is reviewed well. labels Sep 19, 2016
@rphmeier
Copy link
Contributor

There's a typo in the trait bounds here, needs fixing before merge.

@tomusdrw
Copy link
Collaborator Author

Shoot, I'm sorry, I haven't noticed that typo. Thanks!

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Sep 20, 2016
@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 20, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.278% when pulling 0921dff on fixes into 7f7e748 on master.

@rphmeier rphmeier merged commit 93f82a1 into master Sep 20, 2016
@tomusdrw tomusdrw deleted the fixes branch September 23, 2016 18:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants