node: add spv mode with 5pi fallback #409
Conversation
|
ready for review and feel free to pull down for testing. however, i wont merge until i deploy the better version of 5pi by @kurumiimari :D |
| async verifyMessageWithName(name, signature, str) { | ||
| if (await this.getSpvMode()) { | ||
| const result = await this.getNameInfo(name); | ||
| const owner = result?.info?.owner; |
There was a problem hiding this comment.
I think this syntax requires nodejs v16 (or is it 14?) Dunno if you've already mentioned that in the readme or set "engines" in package.json ? I guess it's be electron version anyway not nodejs
There was a problem hiding this comment.
This can be enabled with a webpack configurations. Forgot what the exact option is but our configs already supports it.
app/background/node/service.js
Outdated
| if (addr.version !== 0 || addr.hash.length !== 20) | ||
| return false; | ||
|
|
||
| const MAGIC_STRING = `${pkg.currency} signed message:\n`; |
There was a problem hiding this comment.
Instead of re-implementing this here, you should be able to call a regular verifymessage command once you obtain the address.
There was a problem hiding this comment.
The thing I want to avoid tho is sending the signature to hosted node. It seems better to be able to verify signature locally, instead of relying on a hosted node to tell me. Perhaps we can make it an exportable util function in hsd?
There was a problem hiding this comment.
SPV nodes still have rpc verifymessage, it just takes an addres as input instead of the name. So once you get the corresponding address from the API you can just call that locally.
Also, why not send the signature to the API server? We're trusting that node for lots of other stuff already.
There was a problem hiding this comment.
Trust but verify is the general model. We are trusting the hosted node for things that we could verify locally.
For verifymessage, doing it locally means that we only need to trust the node to send us the correct name owner (which could be independently verify) and then verify signature locally. If we rely on the node, we would only get a boolean of whether or not the message is valid, but no ways to verify independently.
There was a problem hiding this comment.
SPV nodes still have rpc verifymessage
Ah, I will do this
There was a problem hiding this comment.
actually - even trusting the full node to return the owner coin should technically be provable - using Urkel. I don't think we have this in hsd quite yet, but an SPV node could use the same pool.resolve() function it uses to resolve TLDs with Urkel Proofs, but instead of returning the dns resource, it could return the owner. (The entire Namestate object is included in urkel, including owner). If you want to explore this route, maybe start with a PR to hsd instead of trying to implement in the UI layer ;-)
app/background/node/service.js
Outdated
| } | ||
|
|
||
| async _execRPC(method, args, proxy) { | ||
| if (proxy) { |
There was a problem hiding this comment.
Why would we ever use the proxy if not in SPV mode?
There was a problem hiding this comment.
True. I will rename variable
There was a problem hiding this comment.
mm maybe i misunderstand. im not asking about the variabel name but why you need to check BOTH proxy AND this.getSpvMode() ?
There was a problem hiding this comment.
oh! this is actually for the rpc method that we DONT want to proxy to hosted node on SPV, such as verifymessage, sendrawtransactions, sendrawairdrop, etc.
app/background/node/service.js
Outdated
| method: 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Authorization': 'Basic ' + Buffer.from(`x:775f8ca39e1748a7b47ff16ad4b1b9ad`).toString('base64'), |
There was a problem hiding this comment.
Too lazy to change code in old 5pi 😂
New 5pi is gonna public to all by default after we switch
|
Looks pretty simple! Left some comments, have not tested yet. Looks like you caught estimatefee already which is an important thing SPV can't do. 👍 |
|
I think we should also think about overall available network resources when we deploy. We've already seen a ton of hnsd nodes come online with Fingertip release! I think we should consider setting |
Will add max outbound in a separate PR |
| put(prefixHash(hash), name); | ||
| return name; | ||
| } | ||
|
|
||
| async getAuctionInfo(name) { | ||
| return this._execRPC('getauctioninfo', [name]); | ||
| return this._execRPC('getauctioninfo', [name], true); |
There was a problem hiding this comment.
wait, isn't this a wallet RPC? If this RPC called from the full node? Looks like its duplicated from the wallet RPC where it is actually used. I think this can be pulled from node/service.js
There was a problem hiding this comment.
yea this method is actually not being used by UI - will remove
Adding SPV mode with a fallback to 5pi for all calls that requires full node (
getTX,getCoin,getNameInfo, etc). Code include aproxyboolean flag toexecRPCto tell Bob to proxy to 5pi when on spv.Also copied implementation of
verifymessagewithnameso that we dont have to send message to a hosted node to verify.Storage comparison: 31MB vs 9GB 😮
When syncing in spv mode, the node seem to be stucked at around 65%. Closing and reopening Bob a few times (3 times is the charm for me) seems to resume syncing for some reason.
To enable SPV mode, simply click enable and wait for node to restart