On JSON-RPC, authorization serializes v as yParity#209
Conversation
|
this passes hive and everything? ie feels like we should have nimbus-eth1/2 PR:s that include this change so their CI's run |
web3/primitives.nim
Outdated
| minLen: static[int] = 0, | ||
| maxLen: static[int] = high(int)] = distinct seq[byte] | ||
|
|
||
| U8Quantity* = distinct uint8 |
There was a problem hiding this comment.
We don't need a new U8Quantity, only readValue/writeValue for uint8. n-j-s is smart enough today.
Line 307 in 6d46159
There was a problem hiding this comment.
shows we can even get rid of Quantity if needed.
about that, we still lack the "flavor" option to disable the automatic serialization of integers and other primitives, so it's actually risky to rely on uint64
There was a problem hiding this comment.
okay, I think we need to implement status-im/nim-json-serialization#121 first. looks not too difficult
JSON uses yParity for authorization instead of v like in transaction, because v for transaction has additional information mixed in in legacy case (chain ID and magic value); this legacy case is not relevant for authorizations. Existing logic ignored yParity value from JSON and uses default initialized v = 0 value. Create our own web3 type, same as how we do it for transactions and receipts, and use correct yParity key.
jangko
force-pushed
the
dev/etan/ff-yparity
branch
from
50d7ef4 to
7421228
Compare
web3/conversions.nim
Outdated
|
|
||
| # Disable automatic primitive serialization, especially uint64 and uint8 | ||
| # We don't want they are serialized into ordinary number, but hex. | ||
| JrpcConv.automaticSerialization(SomeInteger, false) |
There was a problem hiding this comment.
I think this is what I meant by the feature feeling messy .. ie it feels like it would be easier to start from a point where JrpcConv has no automatic conversions at all and then add them, rather than disabling some conversions here - this approach feels quite weird and risky, also because of status-im/nim-json-rpc#239..
what would happen here? if I (or some dependency) imports this file, automatic serialization will be disabled where exactly? in this module? in all modules due to macrocache? in some modules, ie those that were imported after this one?
| # JrpcConv configuration | ||
| #------------------------------------------------------------------------------ | ||
|
|
||
| JrpcConv.automaticSerialization(string, true) |
There was a problem hiding this comment.
remove the true here? ie is false meaningful?
There was a problem hiding this comment.
Yeah, that can be done later.
Co-authored-by: Jacek Sieka <jacek@status.im>
|
one more step left. the new hardhat expect one of |
jangko
force-pushed
the
dev/etan/ff-yparity
branch
from
a3aa6c8 to
633b8fd
Compare
4 participants
JSON uses yParity for authorization instead of v like in transaction, because v for transaction has additional information mixed in in legacy case (chain ID and magic value); this legacy case is not relevant for authorizations. Existing logic ignored yParity value from JSON and uses default initialized v = 0 value. Create our own web3 type, same as how we do it for transactions and receipts, and use correct yParity key.