aboutsummaryrefslogtreecommitdiff
path: root/src/rpc
AgeCommit message (Collapse)Author
2020-01-29Merge #16702: p2p: supplying and using asmap to improve IP bucketing in addrmanWladimir J. van der Laan
3c1bc40205a3fcab606e70b0e3c13d68b2860e34 Add extra logging of asmap use and bucketing (Gleb Naumenko) e4658aa8eaf1629dd5af8cf7b9717a8e72028251 Return mapped AS in RPC call getpeerinfo (Gleb Naumenko) ec45646de9e62b3d42c85716bfeb06d8f2b507dc Integrate ASN bucketing in Addrman and add tests (Gleb Naumenko) 8feb4e4b667361bf23344149c01594abebd56fdb Add asmap utility which queries a mapping (Gleb Naumenko) Pull request description: This PR attempts to solve the problem explained in #16599. A particular attack which encouraged us to work on this issue is explained here [[Erebus Attack against Bitcoin Peer-to-Peer Network](https://erebus-attack.comp.nus.edu.sg/)] (by @muoitranduc) Instead of relying on /16 prefix to diversify the connections every node creates, we would instead rely on the (ip -> ASN) mapping, if this mapping is provided. A .map file can be created by every user independently based on a router dump, or provided along with the Bitcoin release. Currently we use the python scripts written by @sipa to create a .map file, which is no larger than 2MB (awesome!). Here I suggest adding a field to peers.dat which would represent a hash of asmap file used while serializing addrman (or 0 for /16 prefix legacy approach). In this case, every time the file is updated (or grouping method changed), all buckets will be re-computed. I believe that alternative selective re-bucketing for only updated ranges would require substantial changes. TODO: - ~~more unit tests~~ - ~~find a way to test the code without including >1 MB mapping file in the repo.~~ - find a way to check that mapping file is not corrupted (checksum?) - comments and separate tests for asmap.cpp - make python code for .map generation public - figure out asmap distribution (?) ~Interesting corner case: I’m using std::hash to compute a fingerprint of asmap, and std::hash returns size_t. I guess if a user updates the OS to 64-bit, then the hash of asap will change? Does it even matter?~ ACKs for top commit: laanwj: re-ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 jamesob: ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 ([`jamesob/ackr/16702.3.naumenkogs.p2p_supplying_and_using`](https://github.com/jamesob/bitcoin/tree/ackr/16702.3.naumenkogs.p2p_supplying_and_using)) jonatack: ACK 3c1bc40205a3fcab606e70b0e3c13d68b2860e34 Tree-SHA512: e2dc6171188d5cdc2ab2c022fa49ed73a14a0acb8ae4c5ffa970172a0365942a249ad3d57e5fb134bc156a3492662c983f74bd21e78d316629dcadf71576800c
2020-01-29Merge #17156: psbt: check that various indexes and amounts are within boundsfanquake
deaa6dd144f5650b385658a0c4f9a014aff8dde2 psbt: check output index is within bounds before accessing (Andrew Chow) f1ef7f0aa46338f4cd8de79696027a1bf868f359 Don't calculate tx fees for PSBTs with invalid money values (Andrew Chow) Pull request description: Fixes #17149 Two classes of issues were found by the psbt fuzzer: values out of range and causing overflows, and prevout indexes being out of range. This PR fixes both. When accessing a specific output using the index given in the tx, check that it is actually a possible output before trying to access the output. When summing and checking amounts for `decodepsbt` and `analyzepsbt`, make sure that the values are actually valid money values.. Otherwise, stop summing and don't show the fee. For `analyzepsbt`, return that the next role is the Creator since the Creator needs to remake the transaction to be valid. ACKs for top commit: practicalswift: ACK deaa6dd144f5650b385658a0c4f9a014aff8dde2 -- only change since last ACK was the addition of tests gwillen: tested ACK deaa6dd, would also like to see this merged! Tree-SHA512: 06c36720bbb5a7ab1c29f7d15878bf9f0d3e5760c06bff479d412e1bf07bb3e0e9ab6cca820a4bfedaab71bfd7af813807e87cbcdf0af25cc3f66a53a06dbcfd
2020-01-23Return mapped AS in RPC call getpeerinfoGleb Naumenko
If ASN bucketing is used, return a corresponding AS used in bucketing for a given peer.
2020-01-22Merge #17754: net: Don't allow resolving of std::string with embedded NUL ↵Wladimir J. van der Laan
characters. Add tests. 7a046cdc1423963bdcbcf9bb98560af61fa90b37 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift) fefb9165f23fe9d10ad092ec31715f906e0d2ee7 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift) 9574de86ad703ad942cdd0eca79f48c0d42b102b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift) Pull request description: Don't allow resolving of `std::string`:s with embedded `NUL` characters. Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface Add tests. The only place in where C-style `NUL`-terminated strings are actually needed is here: ```diff + if (!ValidAsCString(name)) { + return false; + } ... - int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes); + int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); if (nErr) return false; ``` Interface changes: ```diff -bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup); +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup); +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -bool LookupSubNet(const char *pszName, CSubNet& subnet); +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); -CService LookupNumeric(const char *pszName, int portDefault = 0); +CService LookupNumeric(const std::string& name, int portDefault = 0); -bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed); +bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed); ``` It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :) ACKs for top commit: EthanHeilman: ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37 laanwj: ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37 Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba
2020-01-16Fix doxygen errorsBen Woosley
Identified via -Wdocumentation, e.g.: ./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation] * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ ./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'? * @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain ^~~~~~~ prevTxsUnival netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation] * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'? * @param outProxyConnectionFailed[out] Whether or not the connection to the ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ outProxyConnectionFailed
2020-01-15scripted-diff: Bump copyright of files changed in 2020MarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2020-01-15scripted-diff: Replace CCriticalSection with RecursiveMutexMarcoFalke
-BEGIN VERIFY SCRIPT- # Delete outdated alias for RecursiveMutex sed -i -e '/CCriticalSection/d' ./src/sync.h # Replace use of outdated alias with RecursiveMutex sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection) -END VERIFY SCRIPT-
2020-01-08net: Avoid using C-style NUL-terminated strings as arguments in the netbase ↵practicalswift
interface
2020-01-02Merge #17781: rpc: Remove mempool global from minerMarcoFalke
faa92a2297b4a6aebdd58d1818c428f1c0346078 rpc: Remove mempool global from miner (MarcoFalke) 6666ef13f167cfe880c2e94c09d003594d010cf3 test: Properly document blockinfo size in miner_tests (MarcoFalke) Pull request description: The miner needs read-only access to the mempool. Instead of using the mutable global `::mempool`, keep a immutable reference to a mempool that is passed to the miner. Apart from the obvious benefits of removing a global and making things immutable, this might also simplify testing with multiple mempools. ACKs for top commit: promag: ACK faa92a2297b4a6aebdd58d1818c428f1c0346078. fjahr: ACK faa92a2297b4a6aebdd58d1818c428f1c0346078 jnewbery: Code review ACK faa92a2297b4a6aebdd58d1818c428f1c0346078 Tree-SHA512: c44027b5d2217a724791166f3f3112c45110ac1dbb37bdae27148a0657e0d1a1d043b0d24e49fd45465ec014224d1b7eb15c92a33069ad883fa8ffeadc24735b
2019-12-30scripted-diff: Bump copyright of files changed in 2019MarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2019-12-23rpc: Remove mempool global from minerMarcoFalke
2019-12-16Merge #17750: util: change GetWarnings parameter to boolMarcoFalke
7aab8d1024996c7c422bd34a8226df0117b813f7 [style] Code style fixups in GetWarnings() (John Newbery) 492c6dc1e742a62599dc6d5ba6c3896825b5144f util: change GetWarnings parameter to bool (John Newbery) 869b6314fd180856b6054fff28b5de994252c54c [qt] remove unused parameter from getWarnings() (John Newbery) Pull request description: `GetWarnings()` changes the format of the output warning string based on a passed-in string argument that can be set to "gui" or "statusbar". Change the argument to a bool: - there are only two types of behaviour, so a bool is a more natural argument type - changing the name to `verbose` does not set any expectations for the how the calling code will use the returned string (currently, `statusbar` is used for RPC warnings, not a status bar) - removes some error-handling code for when the passed-in string is not one of the two strings expected. ACKs for top commit: laanwj: code review ACK 7aab8d1024996c7c422bd34a8226df0117b813f7 practicalswift: ACK 7aab8d1024996c7c422bd34a8226df0117b813f7 -- diff looks correct :) MarcoFalke: ACK 7aab8d1024996c7c422bd34a8226df0117b813f7 otherwise. promag: Code review ACK 7aab8d1024996c7c422bd34a8226df0117b813f7. Tree-SHA512: 75882c6e3e44aa9586411b803149b36ba487f4eb9cac3f5c8f07cd9f586870bba4488a51e674cf8147f05718534f482836e6a4e3f66e0d4ef6821900c7dfd04e
2019-12-16Merge #17564: rpc: Use mempool from node context instead of globalMarcoFalke
fa8e650b525e9493bdfa393c0c3e34cb22c78c08 rest: Use mempool from node context instead of global (MarcoFalke) fa660d65d7cc401ad5bbfdc076a074de19a79329 node: Use mempool from node context instead of global (MarcoFalke) facbaf092f1ab298943206603cff6e6e3d30d452 rpc: Use mempool from node context instead of global (MarcoFalke) Pull request description: Currently they are identical, but in the future we might want to turn the mempool into a unique_ptr. Replacing the global with the mempool pointer from the node context simplifies this step. ACKs for top commit: jnewbery: Code review ACK fa8e650b5 ryanofsky: Code review ACK fa8e650b525e9493bdfa393c0c3e34cb22c78c08, Only the discussed REST server changes since the last review. Tree-SHA512: 0836f3f39cf90306455962918446e5f8612e88c32072b92afc30929aea1f17430bbda0e2b3668d36c9d6b97d63a93cf4903185194571108642b7bf5a39b89125
2019-12-15util: change GetWarnings parameter to boolJohn Newbery
GetWarnings() changes the format of the output warning string based on a passed-in string argument that can be set to "gui" or "statusbar". Change the argument to a bool: - there are only two types of behaviour, so a bool is a more natural argument type - changing the name to 'verbose' does not set any expectations for the how the calling code will use the returned string (currently, 'statusbar' is used for RPC warnings, not a status bar) - removes some error-handling code for when the passed-in string is not one of the two strings expected.
2019-12-15Merge #17728: rpc: require second argument only for scantxoutset start actionWladimir J. van der Laan
7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b rpc: require second argument only for scantxoutset start action (Andrew Chow) Pull request description: It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0: ``` <belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18 <belcher> im on regtest, in case its important <harding> I can confirm `scantxoutset abort` returns the help doc on latest master. Waiting for 0.18.1 to start now to attempt to reproduce there. <harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort"). <jonatack> Same for me as well <harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1. ``` As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them. It appears that this was broken by #16240 which enforced the size of the arguments to match the listed required arguments. To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case. ACKs for top commit: laanwj: ACK 7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b promag: ACK 7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b. Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
2019-12-13Merge #17617: doc: unify unix epoch time descriptionsWladimir J. van der Laan
d94d34f05f4ae3efa07de409489d68bbcc216346 doc: update developer notes wrt unix epoch time (Jon Atack) e2f32cb5c5c7f2b1d1fc7003587b6573fb59526a qa: unify unix epoch time descriptions (Jon Atack) Pull request description: Closes #17613. Updated call sites: mocktime, getblockheader, getblock, pruneblockchain, getchaintxstats, getblocktemplate, setmocktime, getpeerinfo, setban, getnodeaddresses, getrawtransaction, importmulti, listtransactions, listsinceblock, gettransaction, getwalletinfo, getaddressinfo Commands for testing manually: ``` bitcoind -help-debug | grep -A1 mocktime bitcoin-cli help getblockheader bitcoin-cli help getblock bitcoin-cli help pruneblockchain bitcoin-cli help getchaintxstats bitcoin-cli help getblocktemplate bitcoin-cli help setmocktime bitcoin-cli help getpeerinfo bitcoin-cli help setban bitcoin-cli help getnodeaddresses bitcoin-cli help getrawtransaction bitcoin-cli help importmulti bitcoin-cli help listtransactions bitcoin-cli help listsinceblock bitcoin-cli help gettransaction bitcoin-cli help getwalletinfo bitcoin-cli help getaddressinfo ``` ACKs for top commit: laanwj: re-ACK d94d34f05f4ae3efa07de409489d68bbcc216346 Tree-SHA512: 060713ea4e20ab72c580f06c5c7e3ef344ad9c2c9cb034987d980a54e3ed2ac0268eb3929806daa5caa7797c45f5305254fd499767db7f22862212cf77acf236
2019-12-13qa: unify unix epoch time descriptionsJon Atack
to "UNIX epoch time". Call sites updated: ``` mocktime getblockheader getblock pruneblockchain getchaintxstats getblocktemplate setmocktime getpeerinfo setban getnodeaddresses getrawtransaction importmulti listtransactions listsinceblock gettransaction getwalletinfo getaddressinfo ```
2019-12-11rpc: require second argument only for scantxoutset start actionAndrew Chow
The second argument of scanobjects is only required for the start action. Stop and abort actions do not need this.
2019-12-10Don't calculate tx fees for PSBTs with invalid money valuesAndrew Chow
In decodepsbt if an invalid amount is seen, don't calculate the fee but still show the invalid value in the decode. In analyze psbt, if an invalid amount is seen, set the next step to be the creator as the creator needs to remake the transaction so that it is valid.
2019-12-10rpc: add missing newline in analyzepsbt rpcresultJon Atack
follow-up to 638e40c
2019-12-10Merge #17524: psbt: handle unspendable psbtsMarcoFalke
773d4572a4864ab7b6380858d07d9579ff6dd9a2 Mark PSBTs spending unspendable outputs as invalid in analysis (Andrew Chow) 638e40cb6080800c7b0a7f4028f63326acbe4700 Have a PSBTAnalysis state that indicates invalid PSBT (Andrew Chow) Pull request description: When analyzing an unspendable PSBT, report that it is unspendable and exit analysis early. ACKs for top commit: Sjors: ACK 773d457 instagibbs: After some thought ACK https://github.com/bitcoin/bitcoin/commit/773d4572a4864ab7b6380858d07d9579ff6dd9a2 Tree-SHA512: 99b0cb2fa1ea37593fc65a20effe881639d69ddeeecf5197bc87bc7f2220cbeb40f1d429d517e4d27f2e9fb563a00cd845d2b4b1ce05246a75a6cb56fb9b0ba5
2019-12-05node: Use mempool from node context instead of globalMarcoFalke
2019-12-05rpc: Use mempool from node context instead of globalMarcoFalke
Currently they are identical, but in the future we might want to turn the mempool into a unique_ptr. Replacing the global with the mempool pointer from the node context simplifies this step.
2019-11-24wallet: replace raw pointer with const reference in AddrToPubKeyHarris
2019-11-23Merge #17371: Refactor: Require scriptPubKey to get wallet SigningProviderSamuel Dobson
d0dab897afaac0a18aa47d3ce673a4a43a69178a Refactor: Require scriptPubKey to get wallet SigningProvider (Andrew Chow) 4b0c718f8f48c678cbe4575e9a9cf9e62a30f0da Accumulate result UniValue in SignTransaction (Andrew Chow) Pull request description: Easier to review ignoring whitespace: git log -p -n1 -w This commit does not change behavior. It passes new CScript arguments to signing functions, but the arguments aren't currently used. Split from #17261 ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/17371/commits/d0dab897afaac0a18aa47d3ce673a4a43a69178a ryanofsky: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a. Thanks for the SignTransaction update. No other changes since last review Sjors: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a promag: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a. meshcollider: Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a Tree-SHA512: c3f52df20fd9d6b3b5aa65562cf5f7dce7b7f44c148b0f988f8b578fce2a28e9b7bf010f5f04bb5bf60f5272b2899f1dbbfb8aee81579c21c9cba559d1d2bb70
2019-11-22Merge #17519: rpc: Remove unused COINBASE_FLAGSWladimir J. van der Laan
e9a27cf338dc618b8ecab8984abc54d588de8a05 refactor: Remove unused COINBASE_FLAGS (Neha Narula) Pull request description: Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting COINBASE_FLAGS, and it looks like it hasn't been used since P2SH. Following up on #17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig. ACKs for top commit: laanwj: ACK e9a27cf338dc618b8ecab8984abc54d588de8a05 MarcoFalke: ACK e9a27cf338dc618b8ecab8984abc54d588de8a05 💻 Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
2019-11-20refactor: Remove unused COINBASE_FLAGSNeha Narula
Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting COINBASE_FLAGS, and it looks like it hasn't been used since P2SH. Update the help string to remove "flags", which is not specified in BIP 22.
2019-11-19Have a PSBTAnalysis state that indicates invalid PSBTAndrew Chow
Invalid PSBTs need to be re-created, so the next role is the Creator (new PSBTRole). Additionally, we need to know what went wrong so an error field was added to PSBTAnalysis. A PSBTAnalysis indicating invalid will have empty everything, next will be set to PSBTRole::CREATOR, and an error message.
2019-11-18Accumulate result UniValue in SignTransactionAndrew Chow
SignTransaction will be called multiple times in the future. Pass it a result UniValue so that it can accumulate the results of multiple SignTransaction passes.
2019-11-15node: Add reference to mempool in NodeContextMarcoFalke
Currently it is an alias to the global ::mempool and should be used as follows. * Node code (validation and transaction relay) can use either ::mempool or node.mempool, whichever seems a better fit. * RPC code should use the added convenience getter EnsureMempool, which makes sure the mempool exists before use. This prepares the RPC code to a future where the mempool might be disabled at runtime or compile time. * Test code should use m_node.mempool directly, as the mempool is always initialized for tests.
2019-11-06Merge #17382: rpc: Remove unused boost::this_thread::interruption_pointWladimir J. van der Laan
fa5facd3e72b6d61374b0b93b722b55e2b090020 rpc: Remove unused boost::this_thread::interruption_point (MarcoFalke) Pull request description: There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points However, the rpc threads are `std::thread`, which does not have an `std::thread::interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34 Thus, the interruption points can be removed. ACKs for top commit: laanwj: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020, this does nothing. practicalswift: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/17382/commits/fa5facd3e72b6d61374b0b93b722b55e2b090020 Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
2019-11-05rpc: Remove unused boost::this_thread::interruption_pointMarcoFalke
2019-11-05Merge #16899: UTXO snapshot creation (dumptxoutset)Wladimir J. van der Laan
92b2f5306ba0b3f031293cb8f415b67cb002c2f1 test: add dumptxoutset RPC test (James O'Beirne) c1ccbc3ddef931896a7e9dcfa6704e305a69fbff devtools: add utxo_snapshot.sh (James O'Beirne) 57cf74c9918d10c69a46e6ceb3cb1a5e04edf5bc rpc: add dumptxoutset (James O'Beirne) 92fafb3a7da66f737e960e541fcfbcadedf6043a coinstats: add coins_count (James O'Beirne) 707fde7b9ba522c22179e2db0ed7b462c65138d9 add unused SnapshotMetadata class (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`. All of this is unused at the moment. ACKs for top commit: laanwj: ACK 92b2f5306ba0b3f031293cb8f415b67cb002c2f1 Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31
2019-11-05rpc: add dumptxoutsetJames O'Beirne
Allows the creation of a UTXO snapshot to disk.
2019-11-04Merge #17318: replace asserts in RPC code with CHECK_NONFATAL and add linterMarcoFalke
c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 replace asserts in RPC code with CHECK_NONFATAL and add linter (Adam Jonas) Pull request description: - Replace instances of assert in /rpc files and rpcwallet with CHECK_NONFATAL(condition) - Add a linter to prevent future usage of assert being used in RPC code ref https://github.com/bitcoin/bitcoin/pull/17192 ACKs for top commit: practicalswift: ACK c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 -- diff looks correct Tree-SHA512: a16036b6bbcca73a5334665f66e17e1756377d582317568291da1d727fc9cf8c84bac9d9bd099534e1be315345336e5f7b66b93793135155f320dc5862a2d875
2019-11-04Merge #17351: doc: Fix some misspellingsMarcoFalke
ac831339cbfa65b1f7576c53b5d9a94841db9868 doc: Fix some misspellings (randymcmillan) Pull request description: Here is a more thorough lint-spelling update. This PR takes care of easy to fix spelling errors to clean up the linting stages. There are misspellings coded into the functional tests. That is a whole separate job within itself. ACKs for top commit: practicalswift: ACK ac831339cbfa65b1f7576c53b5d9a94841db9868 -- diff looks correct Tree-SHA512: d8fad83fed083715655f148263ddeffc6752c8007d568fcf3dc2c418ccd5db70089ce3ccfd3994fcbd78043171402eb9cca5bdd5125287e22c42ea305aaa6e9d
2019-11-04Merge #17297: refactor: Remove addrdb.h dependency from node.hWladimir J. van der Laan
f44abe4bed25a40145ab168adc1589f5df4146f3 refactor: Remove addrdb.h dependency from node.h (Hennadii Stepanov) Pull request description: `node.h` includes `addrdb.h` just for the sake of `banmap_t` type. This PR makes dependencies simpler and explicit. ~Also needless `typedef` has been removed from `enum BanReason`.~ ACKs for top commit: laanwj: ACK f44abe4bed25a40145ab168adc1589f5df4146f3 practicalswift: ACK f44abe4bed25a40145ab168adc1589f5df4146f3 Tree-SHA512: 33a1be20e5c629daf4a61ebbf93ea6494b9256887cebd4974de4782f6d324404b6cc84909533d9502b2cc19902083f1f9307d4fb7231e67db5b412b842d13072
2019-11-04doc: Fix some misspellingsrandymcmillan
2019-11-02Merge #17285: doc: Bip70 removal follow-upWladimir J. van der Laan
3ed8e3d079a3860dcdf944f7c1aa37765a53da32 doc: Remove explicit network name references (Fabian Jahr) d6e493f0c2850b522a676a005935163beddaa2cc wallet: Remove left-over BIP70 comment (Fabian Jahr) Pull request description: A small follow-up to #17165 which removed BIP70 support. 1. Removes one leftover mention of BIP70 in a comment. 2. Removes BIP70 reference in comments on network/chain name strings. These can be removed as they are not really helpful and also incorrect: BIP70 only defines "main" and "test" but not "regtest". If/When signet gets merged we will add another name to the list that is not defined in BIP70. Mostly there is also an exhaustive list of the options included in the comment anyway. If we would like to keep an identifier for this naming scheme, I would suggest switching to something more generic, like 'short chain name'. Happy to implement that if that is preferred. Alternatively, we could add a reference to `CBaseChainParams`. That would also mean we don't have to change these lines again for signet. ACKs for top commit: MarcoFalke: ACK 3ed8e3d079a3860dcdf944f7c1aa37765a53da32 Tree-SHA512: 9a7c0b9cacbb67bd31a089ffdc6f1ebc7f336493e2c8266eb697da34dce2b505a431d5639a3e4fc34f9287361343e861b55dc2662e0a1d2095cc1046db77d6ee
2019-11-01doc: Remove explicit network name referencesFabian Jahr
2019-10-30replace asserts in RPC code with CHECK_NONFATAL and add linterAdam Jonas
2019-10-30Merge #15921: validation: Tidy up ValidationState interfaceWladimir J. van der Laan
3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf [validation] Remove fMissingInputs from AcceptToMemoryPool() (John Newbery) c428622a5bb1e37b2e6ab2c52791ac05d9271238 [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() (John Newbery) 7204c6434b944f6ad51b3c895837729d3aa56eea [validation] Remove useless ret parameter from Invalid() (John Newbery) 1a37de4b3174d19a6d8691ae07e92b32fdfaef11 [validation] Remove error() calls from Invalid() calls (John Newbery) 067981e49246822421a7bcc720491427e1dba8a3 [validation] Tidy Up ValidationResult class (John Newbery) a27a2957ed9afbe5a96caa5f0f4cbec730d27460 [validation] Add CValidationState subclasses (John Newbery) Pull request description: Carries out some remaining tidy-ups remaining after PR 15141: - split ValidationState into TxValidationState and BlockValidationState (commit from ajtowns) - various minor code style tidy-ups to the ValidationState class - remove the useless `ret` parameter from `ValidationState::Invalid()` - remove the now unused `first_invalid` parameter from `ProcessNewBlockHeaders()` - remove the `fMissingInputs` parameter from `AcceptToMemoryPool()`, and deal with missing inputs the same way as other errors by using the `TxValidationState` object. Tip for reviewers (thanks ryanofsky!): The first commit ("[validation] Add CValidationState subclasses" ) is huge and can be easier to start reviewing if you revert the rote, mechanical changes: Substitute the commit hash of commit "[validation] Add CValidationState subclasses" for <CommitHash> in the commands below. ```sh git checkout <CommitHash> git grep -l ValidationState | xargs sed -i 's/BlockValidationState\|TxValidationState/CValidationState/g' git grep -l ValidationResult | xargs sed -i 's/BlockValidationResult\|TxValidationResult/ValidationInvalidReason/g' git grep -l MaybePunish | xargs sed -i 's/MaybePunishNode\(ForBlock\|ForTx\)/MaybePunishNode/g' git diff HEAD^ ``` After that it's possible to easily see the mechanical changes with: ```sh git log -p -n1 -U0 --word-diff-regex=. <CommitHash> ``` ACKs for top commit: laanwj: ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf amitiuttarwar: code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Also built & ran tests locally. fjahr: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf . Only nit style change and pure virtual destructor added since my last review. ryanofsky: Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Just whitespace change and pure virtual destructor added since last review. Tree-SHA512: 511de1fb380a18bec1944ea82b513b6192df632ee08bb16344a2df3c40811a88f3872f04df24bc93a41643c96c48f376a04551840fd804a961490d6c702c3d36
2019-10-30Merge #16943: test: Add generatetodescriptor RPCWladimir J. van der Laan
fa144e6fde1f546a952023af715032cb6789d948 rpc: Add generatetodescriptor (MarcoFalke) Pull request description: The existing `generatetoaddress` RPC can only generate to scriptPubKeys that can be represented by an address. However, raw scripts (such as `OP_TRUE`) or P2PK can not be represented by an address, which complicates testing. ACKs for top commit: laanwj: ACK fa144e6fde1f546a952023af715032cb6789d948 Tree-SHA512: aee934ab7e33f07c81f3b4c8ec23e7b6ddf63a1f4b86051af0bd76b75d8da1f51627cc682e5c6e42582340ca576bbf8ff724bdd43f87128ccecfa91e52d30ae7
2019-10-30rpc: Add generatetodescriptorMarcoFalke
2019-10-30Merge #16839: Replace Connman and BanMan globals with NodeContext localWladimir J. van der Laan
362ded410b8cb1104b7ef31ff8488fec4824a7d5 Avoid using g_rpc_node global in wallet code (Russell Yanofsky) 8922d7f6b751a3e6b3b9f6fb7961c442877fb65a scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky) e6f4f895d5e42feaf7bfa5f41e80292aaa73cd7d Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky) 4d5448c76b71c9d91399c31b043237091be2e5e7 MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky) 301bd41a2e6765b185bd55f4c541f9e27aeea29d scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky) Pull request description: This change is mainly a naming / organization change intended to simplify #10102. It: - Renames struct InitInterfaces to struct NodeContext and moves it from src/init.h to src/node/context.h. This is a cosmetic change intended to make the point of the struct more obvious. - Gets rid of BanMan and ConnMan globals making them NodeContext members instead. Getting rid of these globals has been talked about in past as a way to implement testing and simulations. Making them NodeContext members is a way of keeping them accessible without the globals. - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This better separates node and wallet rpc methods. Node RPC methods should have access NodeContext, while wallet RPC methods should only have indirect access to node functionality via interfaces::Chain. - Adds NodeContext& references to interfaces::Chain class and the interfaces::MakeChain() function. This is needed to access ConnMan and BanMan instances without the globals. - Gets rid of redundant Node and Chain instances in Qt tests. This is needed due to the previous MakeChain change, and also makes test setup a little more straightforward. More cleanup could be done in the future, but it will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init code. ACKs for top commit: laanwj: ACK 362ded410b8cb1104b7ef31ff8488fec4824a7d5 Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2
2019-10-29[validation] Remove fMissingInputs from AcceptToMemoryPool()John Newbery
Handle this failure in the same way as all other failures: call Invalid() with the reasons for the failure.
2019-10-29[validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders()John Newbery
No callers use the returned value in first_invalid. Remove it from the function signature and don't set it in the function.
2019-10-29[validation] Add CValidationState subclassesJohn Newbery
Split CValidationState into TxValidationState and BlockValidationState to store validation results for transactions and blocks respectively.
2019-10-29refactor: Remove addrdb.h dependency from node.hHennadii Stepanov
2019-10-28Avoid using g_rpc_node global in wallet codeRussell Yanofsky
Wallet code should use interfaces::Chain and not directly access to node state. Add a g_rpc_chain replacement global for wallet code to use, and move g_rpc_node definition to a libbitcoin_server source file so there are link errors if wallet code tries to access it.