-
Notifications
You must be signed in to change notification settings - Fork 56
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
conversion: revert json-marshalling change in #144, marshal hex-format #145
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #145 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 1643 1643
=========================================
Hits 1643 1643 |
Because in many places we jump through hoops in order to force our |
As @fjl noted:
|
What is the status here? will this change from hex to decimal format be reverted or not? Having decimal marshaling is kind of annoying in the ethereum context and would require a proxy type as |
We discussed this, and agreed to not revert it. I implemented a hexutil.U256 type in go-ethereum, and closed this PR.
|
Okay. Just out of curiosity, what is the reason for changing this in the first place? To give some more context: My lib is effected by this breaking change (I presume it will be relatively easy to fix though). But still, this package has hundreds of direct dependents which code might break or act unexpected. This should be a v2 release IMO or kept as is. |
To be more like
Well, depends on how you see it. It doesn't break the programing API, but yes, it does break some places where marshalling is concerned. But many places where you would unmarshal a hex-string, would also accept an int-string. Anyway, go-ethereum is the main driver for this library. |
This undoes the change in json-marshalling, making it so that we (again) marshal
uint256.Int
in hex-format. This is more widely useful in go-ethereum.I originally intended to let it be this way, but I changed my mind due to this comment: #144 (comment).
This would be an alternative to ethereum/go-ethereum#28628
cc @fjl @karalabe