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.

Sign up for GitHub

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

Jump to bottom

Vmtestsfix #320

Merged
merged 4 commits into from
Jan 2, 2020
Merged

Vmtestsfix #320

merged 4 commits into from
Jan 2, 2020

Conversation

MrChico
Copy link
Member

@MrChico MrChico commented Jan 2, 2020

Unfortunately, I introduced a bug in f1b9b34, wherein the rlp encoding of transactions wrapped some fields twice. I.e. a transaction:

(Transaction {txData = "", txGasLimit = 0x61a80, txGasPrice = 0x1, txNonce = 0x0, txR = 0xe94818d1f3b0c69eb37720145a5ead7fbf6f8d80139dd53953b4a782301050a3, txS = 0x1fcf46908c01576715411be0857e30027d6be3250a3653f049b3ff8d74d2540c, txToAddr = Just 0x95e7baea6a6c7c4c2dfeb977efac326af552d87, txV = 0x1c, txValue = 0x186a0})

was encoded as:

[0x80,0x01,0x83061a80,0x94095e7baea6a6c7c4c2dfeb977efac326af552d87,0x830186a0,0x80]

instead of:

[0x,0x01,0x061a80,0x095e7baea6a6c7c4c2dfeb977efac326af552d87,0x0186a0,0x]

This error caused blockchain-tests to not be able to decode any "valid tests", since all transactions were malformed. But because InvalidTx was not treated as a fatal error, no failures were reported and we happily ignored all tests.

This fixes the tx encoding bug, and makes InvalidTx a fatal error.
Let's see how the ci does on this one

@MrChico
Copy link
Member Author

MrChico commented Jan 2, 2020

I have also removed modexp from skipped tests, as its darwin problems were fixed by #250

desaperados
desaperados approved these changes Jan 2, 2020
View reviewed changes
Copy link
Member

@desaperados desaperados left a comment

Choose a reason for hiding this comment

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

LGTM

@MrChico MrChico merged commit 2aec0f6 into master Jan 2, 2020
@MrChico MrChico deleted the vmtestsfix branch January 2, 2020 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@desaperados desaperados desaperados approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MrChico @desaperados