Complete hybrid full block SPV mode #9483
Conversation
|
(Prefer if we don't propagate the misuse of "SPV" for things that don't support fraud proofs) |
|
I kinda want to use an open lock icon instead of the likely meaningless to uses SPV in any case. We also should do something about the confirmed counts in this mode. I'm not sure what. The issue is that confirmations mean less when you're not validating. Perhaps displaying transactions like they are unconfirmed until they have 6 blocks might be the thing to do. Or displaying a visible "not-verified" on any transaction with the not validated flag. |
|
Indeed, I would assume any mode like this shouldn't count confirmation at all. |
|
How about a flag for showing confirmations during SPV mode? Default to off. People who understand the implications and just don't want to bother having to search their address on an explorer can enable in the menu / config |
|
@dabura667 Is it sufficient to simply show it in the transaction details dialog, perhaps? |
|
@luke-jr I would think so, yes. |
|
On transactions screen, fully-confirmed receiving txs show only one confirmation, and fully-confirmed sending txs show "unconfirmed" with question marks. |
|
@molxyz I've also noticed the same thing, confirmations in Transactions list don't update, unless I restart Core. Other than that it works great. I used this for testing: |
|
Thanks for reporting. This seems to be a UI update issue. Will fix it in the next overhaul / PR update. |
|
Thought: Can this be made to work with external wallets/software? |
I don't know what you mean by this. |
|
Needs rebase. What happens with -autorequestblocks=0 -spv=0? I don't know it seems overly complicated. I thought we would just have a single param spvonly that defaults to 0 (equivalent to this autorequestblocks, and your spv is always =1, ie spvonly=0 equivalent to -spv=1 -autorequestblocks=1, spvonly=1 equivalent to -spv=1 -autorequestblocks=0). Not sure, just thinking out loud. |
296635f to
20a3b86
Compare
This is required for features that need to download and process block higher/further-away then the current validation depth
+ Adds a validate=true|false to the SyncTransaction signal
…PC call This allows efficient testing of auxiliary block requests
20a3b86 to
9fab2b2
Compare
|
Rebased.
This would result in a mode where no blocks are automatically requested (only headers are fetched). |
|
Sorry for the very late comment here, but I think this is introducing a lot of complexity and then building on top of it. I think a first step should be what @ryanofsky suggested above ("Get rid of the CAuxiliaryBlockRequest class and integrate prioritized block download logic directly into net_processing.cpp so more code responsible for regular and prioritized block downloads can be shared, and the wallet will not have to be involved in batching and sequencing p2p requests."). This is more generally useful than just lightweight mode too; it could be used for rescanning while pruning too, for example. |
This is actually implemented in #10794. @jonasschnelli, it would probably be good to reference #10794 in the PR description, and make sure the PR description is up to date generally. |
|
@TheBlueMatt pointed out at core dev that the sync implemented here sometimes can't recover from invalid blocks, and that basing this change on #12138 might fix this. |
|
hey, how to run this version locally? thx |
|
I'm also in favor of not using the word SPV in the PR description, which does not imply giving up on the term. Can you clarify this:
Did you mean no blocks older than the wallet? If so, how do you check if the UTXO set is correct? If you don't, then maybe the flag should have a scarier name like @ryanofsky wrote:
My suggestion would be to show the usual IBD progress bar*, as well as mark transactions as not-fully-validated. In that case there's no need for an extra icon.
|
|
|
||
| void CAuxiliaryBlockRequest::processWithPossibleBlock(const std::shared_ptr<const CBlock> pblock, CBlockIndex *pindex) | ||
| { | ||
| // don't process anything if the request was cancled |
There was a problem hiding this comment.
Typo found by codespell: cancled
There was a problem hiding this comment.
This should be fixed throughout in this PR :-)
| this->processedUpToSize++; | ||
|
|
||
| // log some info | ||
| LogPrint("net", "BlockRequest: proccessed up to %ld of total requested %ld blocks\n", this->processedUpToSize, this->vBlocksToDownload.size()); |
There was a problem hiding this comment.
Typo found by codespell: proccessed
| @@ -481,6 +493,27 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<con | |||
| // Make sure pindexBestKnownBlock is up to date, we'll need it. | |||
| ProcessBlockAvailability(nodeid); | |||
|
|
|||
| // if there is an open CAuxiliaryBlockRequest (out-of-band/specific block donwload), privileg it | |||
There was a problem hiding this comment.
Typo found by codespell: donwload annd privileg
| return (mapBlocksInFlight.count(pIndexCheck->GetBlockHash()) == 0); | ||
| }); | ||
|
|
||
| // if we haven't completed the individual CAuxiliaryBlockRequest, we wont continue with "normal" IBD |
There was a problem hiding this comment.
Typo found by codespell: wont
| return; | ||
| } | ||
|
|
||
| // don't request any other blocks if we are in non autorequest mode (usefull for non-validation mode) |
There was a problem hiding this comment.
Typo found by codespell: usefull (also below)
| There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened. |
|
Hi I've always liked the idea of the reference client ie core being available across all platforms. With my work on #16568 proving succeful for android , I'd like to request that an SPV mode be considered seriously for the next major release (0.20). I think ensuring that the core client is accesible across as many platforms (particularly mobile) is a project worth considering. |
|
As we already sync headers , is there a reason to have a separate headers chain ? |
This is the complete patch-set for the hybrid full block SPV mode.
If one enables the SPV mode with
-spv=1it does...validated = false(visible inlisttransactionsetc).Pure full block SPV mode is possible by setting
-autorequestblocks=0, in that mode, no blocks for validating the chain will be downloaded, resulting in a SPV only mode.For better testing, this PR also includes a bump to 0.0005 for the default fallback fee.
Including all required GUI changes and RPC tests:
Screenshots: