Conversation
maidh91
force-pushed
the
expose-multisig-fn
branch
from
883ce5d to
62aa64b
Compare
lejeunerenard
left a comment
lejeunerenard
left a comment
There was a problem hiding this comment.
I think the exports method for exports from lib/multisig.js makes more sense. The methods are currently used externally via the requiring lib/multisig.js and so adding the module via exports fits better.
Also made a note that the core verifier should be used instead of assemble() directly to account for whether the core is a compat core.
| return Replicator.clearRequests(session, err) | ||
| } | ||
|
|
||
| static assemble = assemble |
There was a problem hiding this comment.
The Verifier.assemble() is what you want instead and is accessible via the core.core.verifier property. This is because cores can be an old compat which used a different assemble method for assembling proofs & patches. The main place that we use multisig (besides hyper-multisig) is autobase where we use the verifier.
Assembling is core specific and can be done via the verifier, so there is no need for a static method in my view.
There was a problem hiding this comment.
thanks @lejeunerenard, I will check carefullly the verifier again (it's been running ok so far for ages)
for the export convention, in hyper-multisig repo, we are importing like this, and we want to avoid this pattern (should require('hypercore') only)
const { assemble, partialSignature } = require('hypercore/lib/multisig')cc @HDegroote
2 participants
use in https://github.com/holepunchto/hyper-multisig/blob/dc8477095477c0ff9aa00f23ce905428a748284c/lib/util.js#L23