Conversation
| return computeCID(op.SignedCBORBytes()) | ||
| c, err := drisl.CidForValue(op) | ||
| if err != nil { | ||
| // NOTE: consider changing function signature to return err? |
There was a problem hiding this comment.
Interesting - I wonder what kinds of things internal to CidForValue can actually fail?
There was a problem hiding this comment.
It should only be objects that fail to marshal as valid DRISL or CBOR.
There was a problem hiding this comment.
What could cause that though? I'm just wondering if we can prove it will never fail given the structure of our op object
There was a problem hiding this comment.
Right. It mostly boils down to failing on:
big.Intvalues that are too big/small (exceeds int64)- Tags other than tag 42 (CID)
- Maps with keys that are not strings
- Some banned floating point values: NaN, Inf, -Inf
From what I can see, your structs should be good to go. Regardless returning the error is a good idea.
Note CidForValue adheres to the default rules of the package, so things like floats are allowed, since they are allowed in DRISL. If you'd like to be even stricter you could write your own tiny implementation of CidForValue that using custom EncOptions instead of just calling Marshal. But it shouldn't matter given your structs look to be all strings.
3 participants
This is an update of: #17
Ran
go mod tidyand it is clear how much this cleans up the dependency tree!I didn't update the
CID()method helpers; instead I had them return the empty CID instead of panic-ing. I think it should be very rare (impossible?) to have problems there? Though it is possible we need to enforce size limits.Edit: actually maybe we should be erroring harder on the failed CID generation. There would be a multi-stage attack where a corrupt op gets in the log, and then a successor op has
prevset to the empty CID, and they would "match" (?).