-
Notifications
You must be signed in to change notification settings - Fork 108
Conversation
@rvagg has been working on an implementation actually. it would be nice to eventually align his, the Go implementation, and the Java HAMT that peergos wrote. |
Yeah, would definitely love some review from implementers. Tagging him and ian now |
cc @ianopolous |
Data-Structures/HAMT.md
Outdated
The `KV` is serialized as a cbor array (major type 4) with the 'key' field | ||
serialized as a cbor string (major type 3) (TODO: should this just be major | ||
type 2? its probably good to support arbitrary bytes as keys) and placed in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, we also use byte[] as keys. In our main usage they are random 32 byte arrays (not a hash).
type 2? its probably good to support arbitrary bytes as keys) and placed in the | ||
zero'th position of the array, and the value serialized { in some way } and | ||
placed in array position 1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A cbor map would be even more compact (although, I guess, IPLD doesn't currently support binary keys...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrm.. yeah. that might get complicated
|
||
|
||
## Set Value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weren't we considering some kind of hashing with replacement system to completely fill up each layer? Or was that too expensive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didnt end up investigating it too much, it feels like it might get pretty expensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The buckets system is probably good enough for most cases. What test data did you use when testing depth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just random keys and values, did some number of inserts, measured average densities, depths, etc. I don't think i committed the code, but it was just the tests in go-hamt-ipld, with some stats collection. Could rig it up again pretty quickly
|
||
```go | ||
type Node struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add an optional seed (defaults to 0)? That way we have room to fix the hashmap DoS attack if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been messing with hash algorithm pluggability thinking that (a) different algorithms (and different key lengths) might be optimal in different scenarios, and (b) having the ability to switch it out provides some future-proofing in case of fundamental flaws being discovered in a chosen algorithm. Having space for a seed would also open up space for some keyed algorithms too.
What I can't see (yet) is what kind of use-cases of IPLD are there that attacks against the hash would matter? What's the threat model where this is a concern or is it just a matter of being safe for some as yet unforeseen scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hashmap DoS attack works by attacker inserting keys that hash to the same bucket in a hash map. Something very similar can be done with HAMT by selecting keys to lay in the same branch of the tire.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seed/Nonce works around it by making so the attacker can't simply predict in which branch of the tree will given key lay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not actually sure how adding a seed helps here. It needs to be deterministic, and if its deterministic, then the attacker can know it too and it doesnt make their lives any harder.
I guess forcing a rehash at each layer makes the attack linearly more expensive, but doesnt necessarily prevent attacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, we actually do need to support, e.g., sha256 if we want both security and determinism on systems like Filecoin. Currently, given an N byte insecure hash function, an attacker could create a tree N deep at the target hash, filling the last layer. This could be used to prevent anyone from using a specific key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hard time imagining wanting to use non-deterministic balancing in a distributed system. It seems that would produce either massive flapping if actually used in a frequently updated dataset, and/or introduces a need for coordination where they're previously wasn't one (which is pretty much universally a Bad Thing in a distributed system). Is there a concrete situation where we can imagine using such a thing, and using it well?
"Use a SHA (or other cryptographic function) when it matters" sounds like a much better approach. We're already tossing around enough cryptographic functions that it doesn't sound likely to be much of a cost center to introduce another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hard time imagining wanting to use non-deterministic balancing in a distributed system. It seems that would produce either massive flapping if actually used in a frequently updated dataset, and/or introduces a need for coordination where they're previously wasn't one (which is pretty much universally a Bad Thing in a distributed system). Is there a concrete situation where we can imagine using such a thing, and using it well?
So, the simple use-case is a block-chain where the blockchain determines the seed. Once every N blocks, the hamt would be reseeded automatically.
In general, this will also work for single-writer, multi-reader setups. That's usually the most common case.
"Use a SHA (or other cryptographic function) when it matters" sounds like a much better approach. We're already tossing around enough cryptographic functions that it doesn't sound likely to be much of a cost center to introduce another.
I agree although this still won't be optimal. That is, An attacker could pretty easily create a very deep hamt.
Basically, I'd prefer to leave room for future improvements now instead of having to introduce them later. However, custom hash functions is probably enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Stebalien what youre suggesting would mean rewriting the entire HAMT every epoch. I think thats far worse than a (worst case) 32 deep lookup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm saying it could be rebalanced as necessary. However, I agree the better solution is to just use SHA256.
Note: The worst case without using sha256 isn't a 32 deep lookup, it's a full kv list at the max depth preventing further modifications.
To look up a value in the HAMT, first hash the key using a 128 bit murmur3 hash. | ||
Then, for each layer take the first W bits of the hash, and use that to compute | ||
the index for your key, as follows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't W
always a single byte?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's fixed to an arity of 256, but I'm wondering why 256 is chosen here, is it mainly for the ease of accessing the hash in 8-bit chunks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we cap number of links in protobuf nodes at ~170 so I think the conservative approach was taken and the number of links was kept roughly similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, wanted to make it a number that would result in acceptable maximum node sizes, but 256 specifically was chosen simply because it makes reading the next index off the hash easy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, there is a pathological case here: large keys. Should we impose a size limit (256 bytes?) as most filesystems do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a keysize limit is worthwhile, though that should probably be left up to the application
Data-Structures/HAMT.md
Outdated
If the lookup terminated on a non-nil Pointer with existing KVs: | ||
|
||
1.) If the KVs array has fewer than three items in it, insert the new key value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why fix this at 3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my rough math, it comes out that maximum size of a HAMT object with keys of 256 bytes, with 36 byte CIDs, with 256 slots filled by 3 items and 20% CBOR encoding overhead comes up to about 256KiB which is the default size of an IPFS block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, this is good context, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I also ran some experiments with different number sizes, and setting it to 4 didnt seem to improve things much.
I'll make '3' a constant and define it
Data-Structures/HAMT.md
Outdated
|
||
1.) If the KVs array has fewer than three items in it, insert the new key value | ||
pair into the KVs array in order. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*key order
but we also need to define what that means, strict byte comparison order probably
Data-Structures/HAMT.md
Outdated
insert those four items into that node starting from the current depth | ||
(meaning, if the current tree depth is 3, skip the first `3 * W` bits of the | ||
key hash before starting index calculation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and put the cid of the resulting node in the current node where the KV array was
Data-Structures/HAMT.md
Outdated
|
||
Now, count the total number of KV pairs across all Pointers in the current | ||
Node. If that number is less than four, gather the remaining KV pairs, delete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except if we are at depth 0 (the root)
There's also |
@achingbrain that’s good to know. I thought it was still inside |
the node, and re-insert them. If the node they are re-inserted into also then | ||
has less than four elements in it (the newly reinserted elements are the only | ||
ones in the node) then recurse. | ||
Node. If that number is less than four, and we are not at the root node of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less than M+1
A few top level comments.
|
So my experimental JavaScript version is @ https://github.com/rvagg/iamap, it's more similar to the Peergos CHAMP (and Steindorfer's CHAMP PoC) than go-hamt-ipld. It's agnostic to the storage system but there's a IPLD-ish example that encodes with dag-cbor and uses it as Map whose values are links to Sets (same data structure but where keys are the thing we care about but values just exist to make it work, iterating over it with The major difference with the current go-hamt-ipld is that I went with CHAMP's separate Aside from that, the other thing I've done is just make things flexible:
The pre-serialized shape of a node is:
Where elements are either key/value pairs or links to child nodes. I was thinking of moving that to an array to make it more naturally compact, something like:
Where the map's configuration options are the first part (and could expand easily, perhaps adding a seed or some other configuration option for a future variant), the rest is the state of the current node. This would be a very compact serialization across multiple formats without too much special casing. So far, as I play with this I have some suggestions and some unresolved questions:
|
Note that even with values restricted to cids, you can still do this using a raw cid with identity multihash. I'm also very pro using different hash functions. We have several different CHAMPs in Peergos and we either use SHA256 or the identity (in that case our keys are cryptographically random 32 bytes and not attacker controlled) |
Action items/key questions so far:
Arbitrary values are already allowed, i don't see anything in the spec that implies this. Am I missing something?
I guess ~12 bytes isnt the end of the world. It just feels annoying from a 'save the trees' perspective. |
This is describing go-ipfs-hamt as it is now. Is that being used already? I know that hamt-sharding is used for js-ipfs-mfs so is it the same way in Go? If it's the case that this is describing something that's publishing IPLD data today, a unixfs-v1 feature, how about we rename it to something like "unixfs-v1-hamt.md" and be clear that this is specifically for that purpose and then go about making sure that it accurately describes what is being used today. Then that frees up possibilities for more types in the data-structures/ directory. I've been thinking of this PR in terms of an "ideal HAMT" spec for IPLD, but maybe I was wrong and this is about capturing current state? In #110 I've introduced the term "Multi-block Collections" with an introductory description in the data-structures/ directory. I'm imagining the directory filling out with a bunch of specs for different shaped collections: maps, lists, sets, queues, etc., all foreshadowed in that introductory document. In that case, the current unixfs HAMT deserves recording as it is because it's a thing in the wild, but it shouldn't constrain thinking about expansions to HAMT or even other data structures that implement similar user interfaces (Maps). If this is purely about future work then never mind all that. I'm taking a bit of a detour to @warpfork's IPLD Schemas to see how far we can stretch them to describe these complex block-spanning data structures. I'll try and get back to my JS HAMT with Schemas soon and hopefully we'll have a good language to help spec this out. |
Here's my iteration on this spec: #131 It's very close to this one with the following major differences:
|
Clarify sync spec
Closing as this is quite old and we have a much more detailed HAMT spec now. |
I don't think this is done yet, but it should be good enough to start an implementation with. Please review for missing details, grammar, and confusing stuff.