ssz: finish implementation #42
Conversation
arnetheduck
commented
Dec 15, 2018
arnetheduck
commented
- add object support, simplify implementation
- fix extra round of hashing in tree_hash_root
* add object support, simplify implementation * fix extra round of hashing in tree_hash_root
arnetheduck
force-pushed
the
ssz-refresh
branch
from
c4fe5e7
to
e5946c0
Compare
beacon_chain/ssz.nim
Outdated
| # for which toBytesSSZ is defined! | ||
| # TODO think about this for a bit - depends where the serialization of | ||
| # validator keys ends up going.. | ||
| SomeInteger | Uint24 | EthAddress | Eth2Digest | ValidatorPubKey | |
There was a problem hiding this comment.
We might want to split integers from the others as there is this endianness consideration
There was a problem hiding this comment.
well, endianess and such type-to-byte conversions are the responsibility of toBytesSSZ - this type union describes the types that are statically sized, basically, and therefore don't need a length prefix in the encoding.
| true | ||
|
|
||
| func deserialize[T: enum](dest: var T, offset: var int, data: openArray[byte]): bool = | ||
| # TODO er, verify the size here, probably an uint64 but... |
There was a problem hiding this comment.
we can use case T.sizeof: though to ensure compile-time evaluation the syntax may be awkward.
There was a problem hiding this comment.
the problem here is that sizeof(enum) is undefined in nim - at least I couldn't find a reliable reference.
There was a problem hiding this comment.
It's the smallest size needed to store an enum (1 byte if less than 256 values). Note sure how that works with enum with holes though.
On devel it should be more reliable given the merging of nim-lang/Nim#5664 and its extensive test suite.
There was a problem hiding this comment.
It's the smallest size needed to store an enum (1 byte if less than 256 values). Note sure how that works with enum with holes though.
where does it say that? also, that kind of implicitness is bad when working with bytes on the wire, easy to mess up.
nim-lang/Nim#5664 is pretty messy - the interplay between the c and nim compilers, and the platform and options passed to the c compiler, is incomplete. it will need a lot more rigor to be usable in a cross-platform setting.
There was a problem hiding this comment.
Right it's not in the manual so it's implementation detail. It works like that for the C and C++ backend unless the {.size: mySizeInBytes.} pragma is added.
