Import/Export #61
Import/Export #61
Conversation
nodech
commented
Jul 25, 2019
•
nodech
commented
- Exports Wallet with cosigners and account.
- This format can be reused in bcoin as well (maybe minor improvements) and should be compatible
- ❌ Currently does not export/import imported addresses.
- ❌ received, change and nested depth is not recovered (for now)
nodech
force-pushed
the
import-export
branch
3 times, most recently
from
05e618b
to
93788f7
Compare
| * Wraps Cosigner primitive. | ||
| */ | ||
|
|
||
| class CosignerDetails extends Struct { |
There was a problem hiding this comment.
Why is there a CosignerDetails object when it doesn't add anything additional to the Cosigner?
There was a problem hiding this comment.
To make better wrapper for serializers. Cosigner unlike Wallet and Account is already serializable so I just wrapped it so it does not expose functionality that wont work (mostly client side)
| */ | ||
|
|
||
| class CosignerDetails extends Struct { | ||
| constructor() { |
There was a problem hiding this comment.
Should the constructor be able to take the Cosigner as well? Its not very useful to use new CosignerDetails() unless you wanted a nil cosigner object
There was a problem hiding this comment.
All these serializers give you default object. So I don't think having many ways to pass cosigner will improve much. (Especially these always clone the data passed.)
lib/export.js
Outdated
| master: this.master.toRaw().toString('hex'), | ||
| joinPubKey: this.joinPubKey.toString('hex'), | ||
| timestamp: this.timestamp, | ||
|
|
lib/export.js
Outdated
| bw.writeU40(this.timestamp); | ||
| bw.writeBytes(this.joinPubKey); | ||
|
|
||
| bw.writeU8(this.accounts.length); |
There was a problem hiding this comment.
I don't see any checks to make sure this won't overflow
There was a problem hiding this comment.
Multisig only has one account, but true this needs change to be useful for bcoin.
lib/multisigdb.js
Outdated
| /** | ||
| * Export wallet. (without lock) | ||
| * @param {Number} wid | ||
| * @returns [Wallet, Account[], Cosigner[]] |
| assert(account.type === Account.types.MULTISIG); | ||
| assert(account.name === 'default'); | ||
| assert(account.initialized === true); | ||
| assert(account.accountIndex === 0); |
There was a problem hiding this comment.
Why does the accountIndex need to be 0 here?
There was a problem hiding this comment.
Multisig wallets only have one account in them, so it should never be more than 0.
There was a problem hiding this comment.
That would be good to add as a comment
- Add tests - Add client calls.
nodech
force-pushed
the
import-export
branch
from
5a909e1
to
af60704
Compare
