deserializedHEAD API #18
Conversation
Closes #17
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
Add explicit permissions
taoeffect
left a comment
taoeffect
left a comment
There was a problem hiding this comment.
Great work on this @corrideat ! Minor request re API design:
| controller.error(new Error('chelonia/out/eventsBetween: Mismatched contract ID')) | ||
| return | ||
| } | ||
| const deserializedHEAD = await sbp('chelonia/out/deserializedHEAD', startHash, { contractID }) |
There was a problem hiding this comment.
Somewhat confusing API imo.
- The
contractIDappears to be optional as it's typedcontractID?: string, yet the implementation always does the comparison, indicating it's mandatory. - So if it's mandatory I wouldn't split the parameters like this, but instead move
hashinto the object. Also better either way as it's named parameters like that and named parameters are easier to read at the call site
There was a problem hiding this comment.
You're right, it was meant to be optional (and the comparison therefore should be optional). I was thinking that sometimes one may use this function to lookup the contractID.
| const message = await sbp('chelonia/out/fetchResource', hash, { | ||
| code: multicodes.SHELTER_CONTRACT_DATA | ||
| }) | ||
| const deserializedHEAD = SPMessage.deserializeHEAD(message) |
There was a problem hiding this comment.
Nice catch here noticing that this could be replaced with a call to fetchResource!
|
@taoeffect If you approve, please don't merge it yet, as I'll work on |
|
It looks like all the changes that will be needed should be in place now. I've also modified |
Closes #17