search

ElementsProject / rust-elements

Rust support for Elements transaction/block deserialization
Creative Commons Zero v1.0 Universal
52 stars 33 forks source link

confidential: Don't reverse explicit values for serde #174

Open stevenroose opened 1 year ago

stevenroose commented 1 year ago

So this is a breaking change. It might affect people actually using the serde serialization of confidential::Value::Explicit right now. Which is really unfortunate.

But IMO this is a bug. We for some miraculous reason (probably a mistake on some programmer's end) reverse the bytes when consensus-encoding explicit values. That's fine. But somehow we also ended up doing this for serde, which makes no sense.

I'm hitting this when trying to use this type in a Web setting through WASM and a small number of satoshis will (when reversed) cause a JS overflow (which only supports 53 bit integers).

stevenroose commented 1 year ago

Maybe I should use the opportunity to change the serde for all confidential types to be a bit more JSON-friendly and use the lenght/type of the field to know whether it's confidential or not... Opinions welcome.

apoelstra commented 1 year ago

Why doesn't it make sense for the serde encoding to match the on-wire encoding for explicit amounts, when it matches the encoding for confidential amounts?

sanket1729 commented 1 year ago

I agree that this is more of a bug, but I think that we should just live with it. This change will break a lot of things in many subtle ways. I think some implementations for liquid of electrum server also use serde format as a persistent store.

Can you work around this for your setting?

sanket1729 commented 1 year ago

See https://github.com/ElementsProject/rust-elements/pull/96 and https://github.com/ElementsProject/rust-elements/issues/105 where we broke serde format across versions.

apoelstra commented 1 year ago

If it's a bug, it seems like it's a consensus bug in Elements. We can fix it during the bulletproof hardfork maybe.