-
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: ttGasPrice #956
Conversation
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.
there must be an exception
"TransactionWithGasPriceOverflow" : | ||
{ | ||
"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.
this should trigger an exception as field is >256bits
Hi @holiman , it seems like this field accepts 256bit+ values, although it is impossible to make a valid transaction with such values, should t8ntool raise an exception? |
Interesting. Yeah, whether t9n gives an exception or not is up for debate. I can go whichever way, really. t9n is mostly a tool to aid in test generation, not the actual test executor. Having >256bit gasprice values is interesting -- e.g make a tx where the gas (21K) multiplied by gasPrice becomes something like I think too high gasprices are not intrinsically invalid, just in that 'ghost area' on semi-validness which happens due to balances being capped at 256 bits. |
I just tested this scenario and the transaction is correctly rejected with 'insufficient funds' exception, so it seems ok at least for this client. |
Question here, @holiman also Any of the 256bit overflow in tr fields should trigger an exception as this is a transaction structure unit tests many devs are using for testing |
So you're saying that
Sure, can do that |
Which tx has gasprice exceeding 256 bits? The one in HighGasPrice:
That's "only" |
( and TransactionWithGasPriceOverflow.json was deleted in this PR) |
TransactionWithGasPriceOverflow.json has been added again. I deleted it the first time because, since it was expecting an error, it could not be filled. Now it passes with ethereum/go-ethereum#23743 |
Contains updated TransactionTests for category ttGasPrice.
Outstanding issues: