search

trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

Potentially rework codec output format? #5483

Open haltman-at opened 2 years ago

haltman-at commented 2 years ago

Issue

This is an issue to collect some potential breaking changes to the codec output format we might want to make. Not all of these are compatible with one another, and some of them might be bad ideas. But I thought I'd collect them here!

  1. Replace BNs with BigInts

The format was designed before BigInts were safe to use everywhere. BNs are mutable objects which is annoying. BigInts would be nicer?

  1. Replace BNs and Bigs with strings

Because of its use of BN and Big (and one other thing, see below), the format can't be easily serialized and deserialized with JSON.stringify and JSON.parse, something that has caused no end of headaches. That doesn't mean it can't be serialized or deserialized some other way, but so far we haven't bothered to write that.

Or rather -- we tried to write that, back in PR #2901, and the method we attempted to use for it hit some snags. Not related to the code itself but rather due to some of the things surrounding the code. Well... I won't bother going over that again here. Anyway it was a problem.

So, if we got rid of the BNs and Bigs, and just made it directly serializable with JSON.stringify and JSON.parse, that would be nice. Annoyingly, BigInts can't be serialized and deserialized in this way, so it'd have to be strings or something. :-/ Blech. Using strings for integers bothers me but it may be unavoidable? Using it for decimal fixed-point doesn't bother me so much, but it does have its own problem, of agreeing on a specific format of string...

  1. Remove the circularities :P

This is the other thing that would have to be changed if we want native serialization and deserialization. This is much less big a deal though. Long story short -- for reasons that I'll skip going into, when designing the format, I thought, hey, why not represent circular objects with circular objects? (Marked, of course, so code processing it doesn't go into loops.) This wasn't out of nowhere, there was actually a specific use case I had in mind for this, but it never materialized. So, uh, maybe it would be better to get rid of that.

(That said, this change would actually be less necessary than (2), as if we want to simply handle this step separately, well, you can see that done in #2901 and it works fine; it didn't encounter the typing problems that the use of BNs and Bigs caused...)

Environment

gnidan commented 2 years ago

We may want to hold off on this until we have @truffle/codec-components, since the absence of that library makes this format more externally-facing than if we had that library.

haltman-at commented 2 years ago

Note: @gnidan points out the existence of the superJson package, which would allow us to use BigInts. Obviously Bigs would still be a problem, but I'm more OK with using strings there (so long as we can agree on a string format!).

gnidan commented 2 years ago

I'm torn... I really like the simplicity of just using strings everywhere and avoiding the need for custom JSON logic. But yeah, trade-offs... 🤷