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

EIP 150.1b #2542

Closed
wants to merge 13 commits into from
Closed

EIP 150.1b #2542

wants to merge 13 commits into from

Conversation

gavofyork
Copy link
Contributor

@gavofyork gavofyork commented Oct 9, 2016

As defined in ethereum/EIPs#150 and implemented in ethereum/pyethereum@6d7e28a .

Still TODO:

  • Implement the additional suicide_to_new_account_cost.
  • Tests.

@tomusdrw please review.

@gavofyork gavofyork added the A0-pleasereview 🤓 Pull request needs code review. label Oct 9, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 86.232% when pulling f612828 on gas-fix-hf into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 86.229% when pulling f612828 on gas-fix-hf into 271bcf4 on master.

@@ -73,6 +75,7 @@ impl From<ethjson::spec::EthashParams> for EthashParams {
difficulty_hardfork_transition: p.difficulty_hardfork_transition.map_or(0x7fffffffffffffff, Into::into),
difficulty_hardfork_bound_divisor: p.difficulty_hardfork_bound_divisor.map_or(p.difficulty_bound_divisor.into(), Into::into),
bomb_defuse_transition: p.bomb_defuse_transition.map_or(0x7fffffffffffffff, Into::into),
bad_gas_compatibility_mode_limit: p.bad_gas_compatibility_mode_limit.map_or(0, Into::into),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will break all private chains after upgrade. Maybe leave it disabled by default and just update sample and documentation specs to enable it?

Copy link
Contributor Author

@gavofyork gavofyork Oct 9, 2016

Choose a reason for hiding this comment

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

yeah. the thing is that if we don't default it to 'on', then all private chains will be defaulting to using a broken protocol and run the risk of being internally DoSed.

we can print a warning to ensure they know what went wrong.

@rphmeier rphmeier added the M4-core ⛓ Core client code / Rust. label Oct 10, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 86.301% when pulling a761739 on gas-fix-hf into 271bcf4 on master.

@tomusdrw
Copy link
Collaborator

Looks fine, I think I would get rid of Option (requested), but this can as part of a refactoring later on:

  1. InstructionCost::GasMem -> InstructionRequirements::{Gas, GasMem, GasMemCopy, GasMemProvideGas}
  2. get_gas_cost_mem tuple to struct, also name could be improved
  3. gas_provided could be used only in GasMemProvidedGas branch.

Looks good otherwise 👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 86.339% when pulling bfc3a88 on gas-fix-hf into 271bcf4 on master.

@gavofyork gavofyork added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 11, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 86.332% when pulling 3023c9c on gas-fix-hf into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 86.408% when pulling d6b8e53 on gas-fix-hf into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 86.418% when pulling d6b8e53 on gas-fix-hf into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 86.422% when pulling fa3c730 on gas-fix-hf into 271bcf4 on master.

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 11, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 86.419% when pulling e516b4a on gas-fix-hf into 271bcf4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 86.412% when pulling e516b4a on gas-fix-hf into 271bcf4 on master.

@gavofyork gavofyork closed this Oct 12, 2016
@gavofyork gavofyork deleted the gas-fix-hf branch November 3, 2016 11:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants