aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2019-10-09Merge #15437: p2p: Remove BIP61 reject messagesWladimir J. van der Laan
fa25f43ac5692082dba3f90456c501eb08f1b75c p2p: Remove BIP61 reject messages (MarcoFalke) Pull request description: Reject messages (BIP 61) appear in the following settings: * Parsing of reject messages (in case `-debug=net` is set, off by default). This has only been used for a single `LogPrint` call for several releases now. Such logging is completely meaningless to us and should thus be removed. * The sending of reject messages (in case `-enablebip61` is set, off by default). This can be used to debug a node that is under our control. Instead of hacking this debugging into the p2p protocol, it could be more easily achieved by parsing the debug log. (Use `-printtoconsole` to have it as stream, or read from the `debug.log` file like our python function `assert_debug_log` in the test framework does) Having to maintain all of this logic and code to accommodate debugging, which can be achieved by other means a lot easier, is a burden. It makes review on net processing changes a lot harder, since the reject message logic has to be carried around without introducing any errors or DOS vectors. ACKs for top commit: jnewbery: utACK fa25f43ac5692082dba3f90456c501eb08f1b75c laanwj: I'm still not 100% convinced that I like getting rid of BIP61 conceptually, but apparently everyone wants it, code review ACK fa25f43ac5692082dba3f90456c501eb08f1b75c. ryanofsky: Code review ACK fa25f43ac5692082dba3f90456c501eb08f1b75c Tree-SHA512: daf55254202925e56be3d6cfb3c1c804e7a82cecb1dd1e5bd7b472bae989fd68ac4f21ec53fc46751353056fd645f7f877bebcb0b40920257991423a3d99e0be
2019-10-08Merge #17056: descriptors: Introduce sortedmulti descriptorfanquake
4bb660be90a2811b53855bf1fd33a8dd9ba3db47 Add release note (Andrew Chow) ed96b295d747738334459490c79b7360ab85aaf7 Update descriptors.md to include sortedmulti (Andrew Chow) 80be78ea75ac9833ee3db3d468ed09fc4fe6274c Test sortedmulti descriptor using BIP 67 tests (Andrew Chow) 6f588fd2276e5b713c6d36e3b01288484ddb59c0 Add sortedmulti descriptor and unit tests (Andrew Chow) Pull request description: Adds a `sortedmulti()` descriptor as mentioned in https://github.com/bitcoin/bitcoin/pull/17023#issuecomment-537596416. `sortedmulti()` works in the same way as `multi` does but sorts the pubkeys in the resulting scripts in lexicographic order as described in [BIP67](https://github.com/bitcoin/bips/blob/master/bip-0067.mediawiki). Note that this does not add support for BIP67 nor is BIP67 fully supported by this descriptor (which is why it is not named `multi67()`) as it does not require compressed pubkeys. Tests from BIP67 were added and documentation was updated. ACKs for top commit: instagibbs: re-ACK https://github.com/bitcoin/bitcoin/pull/17056/commits/4bb660be90a2811b53855bf1fd33a8dd9ba3db47 Sjors: re-ACK 4bb660be90a2811b53855bf1fd33a8dd9ba3db47 Tree-SHA512: 93b21112a74ebe0bf316d8f3e0291f69fd975cf0a29332f9728e7b880cad312b8b14007e86adcd7899f117b9303cbcf4cb35f3bb2f2f648d1a446f83f75a70a5
2019-10-08Add sortedmulti descriptor and unit testsAndrew Chow
2019-10-07tests: Pass fuzzing inputs as constant referencespracticalswift
2019-10-02p2p: Remove BIP61 reject messagesMarcoFalke
2019-09-30Merge #16931: test: add unittests for CheckProofOfWorkWladimir J. van der Laan
0cc7dd74e0af735dddf7e786f4ed136c382a4ad5 test: add unittests for CheckProofOfWork (soroosh-sdi) Pull request description: following situations are covered: - negative target - overflow target - target easier then powLimit - invalid hash (hash > target) Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com> ACKs for top commit: promag: ACK 0cc7dd74e0af735dddf7e786f4ed136c382a4ad5, just read the code. laanwj: ACK 0cc7dd74e0af735dddf7e786f4ed136c382a4ad5 Tree-SHA512: 9f9ee952ebb211202939450aa3d61b3c2fae992dcfcab085e877507d78e02ea39a51ccacfc4852a0555f3cba07504ee132abd5cbfed75489553bee45c760bc7e
2019-09-30Merge #16953: doc: Improve test READMEsWladimir J. van der Laan
43e7d576f590e90ad7d1ba3d550671a7958f1188 doc: Improve test READMEs (Fabian Jahr) Pull request description: General improvements on READMEs for unit tests and functional tests: - Give unit test readme a headline - Move general information on `src/test` folder to the top - Add information on logging and debugging unit tests - Improve debugging and logging information in functional testing - Include all available log levels in functional tests ACKs for top commit: laanwj: ACK 43e7d576f590e90ad7d1ba3d550671a7958f1188 Tree-SHA512: 22b27644992ba5d99a885cd51b7a474806714396fcea1fd2d6285e41bdf3b28835ad8c81449099e3ee15a63d57b3ab9acb89c425d9855ed1d9b4af21db35ab03
2019-09-27test: add unittests for CheckProofOfWorksoroosh-sdi
following situations are covered: - negative target - overflow target - target easier then powLimit - invalid hash (hash > target) - zero target Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
2019-09-26doc: Improve test READMEsFabian Jahr
2019-09-26Merge #16577: util: CBufferedFile fixes and unit testWladimir J. van der Laan
efd2474d17098c754367b844ec646ebececc7c74 util: CBufferedFile fixes (Larry Ruane) Pull request description: The `CBufferedFile` object guarantees its user is able to "rewind" the data stream (that's being read from a file) up to a certain number of bytes, as specified by the user in the constructor. This guarantee is not honored due to a bug in the `SetPos` method. Such rewinding is done in `LoadExternalBlockFile()` (currently the only user of this object), which deserializes a series of `CBlock` objects. If that function encounters something unexpected in the data stream, which is coming from a `blocks/blk00???.dat` file, it "rewinds" to an earlier position in the stream to try to get in sync again. The `CBufferedFile` object does not actually rewind its file offset; it simply repositions its internal offset, `nReadPos`, to an earlier position within the object's private buffer; this is why there's a limit to how far the user may rewind. If `LoadExternalBlockFile()` needs to rewind (call `blkdat.SetPos()`), the stream may not be positioned as it should be, causing errors in deserialization. This need to rewind is probably rare, which is likely why this bug hasn't been noticed already. But if this object is used elsewhere in the future, this could be a serious problem, especially as, due to the nature of the bug, the `SetPos()` _sometimes_ works. This PR adds a unit test for `CBufferedFile` that fails due to this bug. (Until now it has had no unit tests.) The unit test provides good documentation and examples for developers trying to understand `LoadExternalBlockFile()` and for future users of this object. This PR also adds code to throw an exception from the constructor if the rewind argument is not less than the buffer size (since that doesn't make any sense). Finally, I discovered that the object is too restrictive in one respect: When the deserialization methods call this object's `read` method, a check ensures that the number of bytes being requested is less than the size of the buffer (adjusting for the rewind size), else it throws an exception. This restriction is unnecessary; the object being deserialized can be larger than the buffer because multiple reads from disk can satisfy the request. ACKs for top commit: laanwj: ACK ~after squash.~ efd2474d17098c754367b844ec646ebececc7c74 mzumsande: I had intended to follow up earlier on my last comment, ACK efd2474d17098c754367b844ec646ebececc7c74. I reviewed the code, ran tests and did a successful reindex on testnet with this branch. Tree-SHA512: 695529e0af38bae2af4e0cc2895dda56a71b9059c3de04d32e09c0165a50f6aacee499f2042156ab5eaa6f0349bab6bcca4ef9f6f9ded4e60d4483beab7e4554
2019-09-18Merge #14696: qa: Add explicit references to related CVE's in ↵Wladimir J. van der Laan
p2p_invalid_block test. 0c62e3aa73839e97e65a3155e06a98d84b700a1e New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. (lucash-dev) 38bfca6bb2ad68719415e9c54a981441052da072 Added comments referencing multiple CVEs in tests and production code. (lucash-dev) Pull request description: This functional test includes two scenarios that test for regressions of vulnerabilities, but they are only briefly described. There are freely available documents explaining in detail the issues, but without explicit mentions, the developer trying to maintain the code needs an additional step of digging in commit history and PR conversations to figure it out. Added comments to explicitly mention CVE-2018-17144 and CVE-2012-2459, for more complete documentation. This improves developer experience by making understanding the tests easier. ACKs for top commit: laanwj: ACK 0c62e3aa73839e97e65a3155e06a98d84b700a1e, checked the CVE numbers, thanks for adding documentation Tree-SHA512: 3ee05351745193b8b959e4a25d50f25a693b2d24b0732ed53cf7d5882df40b5dd0f1877bd5c69cffb921d4a7acf9deb3cc1160b96dc730d9b5984151ad06b7c9
2019-09-17Merge #16865: test: add some unit tests for merkle.cppMarcoFalke
8573429d46f1e014663695e95109c10dfe4d54c9 test: add some unit tests for merkle.cpp (soroosh-sdi) Pull request description: following situations are covered: - empty block - one Tx - Merkle root of a block with odd Txs should not change with repeating last one - Merkle root is computed with combining Merkle root of left subtree and right subtree - block witness is Merkle root of a block when setting first Tx to zero. Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com> ACKs for top commit: laanwj: ACK 8573429d46f1e014663695e95109c10dfe4d54c9 Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
2019-09-16test: add some unit tests for merkle.cppsoroosh-sdi
following situations are covered: - empty block - one Tx - Merkle root of a block with odd Txs should not change with repeating last one - Merkle root is computed with combining Merkle root of left subtree and right subtree - block witness is Merkle root of a block when setting first Tx to zero. Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
2019-09-10util: CBufferedFile fixesLarry Ruane
2019-09-10Merge #16680: Preparations for more testchainsMarcoFalke
3bf9d8cac09fc88727ef2f2a2bea33b90b625e50 Testchains: Qt: Simplify network/chain styles (Jorge Timón) 052c54ecb02695e5d2694e8e0cbf5ccc89de86e8 Testchains: Generic selection with -chain=<str> in addition of -testnet and -regtest (Jorge Timón) Pull request description: Separated from #8994 as suggested by MarcoFalke and Sjors in https://github.com/bitcoin/bitcoin/pull/8994#issuecomment-522555390 You can't really test the qt changes on their own, so to test them, use #8994 . ACKs for top commit: MarcoFalke: ACK 3bf9d8cac09fc88727ef2f2a2bea33b90b625e50 Tree-SHA512: 5b5e6083ebc0a44505a507fac633e7af18037c85e5e73f5d1e6f7e730575d3297ba8a31d1c2441df623b273f061c32d8fa324f4aa6bead01d23e88582029b568
2019-09-07Merge #15759: p2p: Add 2 outbound block-relay-only connectionsfanquake
0ba08020c9791f7caf5986ad6490c16a2b66cd83 Disconnect peers violating blocks-only mode (Suhas Daftuar) 937eba91e1550bc3038dc541c236ac83e0a0e6d5 doc: improve comments relating to block-relay-only peers (Suhas Daftuar) 430f489027f15c1e4948ea4378954df24e3fee88 Don't relay addr messages to block-relay-only peers (Suhas Daftuar) 3a5e885306ea954d7eccdc11502e91a51dab8ec6 Add 2 outbound block-relay-only connections (Suhas Daftuar) b83f51a4bbe29bf130a2b0c0e85e5bffea107f75 Add comment explaining intended use of m_tx_relay (Suhas Daftuar) e75c39cd425f8c4e5b6bbb2beecb9c80034fefe1 Check that tx_relay is initialized before access (Suhas Daftuar) c4aa2ba82211ea5988ed7fe21e1b08bc3367e6d4 [refactor] Change tx_relay structure to be unique_ptr (Suhas Daftuar) 4de0dbac9b286c42a9b10132b7c2d76712f1a319 [refactor] Move tx relay state to separate structure (Suhas Daftuar) 26a93bce29fd813e1402b013f402869c25b656d1 Remove unused variable (Suhas Daftuar) Pull request description: Transaction relay is optimized for a combination of redundancy/robustness as well as bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology. Network topology is better kept private for (at least) two reasons: (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction. (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split). We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary. After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).) ACKs for top commit: sipa: ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 ajtowns: ACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 -- code review, ran tests. ran it on mainnet for a couple of days with MAX_BLOCKS_ONLY_CONNECTIONS upped from 2 to 16 and didn't observe any unexpected behaviour: it disconnected a couple of peers that tried sending inv's, and it successfully did compact block relay with some block relay peers. TheBlueMatt: re-utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83. Pointed out that stats.fRelayTxes was sometimes uninitialized for blocksonly peers (though its not a big deal and only effects RPC), which has since been fixed here. Otherwise changes are pretty trivial so looks good. jnewbery: utACK 0ba08020c9791f7caf5986ad6490c16a2b66cd83 jamesob: ACK https://github.com/bitcoin/bitcoin/commit/0ba08020c9791f7caf5986ad6490c16a2b66cd83 Tree-SHA512: 4c3629434472c7dd4125253417b1be41967a508c3cfec8af5a34cad685464fbebbb6558f0f8f5c0d4463e3ffa4fa3aabd58247692cb9ab8395f4993078b9bcdf
2019-09-06Testchains: Generic selection with -chain=<str> in addition of -testnet and ↵Jorge Timón
-regtest
2019-09-04Add 2 outbound block-relay-only connectionsSuhas Daftuar
Transaction relay is primarily optimized for balancing redundancy/robustness with bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology. Network topology is better kept private for (at least) two reasons: (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction. (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split). We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary. After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).)
2019-09-02Merge #13868: Remove unused fScriptChecks parameter from CheckInputsfanquake
9b92538adebd30353ffc1a427bc20ab0c3fc75f1 Remove unused fScriptChecks parameter from CheckInputs (Matt Corallo) Pull request description: fScriptChecks = false just short-circuits the entire function, so passing it in is entirely useless. This is extracted from #13233 /cc TheBlueMatt. Recommend reviewing with `git show --ignore-all-space`, i.e.: https://github.com/bitcoin/bitcoin/pull/13868/files?w=1 ACKs for top commit: TheBlueMatt: utACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1. Checked diff had no functional change and new comment copy looks correct. kallewoof: ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1 ajtowns: ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1 ; code review, checked tests work. Looks right to me, and fanquake's notes make sense. Could change the coinbase early exit to `assert(!tx.IsCoinBase());`. fanquake: ACK 9b92538adebd30353ffc1a427bc20ab0c3fc75f1 - Notes / testing below. Tree-SHA512: add253a3e8cf4b33eddbc49efcec333c14b5ea61c7d34e43230351d40cff6adc919a75b91c72c4de8647a395284db74a61639f4c67848d4b2fec3a705b557790
2019-08-29doc: Delete stale URL in test READMEMichael Folkson
2019-08-26Remove unused fScriptChecks parameter from CheckInputsMatt Corallo
fScriptChecks = false just short-circuits the entire function, so passing it in is entirely useless.
2019-08-26Support serialization of std::vector<bool>Pieter Wuille
2019-08-23Merge #16570: tests: Make descriptor tests deterministicfanquake
b9ee63c71b751fd67da777774ea8c0b27e7db2f8 Make descriptor test deterministic (David Reikher) Pull request description: This is an improvement to a test, inspired by #14343 - removing non determinism from a test. The test `descriptor_test` is non-deterministic, as it relies on the `MaybeUseHInsteadOfApostrophy` function which randomly either swaps all apostrophes with 'h' or doesn't at all in a descriptor. This fix makes both cases always run, if an apostrophe is found in a test descriptor. This does not reduce test coverage but removes the non-determinism. Additionally, the `MaybeUseHInsteadOfApostrophy` function removed the checksum if found at the end of a descriptor when the apostrophes are swapped by 'h's, since after being swapped the checksum is no longer correct. I instead added re-calculation of the checksum using the `DescriptorChecksum` function, which adds coverage for the case of a descriptors having 'h's instead of apostrophes and a checksum. This was previously lacking. To achieve this I had to move `DescriptorChecksum` and `PolyMod` out of the anonymous namespace in descriptor.cpp to make `DescriptorChecksum` accessible in descriptor_tests.cpp. All tests complete successfully (functional as well as unit tests). ACKs for top commit: achow101: Code Review ACK b9ee63c71b751fd67da777774ea8c0b27e7db2f8 Tree-SHA512: 992c73a6644a07bfe7c72301ee2666f3c4845a012aaedd7a099a05cea8bdac84fa8280b28e44a7856260c00c0be1a6f1b6768f5694c2a22edf4c489e53fec424
2019-08-21Make descriptor test deterministicDavid Reikher
Changed MaybeUseHInsteadOfApostrophy to UseHInsteadOfApostrophe. This function now always replaces apostrophes with 'h'. The original Check function was renamed to DoCheck and it's called with an additional parameter which tells it to either leave the prv and pub arguments as is or replace the apostrophes with 'h'. The test runs with apostrophes replaced in prv only, pub only, prv and pub and without replacement at all. Replacement of apostrophes in a descriptor and then running DoCheck is conditional on whether apostrophes are found in that descriptor. Additionally, instead of dropping the checksum recalculate it after replacing apostrophes with 'h' in the function UseHInsteadOfApostrophe using the GetDescriptorChecksum function. That way, this also introduces an indirect unit test to GetDescriptoChecksum.
2019-08-20util: Add Join helper to join a list of stringsMarcoFalke
2019-08-16Additional tests for other failure casesAndrew Chow
2019-08-16Check error messages in descriptor testsAndrew Chow
2019-08-16Return an error from descriptor Parse that gives more information about what ↵Andrew Chow
failed
2019-08-15Merge #16060: Bury bip9 deploymentsMarcoFalke
e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 [docs] Add release notes for burying bip 9 soft fork deployments (John Newbery) 8319e738f9f118025b332e4fa804d4c31e4113f4 [tests] Add coverage for the content of getblockchaininfo.softforks (James O'Beirne) 0328dcdcfcb56dc8918697716d7686be048ad0b3 [Consensus] Bury segwit deployment (John Newbery) 1c93b9b31c2ab7358f9d55f52dd46340397c906d [Consensus] Bury CSV deployment height (John Newbery) 3862e473f0cb71a762c0306b171b591341d58142 [rpc] Tidy up reporting of buried and ongoing softforks (John Newbery) Pull request description: This hardcodes CSV and segwit activation heights, similar to the BIP 90 buried deployments for BIPs 34, 65 and 66. CSV and segwit have been active for over 18 months. Hardcoding the activation height is a code simplification, makes it easier to understand segwit activation status, and reduces technical debt. This was originally attempted by jl2012 in #11398 and again by me in #12360. ACKs for top commit: ajtowns: ACK e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 ; checked diff to previous acked commit, checked tests still work ariard: ACK e78aaf4, check diff, run the tests again and successfully activated csv/segwit heights on mainnet as expected. MarcoFalke: ACK e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 (still didn't check if the mainnet block heights are correct, but the code looks good now) Tree-SHA512: 7e951829106e21a81725f7d3e236eddbb59349189740907bb47e33f5dbf95c43753ac1231f47ae7bee85c8c81b2146afcdfdc11deb1503947f23093a9c399912
2019-08-15Merge #16443: refactor: have CCoins* data managed under CChainStateMarcoFalke
582d2cd74754d6b9a2394616a9c82a89d2d71976 Cover UTXO set access with lock annotations (James O'Beirne) 569353068568444a25b301bbd6513bb510157dc9 refactor: have CCoins* data managed under CChainState (James O'Beirne) fae6ab6aed3b9fdc9201bb19a307dfc3d9b89891 refactor: pcoinsTip -> CChainState::CoinsTip() (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/2019-04-proposal/proposal --- This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set. We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy. This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations. ACKs for top commit: Sjors: reACK 582d2cd74754d6b9a2394616a9c82a89d2d71976 MarcoFalke: ACK 582d2cd747 Tree-SHA512: ec9d904fe5dca8cd2dc4b7916daa5d8bab30856dd4645987300f905e0a19f9919fce4f9d1ff03eda982943ca73e6e9a746be6cf53b46510de36e8c81a1eafba1
2019-08-15refactor: have CCoins* data managed under CChainStateJames O'Beirne
This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set. We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy. Construction of its various pieces (db, coinscatcher, in-memory cache) is split up so that we avoid flushing bad state to disk if startup is interrupted. We also introduce `CChainState::CanFlushToDisk()` which tells us when it is safe to flush the chainstate based on this partial construction. This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations. Other changes: - A parameter has been added to the CCoinsViewDB constructor that allows the name of the corresponding leveldb directory to be specified. Thanks to Russell Yanofsky and Marco Falke for helpful feedback.
2019-08-14[Consensus] Bury segwit deploymentJohn Newbery
Hardcode segwit deployment height to 481824 for mainnet.
2019-08-14Merge #16248: Make whitebind/whitelist permissions more flexibleWladimir J. van der Laan
c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier) d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier) ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier) e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier) Pull request description: # Motivation In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`. Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum. It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes. When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute. Doing so will also make follow up idea very easy to implement in a backward compatible way. # Implementation details The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`. The following permissions exists: * ForceRelay * Relay * NoBan * BloomFilter * Mempool Example: * `-whitelist=bloomfilter@127.0.0.1/32`. * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`. If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible) When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist` and add to it the permissions granted from `whitebind`. To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node. `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`. # Follow up idea Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way: * Changing `connect` at rpc and config file level to understand the permissions flags. * Changing the permissions of a peer at RPC level. ACKs for top commit: laanwj: re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
2019-08-14Merge #16563: test: Add unit test for AddTimeDataWladimir J. van der Laan
7cd069d8ef900c6c6b904ddd6fbd64e14bd0f53e Add test for AddTimeData (Martin Zumsande) Pull request description: `AddTimeData()` has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it. ACKs for top commit: laanwj: ACK 7cd069d8ef900c6c6b904ddd6fbd64e14bd0f53e, thanks for adding a test Tree-SHA512: 8228f9027e52ed534411d595c7e45cf4edeee9757f26f5141fbcfae3fc6f598a8cea7f734bb8f55238857a37ad2f2d518e859e1fe8c106c0712da976792ac132
2019-08-13Add test for AddTimeDataMartin Zumsande
2019-08-11Make whitebind/whitelist permissions more flexiblenicolas.dorier
2019-08-08util: refactor upper/lowercase functionsKarl-Johan Alm
This includes renaming Downcase() to ToLower() and make it return a string rather than modify referenced arg. Also adds ToUpper() string version.
2019-08-06refactor: pcoinsTip -> CChainState::CoinsTip()James O'Beirne
This aliasing makes subsequent commits easier to review; eventually CoinsTip() will return the CCoinsViewCache managed by CChainState.
2019-08-02Merge #16097: Refactor: Add Flags enum to ArgsManager classMarcoFalke
e6f649cb2c07bf55d9214c2876619c56f1d6fe30 test: Make tests arg type specific (Hennadii Stepanov) b70cc5d73357ea11296f3b6bb81193ba1101e73b Revamp option negating policy (Hennadii Stepanov) db08edb3038a085d3dbce7bb4ec3c1d9b9a5b281 Replace IsArgKnown() with FlagsOfKnownArg() (Hennadii Stepanov) dde80c272ae584410532f48d23866d7d8581a1cc Use ArgsManager::NETWORK_ONLY flag (Hennadii Stepanov) 9a12733508e47f558959f1b0ed9937bc3eff8962 Remove unused m_debug_only member from Arg struct (Hennadii Stepanov) fb4b9f9e3b433d8848832e2c2686cf7b1f212a5e scripted-diff: Use ArgsManager::DEBUG_ONLY flag (Hennadii Stepanov) 1b4b9422cad28d1bead24ff5fd472536954cfaf9 scripted-diff: Use Flags enum in AddArg() (Hennadii Stepanov) 265c1b58d89b7b6fb30468ba402d7f75cc59a510 Add Flags enum to ArgsManager (Hennadii Stepanov) e0d187dfeb18b026de22bd7960b2a50c2b958e1a Refactor InterpretNegatedOption() function (Hennadii Stepanov) e0e18a1017fa3dc5d6ebeda6ec35c4263327d17c refactoring: Check IsArgKnown() early (Hennadii Stepanov) Pull request description: This PR adds the `Flags` enum to the `ArgsManager` class. Also the `m_flags` member is added to the `Arg` struct. Flags denote an allowed type of an arg value and special hints. This PR is only a refactoring and does not change behavior. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/16097/commits/e6f649cb2c07bf55d9214c2876619c56f1d6fe30 MarcoFalke: ACK e6f649cb2c07bf55d9214c2876619c56f1d6fe30 thanks for adding types to the command line options Tree-SHA512: b867f8a9cbce2d2473c293d534af662d8cd5be15060ff0682e97af678974bdaac35e8bc6328ccba32f105034bcd38f169b92a6fb67798667891ce14d5d2a2dea
2019-07-31test: Make tests arg type specificHennadii Stepanov
2019-07-30Merge #16433: txmempool: Remove unused default value ↵Jonas Schnelli
MemPoolRemovalReason::UNKNOWN 0000ff0aa763e3be524ac4537a41048a26529fb2 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke) Pull request description: The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default. ACKs for top commit: practicalswift: utACK 0000ff0aa763e3be524ac4537a41048a26529fb2 promag: ACK 0000ff0aa763e3be524ac4537a41048a26529fb2. jonasschnelli: utACK 0000ff0aa763e3be524ac4537a41048a26529fb2 Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
2019-07-27scripted-diff: Use ArgsManager::DEBUG_ONLY flagHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i 's/unsigned int flags, const bool debug_only,/unsigned int flags,/' src/util/system.h src/util/system.cpp sed -i 's/ArgsManager::NONE, debug_only/flags, false/' src/util/system.cpp sed -i 's/arg.second.m_debug_only/(arg.second.m_flags \& ArgsManager::DEBUG_ONLY)/' src/util/system.cpp sed -i 's/ArgsManager::ALLOW_ANY, true, OptionsCategory::/ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::/' $(git grep --files-with-matches 'AddArg(' src) sed -i 's/ArgsManager::ALLOW_ANY, false, OptionsCategory::/ArgsManager::ALLOW_ANY, OptionsCategory::/' $(git grep --files-with-matches 'AddArg(' src) -END VERIFY SCRIPT-
2019-07-27scripted-diff: Use Flags enum in AddArg()Hennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i 's/const bool debug_only,/unsigned int flags, &/' src/util/system.h src/util/system.cpp sed -i -E 's/(true|false), OptionsCategory::/ArgsManager::ALLOW_ANY, &/' $(git grep --files-with-matches 'AddArg(' src) -END VERIFY SCRIPT-
2019-07-24Refactor out translation.hHennadii Stepanov
This is a prerequisite for introducing bilingual error messages. Note: #includes are arranged by clang-format-diff.py script.
2019-07-22txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWNMarcoFalke
2019-07-18test: remove redundant setup in addrman_testszenosage
2019-07-11Merge #16227: Refactor CWallet's inheritance chainWladimir J. van der Laan
93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow) 8f5b81e6edae9cb22559545de63f391d97c15701 Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow) 37a79a4fccbf6cd65a933594e24e59d36e674653 Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow) 16f8096e911e4d59292240a17e2d4004f0500b9e Move KeyOriginInfo to its own header file (Andrew Chow) d9becff4e13da8e182631baa79b9794c03d44434 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow) a913e3f2fbeb1352fc66f334d4f5f7332ea89ad7 Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow) c7797ec65544bd23a2e571b2892e1bf512f2a485 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow) 1b699a5083b435c2b79f3951f94ac9f967d24f6c Add HaveKey and HaveCScript to SigningProvider (Andrew Chow) Pull request description: This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is: ``` SigningProvider -> FillableSigningProvider -> CWallet ``` `SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`). Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure. ACKs for top commit: Sjors: re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment. instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/16227/commits/93ce4a0b6fb54efb1f424a71dfc09cc33307e5b9 Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
2019-07-11Merge #15649: Add ChaCha20Poly1305@Bitcoin AEADWladimir J. van der Laan
bb326add9f38f2a8e5ce5ee29d98ce08038200d8 Add ChaCha20Poly1305@Bitcoin AEAD benchmark (Jonas Schnelli) 99aea045d688059caf89c0e485fa427bd28eddd8 Add ChaCha20Poly1305@Bitcoin tests (Jonas Schnelli) af5d1b5f4a7b56628a76af21284c258d845894f0 Add ChaCha20Poly1305@Bitcoin AEAD implementation (Jonas Schnelli) Pull request description: This adds a new AEAD (authenticated encryption with additional data) construct optimised for small messages (like used in Bitcoins p2p network). Includes: #15519, #15512 (please review those first). The construct is specified here. https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#ChaCha20Poly1305Bitcoin_Cipher_Suite This aims for being used in v2 peer-to-peer messages. ACKs for top commit: laanwj: code review ACK bb326add9f38f2a8e5ce5ee29d98ce08038200d8 Tree-SHA512: 15bcb86c510fce7abb7a73536ff2ae89893b24646bf108c6cf18f064d672dbbbea8b1dd0868849fdac0c6854e498f1345d01dab56d1c92031afd728302234686
2019-07-10Merge #16237: Have the wallet give out destinations instead of keysWladimir J. van der Laan
8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Add GetNewChangeDestination for getting new change Destinations (Andrew Chow) 33d13edd2bda0af90660e275ea4fa96ca9896f2a Replace CReserveKey with ReserveDestinatoin (Andrew Chow) 172213be5b174243dc501c1103ad5fe2fee67a16 Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow) Pull request description: The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool. ACKs for top commit: instagibbs: re-utACK https://github.com/bitcoin/bitcoin/pull/16237/commits/8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf sipa: ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK. laanwj: ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500
2019-07-09Add GetNewDestination to CWallet to fetch new destinationsAndrew Chow
Instead of having the same multiple lines of code everywhere that new destinations are fetched, introduce GetNewDestination as a member function of CWallet which does the key fetching, label setting, script generation, and destination generation.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-09 16:12:00 +0000