feat(types): add Transaction::{to/from bytes, base_64} #303
Conversation
| pub fn bcs_serialize(&self) -> Result<Vec<u8>> { | ||
| Ok(bcs::to_bytes(&self.0)?) | ||
| /// Serialize the transaction as a `Vec<u8>` of BCS bytes. | ||
| pub fn to_bytes(&self) -> Result<Vec<u8>> { |
There was a problem hiding this comment.
if feel like this naming might conflict with non-encoded bytes. There is (raw) bytes and theres BCS encoded bytes. For future proofing I am wondering if it's better to call this to_bcs/from_bcs. In my opinion it's clearer because one immediately knows what kind of bytes to pass in or how to further process received bytes. Also would be consistent with other signatures that mention the encoding, see to_base64. (NOTE that base64 doesn't mean String necessarily, it just happens to map easily to characters), so it could also return bytes, i.e. a Vec<u8>.
There was a problem hiding this comment.
But is there any use case for bytes that aren't BCS encoded for a transaction? I haven't looked into gRPC what encoding is used there and if that might be relevant, but apart from that I don't have an idea where this could even just theoretically be interesting. I went for this name because BCS is not common, but everyone understands what bytes are. I would rather use from/to_bcs_bytes() at least
There was a problem hiding this comment.
Just checked what Sui does with gRPC and they also use BCS for the transactions there https://github.com/MystenLabs/sui-rust-sdk/blob/master/crates/sui-rpc/src/proto/sui/rpc/v2/transaction.rs
There was a problem hiding this comment.
Even if there's no other use case (right now or forever), I still think it's good practice to mention the encoding as we do in general { to/from_hex, to/from_base64, to/from_json }. Like whenever you need an encoder/decoder to process the bytes or string I would as a personal preference mention that encoding in the method name. I wouldn't call it from/to_bcs_bytes for the same reason we don't call it from/to_hex_string or from/to_base64_string. It's kind of redundant and you can quickly look at the return type to see that it's bytes/string. It's self-documenting. The encoding however is not. But I don't know how others feel about it. 🤷
3 participants
Renamed bcs_serialize() to to_bytes() because I think that's clearer to understand for most users.
Also added from_bytes(), to_base64() and from_base64() to make these conversions easier and exposed them all to the bindings.
Added a dry_run_bytes example which makes use of
from_base64().Fixes #250