ipld / specs

Content-addressed, authenticated, immutable data structures
Other
592 stars 108 forks source link

schema/DM: clarify size of `Int` #272

Open ribasushi opened 4 years ago

ribasushi commented 4 years ago

While reviewing the draft btc spec, I simultaneously came across:

Given that the DataModel is strategically decoupled from the wire-bytes via the codecs: shouldn't we take the opportunity to say "an int is an int, and that's that", and have language angle being dealt with by an implementation, providing transparency to the end-user. Prior art/discussion: https://github.com/golang/go/issues/19623

I searched through the issues and did not find any type of discussion on this.

rvagg commented 4 years ago

This keeps on coming up and the place we've lingered at for now has been to push this concern down as far as we can and keep it away from the DM. As far as I was aware we were already saying "an int is an int, and that's it".

The parts of the docs you're reviewing for Bitcoin are focused on the binary format decode/encode so have to get to that level of detail, when it gets above those concerns to how this is all represented in the data model it becomes much more agnostic (those docs are still to come!). I think that works fine in most cases here. We've developed (recently) a convention of using annotations below any schema text where these additional details need to be provided. That's going to come into play more when we go for codegen and people start insisting that numbers be unsigned or 64 bits when turned into Go. The current idea floating around for that is to have an adjacent annotation language that either sits alongside (or more controversially - inside) the schemas that's only used when codegen is run.

I think the uint256 thing should be ignored entirely. It's a fiction that is only useful in very limited cases. It's a digest and should be treated as bytes. I only include that language because it's a concept that lives in the Bitcoin world. I'd prefer it didn't invade our space. In Bitcoin, it's literally class uint256 : public base_blob<256> - i.e. a fiction.

warpfork commented 3 years ago

I may have addressed this while writing some blurbs about int size in the new docs repo: https://github.com/ipld/docs/blob/ebe0b933ff8137972f35997f309a6c9e4948a223/details/numeric-size.md I think similar remarks may exist elsewhere in the specs repo now as well, but these documents are also purposefully codec-agnostic.