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

Fix ecmul_0-3_5616_28000_96 #473

Merged
merged 1 commit into from Mar 13, 2019
Merged

Fix ecmul_0-3_5616_28000_96 #473

merged 1 commit into from Mar 13, 2019

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Mar 13, 2019

This PR fixes the failing ecmul_0-3_5616_28000_96 test case. You might notice that the fix is not done in code, but in the test suite. I noticed that the tx is not being executed at all, because the tx sender doesn't have enough balance, but I was puzzled why then this invalid test is failing. I found an explanation in PegaSysEng/pantheon#106. Here's a summary:

A requirement of the test harness is that if a test is invalid and the coinbase account is empty, it should be dropped from the state. This is mainly for state tests, as in blockchain tests, an invalid tx is simply rejected.

It might be a good idea to verify this nevertheless.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Think this is a good fix with enough validation for now, further digging would require too much extra work and this won't get lost since it is well documented in the test suite code, will approve.

@s1na s1na merged commit 664369c into master Mar 13, 2019
@s1na s1na deleted the test/skipped-ecmul branch March 13, 2019 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants