core/types: support yParity field in JSON transactions #27744
Conversation
| @@ -155,23 +186,24 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error { | |||
| return errors.New("missing required field 'input' in transaction") | |||
| } | |||
| itx.Data = *dec.Input | |||
| if dec.V == nil { | |||
There was a problem hiding this comment.
Seemed to look better to me.
There was a problem hiding this comment.
Feels weird to have them out of order. v, r, s ordering is pretty widely understood.
| if dec.S == nil { | ||
| return errors.New("missing required field 's' in transaction") | ||
| } | ||
| itx.S = (*big.Int)(dec.S) | ||
| withSignature := itx.V.Sign() != 0 || itx.R.Sign() != 0 || itx.S.Sign() != 0 | ||
| if withSignature { |
There was a problem hiding this comment.
So before this change, sanityCheckSignature would only run when any value v, r, or s was non-zero. Now it runs always, meaning users who were unmarshalling txs with all zeros will now have an error?
There was a problem hiding this comment.
I have restored this behavior now, but I don't really get it. The signature values are mandatory, but all-zero is OK?
There was a problem hiding this comment.
The only place I am aware this functionality is used is in t8n to denote that the transaction should be signed using the provided secretKey:
go-ethereum/cmd/evm/testdata/8/txs.json
Lines 19 to 21 in 7f756dc
Would probably be good to get rid of all-zero values at some point, but I'm not sure what else depends on this behavior.
There was a problem hiding this comment.
Either way it doesn't matter since an all zero signature will be invalid anyway.
This adds support for the "yParity" field in transaction objects returned by RPC APIs. We somehow forgot to add this field even though it has been in the spec for a long time.
…eum#27744)" This reverts commit 932ffcb.
…eum#27744)" This reverts commit 932ffcb.
Fixes #27727