CHub Rev. 3 #1408
CHub Rev. 3 #1408
Conversation
| // any futher uses are not yet supported. | ||
| // This API is subject to change dramatically overnight, do not | ||
| // depend on it for anything. | ||
|
|
There was a problem hiding this comment.
What is the big-picture purpose of CBlockStore ?
Is it for storing blocks in the database?
Being told about new blocks and keeping track of the best chain?
Both? Something else?
And what's up with ProcessCallbacks-- I'm not seeing why that's necessary, or why BlockStore would expose threads, but maybe that's because I don't understand the big-picture purpose/design.
There was a problem hiding this comment.
The purpose of CBlockStore is to act as a hub (oops, Im gonna rename it to CHub) between p2p, wallet and the block storage. The block storage (including being told about new blocks, holding the chain (or not, if SPV)) will be hidden behind CHub, as seen by p2p and wallet. A CHub can then callback to a CBlockStore/CSPVBlockStore/etc. Eventually, you could even run p2p and wallet clients in separate processes for additional security, that is the long-long-long term goal.
In terms of exposing threads, that is more for added functionality later which cleans up the API a bit (when you need to run n regular threads, and possibly multiple callback threads).
|
Updated with all the feedback so far. Any further work (aside from bug-fixes) will be done on other branches, so this branch is ready for review. Take special note of the last commit (and its very long commit message). |
| @@ -57,7 +58,7 @@ class CKeyPool | |||
| /** A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, | |||
| * and provides the ability to create new transactions. | |||
| */ | |||
| class CWallet : public CCryptoKeyStore | |||
| class CWallet : public CCryptoKeyStore, public CHubListener | |||
There was a problem hiding this comment.
I suppose CHubListener could be derived protected. No class except CWallet should be able to call RegisterWithHub and related functions. The callback functions in CHubListener can be made protected directly I suppose - they are only intended to be overridden, not to be called by anyone except via CHub's signals.
There was a problem hiding this comment.
Currently pwalletMain->RegisterWithHub(phub) is called from init.cpp, and I kinda like that design. That said, I did go ahead and make the callback virtual functions protected.
The goal is for p2p code/wallet to only get information/communicate information about the blockchain through CHub, giving Bitcoin a much more clearly-defined structure and allowing for the removal of a ton of the current global mess.
Replacing ProcessBlock with EmitBlock, and creating callbacks for CommitBlock.
Also rename NotifyBlocksChanged to NotifyNewBlock and remove from the uiInterface signals list. This removes some functionality, but NofiyBlocksChanged was not used anyway, so it shouldn't matter. That said, if it is ever needed, it would be fairly trivial to add a new callback for it in CHub.
Replace ProcessAlert with calls to EmitAlert and create callbacks for CommitAlert.
There was no reason to use CDataStream as the transaction was already being serialized/deserialized several times, with this change, transactions coming in over network are deserialized once when received, and then only reserialized in the call to RelayMessage, which will be called in a callback thread, not blocking cs_main.
* This removes not only CTransaction::AcceptToMemoryPool, but
also CMerkleTx::AcceptToMemoryPool. It also moves
CWalletTx::AcceptWalletTransaction to wallet.cpp
* This adds a fCheckInputs flag to EmitTransaction, which is
similar to the fCheckInputs flag to AcceptToMemoryPool,
however, it has stricter guidlines that it should only be set
"when transaction is a supporting tx for one of our own."
Additionally, "fCheckInputs is ignored (and set to true)
if !IsInitialBlockDownload() && !fClient"
As a part of these guidelines,
CWalletTx::AcceptWalletTransaction calls EmitTransaction with
fCheckInputs set to true (the default) on the final
transaction, whereas it used to call with fCheckInputs set to
false. This has the important side-effect of allowing wallet-
generated transactions to end up getting AddOrphanTx'd.
However, if a supporting transaction to one of our own had
previously been AddOrphanTx'd, it would immediately be added
to memory pool as it is "a supporting tx for one of our own"
and thus is re-added with fCheckInputs=false.
Note that the possibility of a wallet transaction getting
AddOrphanTx'd is very low, and should only happen if
a) a transaction's input is a generate and we are missing that
block (note that no transactions should be generated with a
generation input if we don't have that block anyway).
b) We match the !IsInitialBlockDownload() && !fClient check,
are not caught up to the latest block, and an input is in a
block we do not yet have (possible after the last
checkpoint). This situation is temporary and should resolve
itself once we catch up (though AddOrphanTx'd transactions
may be permanently orphaned).
Largely, these guidelines are there because there is no reason
to add a transaction without checking its inputs, as we have
those inputs available, and checking them as any other
transaction would provides additional sanity-checks.
* A second EmitTransaction was added with tx of type CMerkleTx.
This keeps behavior of CMerkleTx::AcceptToMemoryPool the same
in fClient mode. Note that new behavior was invented for
CHub::EmitTransaction(CTransaction&...) in fClient mode,
namely that ClientConnectInputs is only checked if
fCheckInputs is true. This was chosen to make emitting a
transaction possible in fClient mode even if its inputs are
not available, but could be changed if support for that is not
needed when fClient mode is actually implemented.
|
This needs rebasing, and Im not going to keep rebasing this stuff without any interest in eventually merging. If it ever gets interest, I may reopen. |
* PS should limit entry size, not mixing amount * There should be no fee in mixing tx * make sure pwalletMain is not null in PrepareDenominate * no need for "double" in GetAverageAnonymizedRounds, "float" should be enough * add strErrorRet
[PORT] depends: make osx output deterministic
c49bc29 [Model] Save cache BlockIndex tip in the model (random-zebra) Pull request description: **Probelm**: GUI client model requires calls to `chainActive` (thus locking the main UI thread) for the following functions: - getNumBlocks - getLastBlockDate - getLastBlockHash - getVerificationProgress **Fix**: use the `NotifyBlockTip` core signal to update a cached tip blockindex in `BlockTipChanged`, and rely on this object for the aforementioned functions. **Note**: This should allow to complete bitcoin#1047 ACKs for top commit: furszy: nice, tested ACK c49bc29. Fuzzbawls: ACK c49bc29 Tree-SHA512: 83eb3f930d3550b1d0fc46aec3812e8cd8c57d5d7a047b42e1fff22c6cf4949cf34f70b5c3a562776e4f26182d6a1f81576a011ba4fafb4efbd914a029adf2b7
Due to several changes in master that would require rethinking decisions made in the second revision of CBlockStore (#771) and thus very large code changes to rebase, I decided it was better to redo and manually rebase and re-commit the parts that remain relevant, also decreasing the total size of the changes to just those required to get CHub working on master. Some of the more disturbing changes can be added later after merge.
This is early in the process, but I thought I'd get it in the queue and let people review the commits as they come in and give feedback, if they wish.