-
Notifications
You must be signed in to change notification settings - Fork 318
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
TransactionTests Update: ttRSValue #959
Conversation
src/TransactionTestsFiller/ttRSValue/TransactionWithSvaluePrefixed00Filler.json
Show resolved
Hide resolved
], | ||
"TransactionWithSvalue1" : | ||
{ | ||
"expectException" : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it a github bug or ident here is different to the one with transaction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indent was incorrect in the original file, I tried to make indentation consistent in all files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you added TAB
src/TransactionTestsFiller/ttRSValue/TransactionWithRvaluePrefixed00Filler.json
Show resolved
Hide resolved
{ | ||
"expectException" : | ||
{ | ||
"<=EIP150": "InvalidVRS", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I forgot I actully support <= in network translation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here it should reject I think not with invalid chain id but with prefixed 00, as the v value limitations. need to check with the v value cap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the expected to LeadingZerosV, and changed the V value to bigint.
Let me know if this is ok.
@@ -14,9 +13,9 @@ | |||
"nonce" : "0", | |||
"to" : "095e7baea6a6c7c4c2dfeb977efac326af552d87", | |||
"value" : "11", | |||
"v" : "27", | |||
"v" : "28", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
Contains updated TransactionTests for category ttRSValue.