search

filecoin-project / go-amt-ipld

Implementation of an array mapped trie using go and ipld
Other
9 stars 15 forks source link

fix: bitWidth can't be '1' #58

Closed rvagg closed 2 years ago

rvagg commented 2 years ago

@Stebalien I think we can actually have a bitWidth of 1 for a branching factor of 2, so this might just be an exercise in making the error match the condition, or we could say that a branching factor of 2 is a little bit silly. But maybe that's not our place to say, if someone wants to make ridiculous trees.

The message here could be something like bitWidth must be at least 1 (branching factor of 2) and leave the <1, or change it to <=1 and say bitWidth must be at least 2 (branching factor of 4).

Or we could just throw our hands in the air and say that bitWidth doesn't make as much sense for an AMT as it does for a HAMT and how about we just use "width" and drop this terminology that's going to be confusing to most users. We'd just make sure it's a power of 2 value so we can properly set the bitWidth property in the encoded nodes, and hide that detail from users.

Stebalien commented 2 years ago

Oh, sorry, I got my comparisons confused and misread the title as a statement of what is, not what will be (after the patch).

Yeah, a bitwidth of 1 should be possible. Let's fix the error message.

rvagg commented 2 years ago

Fixed, and made a minor related doc fix while I was at it.