more RPC abstraction/encapsulation work
move tallyitem struct definition to wallet.{h,cpp} #2071
pull mikegogulski wants to merge 4 commits into bitcoin:master from mikegogulski:82afcf6b0f6b6f286dca8d5444c43997bb602e1b changing 5 files +61 −58-
mikegogulski commented at 5:36 PM on December 4, 2012: none
-
make WalletTxToJSON into a method of CWalletTx bf484901c9
-
move AcentryToJSON to method of CAccountingEntry af7097b658
-
acda14866a
remove commented-out extern declaration for ValueFromAmount, now inlined
in wallet.h
-
move tallyitem struct definition to wallet.[hc]pp 82afcf6b0f
-
sipa commented at 5:41 PM on December 4, 2012: member
I'm very much in favor of encapsulating the actual logic for many RPC methods to where it belongs. However, that doesn't mean that the conversion to JSON should happen there.
For example, if CWallet and CWalletTx had a cleaner interface for requesting information, this could reduce duplication between RPC and GUI. However, that probably means something a thin layer in between of data structure to represent information extracted from and sent to the wallet. The GUI would inspect these structure and convert them to GUI elements, the RPC would inspent them and convert to JSON.
-
mikegogulski commented at 5:45 PM on December 4, 2012: none
Yeah, I'm going to back out the JSON stuff. My eventual goal for this is to make CWallet a fully-functional first-class object, hide CWalletDB entirely, and have the RPC methods talking strictly to the wallet object. In addition to the "standard" benefits of proper encapsulation, this will eventually allow the client to support, for example, multiple wallets.
- mikegogulski closed this on Dec 4, 2012
-
sipa commented at 5:49 PM on December 4, 2012: member
@mikegogulski Sounds great, I think that's what we want. Note that there are some plans to move to another database backend for the wallets. Just so you don't waste effort on code that's going to be thrown out anyway.
-
BitcoinPullTester commented at 5:52 PM on December 4, 2012: none
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/82afcf6b0f6b6f286dca8d5444c43997bb602e1b for binaries and test log.
-
mikegogulski commented at 6:20 PM on December 4, 2012: none
Got it. I think it should be no problem, since the model I'm stumbling toward is like:
consumer(e.g. bitcoind, bitcoin-qt, others)->walletRPCinterface->walletobject->... ->...walletdbobject->nativedbobject ->...blockchainRPCinterface->blockchainobject->whateverwhere walletRPCinterface is the one and only interface to the wallet object, nothing but the wallet object talks to the wallet database object, and the wallet interacts with the blockchain strictly via a (new) RPC interface.
- HashUnlimited referenced this in commit 54b9aa6782 on Jun 4, 2018
- DrahtBot locked this on Sep 8, 2021