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.

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

Emit better balance diffs when comparing accounts #1580

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

lithp
Copy link
Contributor

@lithp lithp commented Dec 12, 2018

Instead of emitting expected - actual, emit actual - expected.

Rationale (by @veox):

For example, as things stand, if a fixture-expected value is 1000, and
the actual produced by py-evm is 500, the printed message will show a
delta of 500 - that is, a net positive, whereas what truly happened is
py-evm having produced a value that's too low. This is confusing: say, a
miner's account balance showing "Delta: 500" makes one think that the
miner received 500 wei more than expected (which is not true).

Reference:

Cute Animal Picture

babyhawk

Instead of emitting expected - actual, emit actual - expected.

Rationale (by @veox):

> For example, as things stand, if a fixture-expected value is 1000, and
> the actual produced by py-evm is 500, the printed message will show a
> delta of 500 - that is, a net positive, whereas what truly happened is
> py-evm having produced a value that's too low. This is confusing: say, a
> miner's account balance showing "Delta: 500" makes one think that the
> miner received 500 wei more than expected (which is not true).

Reference:
- ethereum#1181 (comment)
- ethereum#1573 (comment)
@carver carver merged commit 92690f6 into ethereum:master Dec 12, 2018
@lithp lithp deleted the lithp/reverse-diff-sign branch December 12, 2018 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants