Transaction potentially rejected for the wrong reasons

[norswap]

First of all, I know transactions tests are not being maintained, see #906

Nevertheless, I thought I'd report some issue with them, in case someone is interested.

The issue is that some transactions in those tests have a "malleable signature", essentially, a signature s that is higher than the ellipctic curve parameter N divided by 2. This should be rejected after EIP-2 (to avoid replay attacks) and so is only valid on Frontier.

The affected transactions are:

• TransactionTests/ttData/DataTestNotEnoughGAS.json
• TransactionTests/ttGasPrice/TransactionWithGasPriceOverflow.json
• TransactionTests/ttNonce/TransactionWithNonceOverflow.json

All of these are illegal transactions, and so should fail to parse or to be accepted in general.

The problem is that currently, an implementation may reject them (except for Frontier) because the signature is invalid, and not because it validated the thing that was supposed to be tested (e.g. for TransactionWithGasPriceOverflow: that the gas price does not overflow, i.e. is encoded on more than 32 bytes), making these tests much less useful than they should be.

I regenerated the signature on the transactions so that it is EIP-2 compliant.

What I did to work around this was regenerate the signatures on the transactions, here's the nanoeth commit.

I could submit this change as a PR if it's deemed useful. However then I'd need to know how to update the metadata fields. And probably update the filler files too, for consistency?

[winsvega]

Hm. Transaction tests currently not supported as there is no legitimate way to generate it.

You can open a PR, we could pretend that it is still being filled from changed fillers.

Ideally there would be a mechanism to provide data from filler to the client and get the result if a transaction is legit. But apparently its much less relevant then the core transition tests.

[norswap]

Will do the PR then!

I might make such a tool - are you attached to the current JSON format, or could it be changed? Of course, it wouldn't be useful immediately, since my "client" is itself being implemented and almost assuredly has bugs.

[winsvega]

Also the existing ones are verified by aleth constantinople implementation. Are you sure the signature is incorrect?

[norswap]

To clarify: these are negative tests, meaning that the transaction should be rejected for the tests to pass.

Are you saying that you check the signature (and only the signature) with aleth, and aleth says that all signatures are valid (even on these negative tests)?

I would be very surprised if it's a bug on my side — it would mean I'm parsing the signature values incorrectly. But if that was the case, how come I can verify every other signature, whether I sign it myself or it's signed by someone else?

[winsvega]

just checked this file DataTestNotEnoughGASFiller.json and filled DataTestNotEnoughGAS the test is correct with s/2 requirements, what is the issue?

Those tests clearly say that transaction is expected to be rejected.

[winsvega]

reworked