sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-27 21:12:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity
25,131 Commits 24 Branches 554 Tags 576 MiB
f41a113263
Commit Graph

2096 Commits

Author SHA1 Message Date
MarcoFalke
70371cf203
Merge #20159: test: mining_getblocktemplate_longpoll.py improvements (use MiniWallet, add logging) 
b128b566725a5037fdaea99940d1b9de5553d198 test: add logging for mining_getblocktemplate_longpoll.py (Sebastian Falbesoner)
8ee3536b2b77aeb3a48df5b34effbc7345ef34d8 test: remove unused helpers random_transaction(), make_change() and gather_inputs() (Sebastian Falbesoner)
fddce7e199308d96e366d700dca982ef088ba98b test: use MiniWallet for mining_getblocktemplate_longpoll.py (Sebastian Falbesoner)

Pull request description:

  This PR enables one more of the non-wallet functional tests (mining_getblocktemplate_longpoll.py) to be run even with the Bitcoin Core wallet disabled by using the new MiniWallet instead, as proposed in #20078. Also adds missing log messages for the subtests.

  This was the only functional test that used the `random_transaction` helper in `test_framework/util.py`, hence it is removed, together with other helpers (`make_change` and `gather_inputs`) that were again only used by `random_transaction`.

ACKs for top commit:
  MarcoFalke:
    ACK b128b566725a5037fdaea99940d1b9de5553d198

Tree-SHA512: 09a5fa7b0f5976a47040f7027236d7ec0426d5a4829a082221c4b5fae294470230e89ae3df0bca0eea26833162c03980517f5cc88761ad251c3df4c4a49bca46
 2024-01-26 12:50:30 -06:00
MarcoFalke
52ff3ec054
Merge #20876: test: Replace getmempoolentry with testmempoolaccept in MiniWallet 
faabc26a61873b2cd0390a21df571fe53c893c11 test: Replace getmempoolentry with testmempoolaccept in MiniWallet (MarcoFalke)

Pull request description:

  This is a refactor to not use the return value of `sendrawtransaction` and `getmempoolentry` with the goal that submitting the tx to the mempool will become optional.

ACKs for top commit:
  mjdietzx:
    ACK faabc26a61873b2cd0390a21df571fe53c893c11

Tree-SHA512: 4260a59e65fed1c807530dad23f1996ba6e881bcce91995f5b498a0be6001f67b3e0251507c8801750fe105410147c68bb2f393ebe678c6a3a4d045e5d72fc19
 2024-01-26 12:50:30 -06:00
MarcoFalke
471c2cb211
Merge #20688: test: run mempool_compatibility.py even with wallet disabled 
a7599c80ebb9579df45e2d6ccf3168302cf42f03 test: run mempool_compatibility.py even with wallet disabled (Michael Dietz)

Pull request description:

  Another functional test rewritten as proposed in https://github.com/bitcoin/bitcoin/issues/20078

ACKs for top commit:
  MarcoFalke:
    review ACK a7599c80ebb9579df45e2d6ccf3168302cf42f03 didn't test

Tree-SHA512: cc7a617e5489ed27bbdbdee85a82fa08525375061f7f4524577a6b8ecb340396adac88419b51f513be22ca53edd0a3bd5d572d9f43ffc2c18550b0ef9069d238
 2024-01-26 12:50:30 -06:00
Wladimir J. van der Laan
eddf1307fa
Merge #19922: test: Run rpc_txoutproof.py even with wallet disabled 
faf251d854e3a670533ea3e9087e82c92f3ae533 test: gettxoutproof duplicate txid (João Barbosa)
faf5eb45c4a08fbfd9a8c12534bca8adfe756ef2 test: Test empty array in gettxoutproof (MarcoFalke)
fa56e866e8ac08b35e775a4e37a4e5849e093c7d test: Run rpc_txoutproof.py even with wallet disabled (MarcoFalke)
faba790bd40b5a9e8849997785020790ff60571b test: MiniWallet: Default fee_rate in send_self_transfer, Pass in utxo_to_spend (MarcoFalke)
fa65a11d0c9a34ff7f4cc4efd53367794e751749 test: bugfix: Actually pick largest utxo (MarcoFalke)

Pull request description:

  Run the consensus test even when the wallet was not compiled. Also:

  * Minor bugfix in MiniWallet
  * Two new test cases (one cherry-picked from #19847)

ACKs for top commit:
  jnewbery:
    utACK faf251d854e3a670533ea3e9087e82c92f3ae533. Thanks Marco!
  kristapsk:
    ACK faf251d854e3a670533ea3e9087e82c92f3ae533

Tree-SHA512: a5ab33695c88cfb3c369021d4506069c08ce298e24e891db55159130693ed3817444c72f6aad3f472235aa4597b2c601010af714411c2ec8ad9c2d2e0b00ecbc
 2024-01-26 12:50:29 -06:00
MarcoFalke
403f7939b6
Merge #19800: test: Mockwallet 
fa188c9c59b8c3e43c31be01797f073e27a7bc10 test: Use MiniWalet in p2p_feefilter (MarcoFalke)
fa39c62eb7f39e7d249b8d46c075c4e7a9aec684 test: inline hashToHex (MarcoFalke)

Pull request description:

  This introduces a minimalistic test wallet, which can be used as a drop in replacement for the Bitcoin Core wallet to create dummy transactions with a given fee rate.

ACKs for top commit:
  jnewbery:
    utACK fa188c9c59b8c3e43c31be01797f073e27a7bc10

Tree-SHA512: 0aad9cb14eea4f0055bd6a47cc8c8f82a16941b152598c3bf1e083aae84cca4ffa23f0b854a362a68be1b917deba1b5ec7c0207b63b0805d747ba9a7d1d82efe
 2024-01-26 12:50:27 -06:00
Samuel Dobson
4d22fe2498
Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo for segwit inputs 
84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow)
46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow)
5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow)
72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow)

Pull request description:

  Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition.

  Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper.

  Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs.

  As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs.

ACKs for top commit:
  Sjors:
    utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators)
  ryanofsky:
    Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to
  meshcollider:
    utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3

Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
 2024-01-23 22:14:13 -06:00
Wladimir J. van der Laan
6317a2951f
Merge #19818: p2p: change CInv::type from int to uint32_t, fix UBSan warning 
7984c39be11ca04460883365e1ae2a496aaa6c0e test framework: serialize/deserialize inv type as unsigned int (Jon Atack)
407175e0c2bc797599ebd9c0a1f2ec89ad7af136 p2p: change CInv::type from int to uint32_t (Jon Atack)

Pull request description:

  Fixes UBSan implicit-integer-sign-change issue per https://github.com/bitcoin/bitcoin/pull/19610#issuecomment-680686460.

  Credit to Crypt-iQ for finding and reporting the issue and to vasild for the original review suggestion in https://github.com/bitcoin/bitcoin/pull/19590#pullrequestreview-455788826.

  Closes #19678.

ACKs for top commit:
  laanwj:
    ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e
  MarcoFalke:
    ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e 🌻
  vasild:
    ACK 7984c39be

Tree-SHA512: 59f3a75f40ce066ca6f0bb1927197254238302b4073af1574bdbfe6ed580876437be804be4e47d51467d604f0d9e3a5875159f7f2edbb2351fdb2bb9465100b5
 2024-01-22 19:44:37 -06:00
Samuel Dobson
76d30c9607
Merge #18244: rpc: fundrawtransaction and walletcreatefundedpsbt also lock manually selected coins 
6d1f51343cf11b07cd401fbd0c5bc3603e185a0e [rpc] fundrawtransaction, walletcreatefundedpsbt lock manually selected coins (Sjors Provoost)

Pull request description:

  When using `fundrawtransaction` and `walletcreatefundedpsbt` with `lockUnspents`, it would only lock automatically selected coins, not manually selected coins. That doesn't make much sense to me if the goal is to prevent accidentally double-spending yourself before you broadcast a transaction.

  Note that when  creating a transaction, manually selected coins are automatic "unlocked" (or more accurately: the lock is ignored). Earlier versions of this PR introduced an error when a locked coin is manually selected, but this idea was abandoned after some discussion. An application that uses this RPC should either rely on automatic coin selection (with `lockUnspents`) or handle lock concurrency itself with manual coin selection. In particular it needs to make sure to avoid/pause calls with automatic coin selection between calling `lockunspent` and the subsequent spending RPC.

  See #7518 for historical background.

ACKs for top commit:
  meshcollider:
    Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e
  fjahr:
    Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e

Tree-SHA512: 8773c788d92f2656952e1beac147ba9956b8c5132d474e0880e4c89ff53642928b4cbfcd1cb3d17798b9284f02618a8830c93a9f7a4733e5bded96adff1d5d4d
 2024-01-22 19:44:36 -06:00
MarcoFalke
261d2a0ded
Merge #19730: ci: Set increased --timeout-factor by default 
fa330ec2fe5f5ba68a8d43fff0b19584c0b1ff39 test: Remove confusing and broken use of wait_until global (MarcoFalke)
fa6583c30bf7d82cf7ffdae995f8f16524ad2c0d ci: Set increased --timeout-factor by default (MarcoFalke)

Pull request description:

  Assuming that tests don't have a logic error or race, setting a high timeout should not cause any issues. The tests will still pass just as fast in the fastest case, but it allows for some buffer in case of slow disks or otherwise starved ci machines.

  Fixes #19729

ACKs for top commit:
  hebasto:
    ACK fa330ec2fe5f5ba68a8d43fff0b19584c0b1ff39, I have reviewed the code, and it looks OK, I agree it can be merged.

Tree-SHA512: 3da80ee008c7b08bab5fdaf7804d57c79d6fed49a7d37b9c54fc89756659fcb9981fd10afc4d07bd90d93c1699fd410a0110a3cd34d016873759d114ce3cd82d
 2024-01-22 19:44:36 -06:00
Konstantin Akimov
9b3d2e0c17
Merge bitcoin#18044: Use wtxid for transaction relay 
This backport is marked as full, not partial, but it has only refactorings
and non-witness related changes.

Included commits are:
 - test: Update test framework p2p protocol version to 70016
 - Rename AddInventoryKnown() to AddKnownTx()
 - Add support for tx-relay via wtxid

    This adds a field to CNodeState that tracks whether to relay transactions with
    that peer via wtxid, instead of txid. As of this commit the field will always
    be false, but in a later commit we will add a way to negotiate turning this on
    via p2p messages exchanged with the peer.

 - Just pass a hash to AddInventoryKnown

    Since it's only used for transactions, there's no need to pass in an inv type.

 - Add wtxid to mempool unbroadcast tracking
 2024-01-22 19:44:33 -06:00
Konstantin Akimov
d17665307b
feat: new rpc getrawtransactionmulti (#5839) 
## Issue being fixed or feature implemented
For platform needs `getrawtransactionmulti` will help to reduce amount
of rpc calls for sake of performance improvement.

## What was done?
Implemented new RPC, basic functional test, release note.

## How Has This Been Tested?
On testnet:
```
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}'
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}'  true
```

## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: pasta <pasta@dashboost.org>
 2024-01-22 19:33:24 -06:00
MarcoFalke
4c8e77a48d
Merge #19752: test: Update wait_until usage in tests not to use the one from utils 
d841301010914203fb5ef02627c76fad99cb11f1 test: Add docstring to wait_until() in util.py to warn about its usage (Seleme Topuz)
1343c86c7cc1fc896696b3ed87c12039e4ef3a0c test: Update wait_until usage in tests not to use the one from utils (Seleme Topuz)

Pull request description:

  Replace global (from [test_framework/util.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/util.py#L228)) `wait_until()` usages with the ones provided by `BitcoinTestFramework` and `P2PInterface` classes.

  The motivation behind this change is that the `util.wait_until()` expects a timeout, timeout_factor and lock and it is not aware of the context of the test framework. `BitcoinTestFramework` offers a `wait_until()` which has an understandable amount of default `timeout` and a shared `timeout_factor`. Moreover, on top of these, `mininode.wait_until()` also has a shared lock.

  closes #19080

ACKs for top commit:
  MarcoFalke:
    ACK d841301010914203fb5ef02627c76fad99cb11f1 🦆
  kallewoof:
    utACK d841301010914203fb5ef02627c76fad99cb11f1

Tree-SHA512: 81604f4cfa87fed98071a80e4afe940b3897fe65cf680a69619a93e97d45f25b313c12227de7040e19517fa9c003291b232f1b40b2567aba0148f22c23c47a88
 2024-01-20 00:07:11 +07:00
MarcoFalke
4171afe54e
Merge #19552: test: fix intermittent failure in p2p_ibd_txrelay 
12410b1feb80189061eb4a2b43421e53cbb758a8 test: fix intermittent p2p_ibd_txrelay race, add test_framework.py#wait_until (Jon Atack)

Pull request description:

  To fix these intermittent failures in Travis CI.
  ```
  162/163 - p2p_ibd_txrelay.py failed, Duration: 2 s

  stdout:
  2020-07-19T05:44:17.213000Z TestFramework (INFO):
      Check that nodes set minfilter to MAX_MONEY while still in IBD
  2020-07-19T05:44:17.216000Z TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/test_framework.py", line 117, in main
      self.run_test()
    File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/p2p_ibd_txrelay.py", line 30, in run_test
      assert_equal(conn_info['minfeefilter'], MAX_FEE_FILTER)
    File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/util.py", line 49, in assert_equal
      raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))

  AssertionError: not(0E-8 == 0.09170997)
  2020-07-19T05:44:17.293000Z TestFramework (INFO): Stopping nodes
  ```

  At Marco's suggestion, cherry-picked part of #19134 to nicely simplify using `wait_until`.

ACKs for top commit:
  vasild:
    ACK 12410b1fe

Tree-SHA512: 615f509883682fd693e578b259cba35a9fa0bc519f1394e88c857e8b0650bfec5397bfa856cfa9e6d5ef81d0ee6ad02e4ad2b0eb0bd530b4c281cbe3e663790b
 2024-01-20 00:07:11 +07:00
Konstantin Akimov
f34889dcf4
Merge #19760: test: Remove confusing mininode terminology 
d5800da5199527a366024bc80cad7fcca17d5c4a [test] Remove final references to mininode (John Newbery)
5e8df3312e47a73e747ee892face55ed9ababeea test: resort imports (John Newbery)
85165d4332b0f72d30e0c584b476249b542338e6 scripted-diff: Rename mininode to p2p (John Newbery)
9e2897d020b114a10c860f90c5405be029afddba scripted-diff: Rename mininode_lock to p2p_lock (John Newbery)

Pull request description:

  New contributors are often confused by the terminology in the test framework, and what the difference between a _node_ and a _peer_ is. To summarize:

  - a 'node' is a bitcoind instance. This is the thing whose behavior is being tested. Each bitcoind node is managed by a python `TestNode` object which is used to start/stop the node, manage the node's data directory, read state about the node (eg process status, log file), and interact with the node over different interfaces.
  - one of the interfaces that we can use to interact with the node is the p2p interface. Each connection to a node using this interface is managed by a python `P2PInterface` or derived object (which is owned by the `TestNode` object). We can open zero, one or many p2p connections to each bitcoind node. The node sees these connections as 'peers'.

  For historic reasons, the word 'mininode' has been used to refer to those p2p interface objects that we use to connect to the bitcoind node (the code was originally taken from the 'mini-node' branch of https://github.com/jgarzik/pynode/tree/mini-node). However that name has proved to be confusing for new contributors, so rename the remaining references.

ACKs for top commit:
  amitiuttarwar:
    ACK d5800da519
  MarcoFalke:
    ACK d5800da5199527a366024bc80cad7fcca17d5c4a 🚞
Tree-SHA512: 2c46c2ac3c4278b6e3c647cfd8108428a41e80788fc4f0e386e5b0c47675bc687d94779496c09a3e5ea1319617295be10c422adeeff2d2bd68378e00e0eeb5de
 2024-01-20 00:07:10 +07:00
fanquake
9d33b30a87
Merge #19674: refactor: test: use throwaway _ variable for unused loop counters 
dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 refactor: test: use _ variable for unused loop counters (Sebastian Falbesoner)

Pull request description:

  This tiny PR substitutes Python loops in the form of `for x in range(N): ...` by `for _ in range(N): ...` where applicable. The idea is indicating to the reader that a block (or statement, in list comprehensions) is just repeated N times, and that the loop counter is not used in the body, hence using the throwaway variable. This is already done quite often in the current tests (see e.g. `$ git grep "for _ in range("`). Another alternative would be using `itertools.repeat` (according to Python core developer Raymond Hettinger it's [even faster](https://twitter.com/raymondh/status/1144527183341375488)), but that doesn't seem to be widespread in use and I'm not sure about a readability increase.

  The only drawback I see is that whenever one wants to debug loop iterations, one would need to introduce a loop variable again. Reviewing this is basically a no-brainer, since tests would fail immediately if a a substitution has taken place on a loop where the variable is used.

  Instances to replace were found by `$ git grep "for.*in range("` and manually checked.

ACKs for top commit:
  darosior:
    ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64
  instagibbs:
    manual inspection ACK dac7a111bd
  practicalswift:
    ACK dac7a111bdd3b0233d94cf68dae7a8bfc6ac9c64 -- the updated code is easier to reason about since the throwaway nature of a variable is expressed explicitly (using the Pythonic `_` idiom) instead of implicitly. Explicit is better than implicit was we all know by now :)

Tree-SHA512: 5f43ded9ce14e5e00b3876ec445b90acda1842f813149ae7bafa93f3ac3d510bb778e2c701187fd2c73585e6b87797bb2d2987139bd1a9ba7d58775a59392406
 2024-01-20 00:07:09 +07:00
fanquake
7c723d88c6
Merge bitcoin/bitcoin#25096: [net] Minor improvements to addr caching 
292828cd7744ec7eadede4ad54aa2117087c5435 [test] Test addr cache for multiple onion binds (dergoegge)
3382905befd23364989d941038bf7b1530fea0dc [net] Seed addr cache randomizer with port from binding address (dergoegge)
f10e80b6e4fbc151abbf1c20fbdcc3581d3688f0 [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer (dergoegge)

Pull request description:

  The addr cache id randomizer is currently supposed to be seeded with the network of the inbound connection and the local socket (only the address is used not the port):  a8098f2cef/src/net.cpp (L2800-L2804)

  For inbound onion connections `CNode::addr.GetNetwork()` returns `NET_UNROUTABLE` and `CNode::addrBind` is set to `127.0.0.1:<onion bind port>`. This results in the same addr cache for all inbound connections on 127.0.0.1 binds.

  To avoid the same addr cache across all onion and other 127.0.0.1 binds, we should seed the addr cache randomizer with the correct network for inbound onion connections (using `CNode::ConnectedThroughNetwork()`) as well as the port of `CNode::addrBind`.

ACKs for top commit:
  sipa:
    utACK 292828cd7744ec7eadede4ad54aa2117087c5435
  mzumsande:
    Code Review ACK 292828cd7744ec7eadede4ad54aa2117087c5435
  naumenkogs:
    utACK 292828cd7744ec7eadede4ad54aa2117087c5435

Tree-SHA512: d0be13bab6bc121c2926d4b168687f6c2ed4ce0c9dd19be71eb4886adeba8afc3daacdc4e232a0ba3b03a89d69b618abc5595b69abd1ad0c476d825bc6ea1f9f
 2024-01-19 11:02:23 -06:00
MacroFake
4d56397d96
Merge bitcoin/bitcoin#25253: test: add coverage for non-hex value to -minimumchainwork 
ebfc308ea4b8851118e8194d837556bf443c329c test: add coverage for non-hex value to -minimumchainwork (brunoerg)

Pull request description:

  This PR adds test coverage for the following init error:
  b9ef5a10e2/src/init.cpp (L917-L919)

  Passing a non-hex value to -minimumchainwork should throw an initial error.

ACKs for top commit:
  laanwj:
    Code review ACK ebfc308ea4b8851118e8194d837556bf443c329c
  kristapsk:
    re-ACK ebfc308ea4b8851118e8194d837556bf443c329c

Tree-SHA512: c665903757ae3b8b2480df97bb888e60ba4387b009fcb8031041822e87a155a0e4950ebe79873c1034f0826504521d82b1fdfdb5e8378b227d14ca545b8d4e11
 2024-01-19 11:02:23 -06:00
MarcoFalke
adff1fc13b
Merge bitcoin/bitcoin#22102: Remove Warning: from warning message printed for unknown new rules 
6d7e46ce23217da53ff52f535879c393c02fa2b2 Remove `Warning:` (Prayank)

Pull request description:

  Reason: I noticed that `Warning` is printed 2 times in `-getinfo` while reviewing https://github.com/bitcoin/bitcoin/pull/21832#issuecomment-851004943

  Same string is used for GUI, log and stderr. If we need to add `Warning:` in GUI or other place we can always prepend to this string.

  CLI:

  ```
  Warnings: Unknown new rules activated (versionbit 28)

  ```

  GUI:

  ![image](https://user-images.githubusercontent.com/13405205/120110401-e36ab180-c18a-11eb-8031-4d52287dc263.png)

ACKs for top commit:
  MarcoFalke:
    review ACK 6d7e46ce23217da53ff52f535879c393c02fa2b2

Tree-SHA512: 25760cf6d850f6c2216d651fa46574d2d21a9d58f4577ecb58f9566ea8cd07bfcd6679bb8a1f53575e441b02d295306f492c0c99a48c909e60e5d977ec1861f6
 2024-01-19 11:02:21 -06:00
Wladimir J. van der Laan
bfc5f4bcc5
Merge #19655: rpc: Catch listsinceblock target_confirmations exceeding block count 
c133cdcdc3397a734d57e05494682bf9bf6f4c15 Cap listsinceblock target_confirmations param (Adam Stein)

Pull request description:

  This addresses an issue brought up in #19587.

  Currently, the `target_confirmations` parameter to `listsinceblock` is not checked for being too large. When `target_confirmations` is greater than one more than the current number of blocks, `listsinceblock` fails with error code -1. In comparison, when `target_confirmations` is less than 1,  a -8 "Invalid parameter" error code is thrown.

  This PR fixes the issue by returning a -8 "Invalid parameter" error if the `target_confirmations` value corresponds to a block with more confirmations than the genesis block. This happens if `target_confirmations` exceeds one more than the number of blocks.

ACKs for top commit:
  laanwj:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15
  ryanofsky:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15. Just suggested changes since last review. Thanks!

Tree-SHA512: 02680f4cb937d2c24d5019abd0ebfa188b8a50679a1e64e9c26bfe5c17eef6aea906832e6e2d492ba8a2ea160041bf185d66795ee691e340f6793db03c21b89a
 2024-01-19 10:34:33 -06:00
Konstantin Akimov
d5c5a266f5
fix: make llmq_test_instantsend great again (#5832) 
## Issue being fixed or feature implemented
Running 3 nodes on RegTest as platform does uses do not let to create
`llmq_test_instantsend` quorum:

```
1. switch to `llmq_test_instantsend`:
+        self.extra_args = [["-llmqtestinstantsenddip0024=llmq_test_instantsend"]] * 5

2. removed cycle-quorum related code:

-        self.move_to_next_cycle()
-        self.log.info("Cycle H height:" + str(self.nodes[0].getblockcount()))
-        self.move_to_next_cycle()
-        self.log.info("Cycle H+C height:" + str(self.nodes[0].getblockcount()))
-        self.move_to_next_cycle()
-        self.log.info("Cycle H+2C height:" + str(self.nodes[0].getblockcount()))
-
-        self.mine_cycle_quorum(llmq_type_name='llmq_test_dip0024', llmq_type=103)

3. added new quorum:

+        self.mine_quorum(llmq_type_name='llmq_test_instantsend', llmq_type=104)

and eventually it stucked, no quorum happens

2024-01-13T19:18:49.317000Z TestFramework (INFO): Expected quorum_0 at:984
2024-01-13T19:18:49.317000Z TestFramework (INFO): Expected quorum_0 hash:6788e18f0235a5c85f3d3c6233fe132a80e74a2912256db3ad876a8ebf026048
2024-01-13T19:18:49.317000Z TestFramework (INFO): quorumIndex 0: Waiting for phase 1 (init)
<frozen>
```

## What was done?
Updated condition to enable "llmq_test_instantsend":
 - it is RegTest and DIP0024 is not active
- it is RegTest, DIP0024 is active, and specified as
`llmqTypeDIP0024InstantSend`

## How Has This Been Tested?
Run unit and functional tests.
Beside that functional test feature_asset_locks.py now uses this quorum
for instant send and that's an arrow that hit 2 birds: we have test for
command line option `-llmqtestinstantsenddip0024` and code of
feature_asset_locks.py is simplified.


## Breaking Changes
yes, that's a bugfix that fix quorum `llmq_test_instantsend` absentance
on regtest after dip-0024 activation.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2024-01-19 09:14:04 -06:00
Konstantin Akimov
852adb56ae
refactor: split llmq/utils to Quorum Calculation and llmq/options (#5790) 
## Issue being fixed or feature implemented
`llmq/utils` has simple util code that used all over code base and also
have too heavy code for calculation quorums such as:
`GetAllQuorumMembers`, `EnsureQuorumConnections` and other.

These helpers for calculation quorums are used only by
evo/deterministicmns, evo/simplifiedmns and llmq/* modules, but
llmq/utils is included in many other modules for various trivial
helpers.



## What was done?
Prior work:
 - https://github.com/dashpay/dash/pull/5753
 - #5486
 See also #4798

This PR remove all non-quorum calculation code from llmq/utils.
Eventually it happens that easier to take everything out rather than
move Quorum Calculation to new place atm:
- new module llmq/options have a code related to various params, command
line options, spork-related etc
- llmq/utils is not included in various files which do not use any
llmq/utils code
 - helper `BuildCommitmentHash` goes to llmq/commitment
 - helper `BuildSignHash` goes to llmq/signing
- helper `GetLLMQParam` inlined since it's trivial (it has not been
trivial when introduced ages ago)
- removed dependency of `IsQuorumEnabled` on CQuorumManager which means
`quorumManager` deglobalization is done for 90%


## How Has This Been Tested?
 - Run unit functional tests
- updated circular dependencies
`test/lint/lint-circular-dependencies.sh`
- check that llmq/utils is not included without needs to calculate
Quorums Members
```
$ grep -r include src/ 2> /dev/null | grep -v .Po: | grep -vE 'llmq/utils.(h|cpp)': | grep llmq/utils  
src/evo/mnauth.cpp:#include <llmq/utils.h>
src/evo/deterministicmns.cpp:#include <llmq/utils.h>
src/llmq/quorums.cpp:#include <llmq/utils.h>
src/llmq/blockprocessor.cpp:#include <llmq/utils.h>
src/llmq/commitment.cpp:#include <llmq/utils.h>
src/llmq/debug.cpp:#include <llmq/utils.h>
src/llmq/dkgsessionhandler.cpp:#include <llmq/utils.h>
src/llmq/dkgsession.cpp:#include <llmq/utils.h>
src/llmq/dkgsessionmgr.cpp:#include <llmq/utils.h>
src/rpc/quorums.cpp:#include <llmq/utils.h>
```


## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
 2024-01-17 19:56:41 -06:00
MarcoFalke
6f02baba76
Merge #19252: test: wait for disconnect in disconnect_p2ps + bloomfilter test followups 
9a40cfc558b3f7fa4fff1270f969582af17479a5 [refactor] use waiting inside disconnect_p2ps (gzhao408)
aeb9fb414e2d000830287d9dd3fed7fc2eb570d2 [test] wait for disconnect_p2ps to be reflected in getpeerinfo (gzhao408)
e81942d2e1288367e8da94adb2b2a88be99e4751 [test] logging and style followups for bloomfilter tests (gzhao408)

Pull request description:

  Followup to #19083 which adds bloomfilter-related tests.

  1. Make test_node `disconnect_p2ps` wait until disconnection is complete to avoid race conditions (and not place the burden on tests) from MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/19083#discussion_r437383989). And clean up any redundant `wait_until`s in the functional tests.
  2. Clean up style + logging in p2p_filter.py and p2p_nobloomfilter_messages.py and jonatack's other [comments](https://github.com/bitcoin/bitcoin/pull/19083#pullrequestreview-428955784)

ACKs for top commit:
  jonatack:
    Code review ACK 9a40cfc from re-reviewing the diff and `git range-diff 5cafb46 8386ad5 9a40cfc`
  MarcoFalke:
    ACK 9a40cfc558b3f7fa4fff1270f969582af17479a5 🐂

Tree-SHA512: 2e14b1c12fc08a355bd5ccad7a2a734a4ccda4bc7dc7bac171cb57359819fc1599d764290729af74832fac3e2be258c5d406c701e78ab6d7262835859b9a7d87
 2024-01-16 15:05:10 -06:00
fanquake
65217a74a3
Merge #19260: p2p: disconnect peers that send filterclear + update existing filter msg disconnect logic 
3a10d935ac8ebabdfd336569d943f042ff84b13e [p2p/refactor] move disconnect logic and remove misbehaving (gzhao408)
ff8c430c6589ea72b9e169455cf6437c8623cc52 [test] test disconnect for filterclear (gzhao408)
1c6b787e0319c44f0e0bede3f4a77ac7c2089db2 [netprocessing] disconnect node that sends filterclear (gzhao408)

Pull request description:

  Nodes that don't have bloomfilters turned on (i.e. no `NODE_BLOOM` service) should disconnect peers that send them `filterclear` P2P messages.

  Non-bloomfilter nodes already disconnect peers for [`filteradd` and `filterload`](19e919217e/src/net_processing.cpp (L2218)), but #8709 removed `filterclear` so it could be used to reset tx relay. This isn't needed now because using `feefilter` message is much better for this purpose (See #19204).

  Also refactors existing disconnect logic for `filteradd` and `filterload` into respective message handlers and removes banning for them.

ACKs for top commit:
  jnewbery:
    Code review ACK 3a10d935ac8ebabdfd336569d943f042ff84b13e
  naumenkogs:
    utACK 3a10d93
  gillichu:
    tested ACK: quick test_runner on macOS [`3a10d93`](3a10d935ac)
  MarcoFalke:
    re-ACK 3a10d935ac only change is replacing false with true 🚝

Tree-SHA512: 7aad8b3c0b0e776a47ad52544f0c1250feb242320f9a2962542f5905042f77e297a1486f8cdc3bf0fb93cd00c1ab66a67b2ec426eb6da3fe4cda56b5e623620f
 2024-01-16 15:05:10 -06:00
MarcoFalke
cc6ebdb8da
Merge #19083: test: msg_mempool, fRelay, and other bloomfilter tests 
(BACKPORT NOTICE: p2p_filter.py has also fixes for d5fbd4a92a3e7c1f8266d4cb4b639a0fe4c4c61f:test/functional/p2p_filter.py)
+    Merge #18726: test: check misbehavior more independently in p2p_filter.py
---------------------

dca73941eb0f0a4c9b68efed3870b536f7dd6cfe scripted-diff: rename node to peer for mininodes (gzhao408)
0474ea25afc65546cbfe5f822c0212bf3e211023 [test] fix race conditions and test in p2p_filter (gzhao408)
4ef80f0827392a1310ca5a29cc1f8f5ca5d16f95 [test] sending invalid msgs to node with bloomfilters=0 causes disconnect (gzhao408)
497a619386008dfaec0db15ecaebcdfaf75f5011 [test] add BIP 37 test for node with fRelay=false (gzhao408)
e8acc6015695c8439fc971a12709468995b96dcf [test] add mempool msg test for node with bloomfilter enabled (gzhao408)

Pull request description:

  This PR adds a few tests that are bloomfilter-related, including behavior for when bloomfilters are turned _off_:
  1. Tests p2p message `msg_mempool`: a node that has `peerbloomfilters` enabled should send its mempool (disabled behavior already tested [here](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_mempool.py)).
  2. Tests that bloomfilter peers with [`fRelay=False`](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki#extensions-to-existing-messages) in the `version` message should not receive any invs until they set the filter. The rest is the same as what’s already tested in `p2p_filter.py`.
  3. Tests that peers get disconnected if they send `filterload` or `filteradd` p2p messages to a node with bloom filters disabled.
  4. Refactor: renames p2p_mempool.py to p2p_nobloomfilter_messages.py.
  5. Fixes race conditions in p2p_filter.py

ACKs for top commit:
  MarcoFalke:
    ACK dca73941eb only changes is restoring accidentally deleted test 🍮
  jonatack:
    ACK dca73941eb0f0a4c9b68efed3870b536f7dd6cfe modulo a few nits if you retouch, happy to re-ACK if you take any of them but don't feel obliged to.

Tree-SHA512: 442aeab0755cb8b830251ea170d1d5e6da8ac9029b3276d407a20ee3d588cc61b77b8842368de18c244056316b8c63b911776d6e106bc7c023439ab915b27ad3
 2024-01-16 15:05:09 -06:00
MarcoFalke
94680f4ee3
Merge #19177: test: Fix and clean p2p_invalid_messages functional tests 
af2a145e575f23c64909e6cf1fb323c603bda7ca Refactor resource exhaustion test (Troy Giorshev)
5c4648d17ba18e4194959963994cc6b37053f127 Fix "invalid message size" test (Troy Giorshev)
ff1e7b884447a5ba10553b2d964625f94e255bdc Move size limits to module-global (Troy Giorshev)
57890abf2c7919eddfec36178b1136cd44ffe883 Remove two unneeded tests (Troy Giorshev)

Pull request description:

  This PR touches only the p2p_invalid_messages.py functional test module.  There are two main goals accomplished here.  First, it fixes the "invalid message size" test, which previously made a message that was invalid for multiple reasons.  Second, it refactors the file into a single consistent style.  This file appears to have originally had two authors, with different styles and some test duplication.

  It should now be easier and quicker to understand this module, anticipating the upcoming [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and [AltNet](https://github.com/bitcoin/bitcoin/issues/18989) changes.

  This should probably go in ahead of #19107, but the two are not strictly related.

ACKs for top commit:
  jnewbery:
    ACK af2a145e575f23c64909e6cf1fb323c603bda7ca
  MarcoFalke:
    re-ACK af2a145e57 🍦

Tree-SHA512: 9b57561e142c5eaefac5665f7355c8651670400b4db1a89525d2dfdd20e872d6873c4f6175c4222b6f5a8e5210cf5d6a52da69b925b673a2e2ac30a15d670d1c
 2024-01-16 15:05:09 -06:00
MarcoFalke
5d42d044df
Merge #21117: test: remove assert_blockchain_height 
fa0a4d6c605b8ed47796f68068d6273bef7fcaef test: remove assert_blockchain_height (MarcoFalke)

Pull request description:

  This simplifies the code and solves intermittent timeouts caused by commit 0d39b5848a7a341cd2b958336861cdd4098e2616.

  E.g. https://cirrus-ci.com/task/5196092369272832?command=ci#L3126

  ```
   test  2021-02-08T12:27:56.275000Z TestFramework (ERROR): Assertion failed
                                     Traceback (most recent call last):
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 127, in main
                                         self.run_test()
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/feature_assumevalid.py", line 180, in run_test
                                         self.assert_blockchain_height(self.nodes[0], 101)
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/feature_assumevalid.py", line 92, in assert_blockchain_height
                                         assert False, "blockchain too short after timeout: %d" % current_height
                                     AssertionError: blockchain too short after timeout: 101

ACKs for top commit:
  glozow:
    ACK fa0a4d6c60

Tree-SHA512: 3859b0c1581c21f03c775f119204cc3a219e5d86346883634886f6da46feaf254b9c6c49c1ec4581ffe409cdf05f6e91ec38c3976c3daf4a9e67f96ddc1e0dce
 2024-01-16 09:29:49 -06:00
MarcoFalke
63885189ae
Merge #20944: rpc: Return total fee in getmempoolinfo 
fa362064e383163a2585ffbc71ac1ea3bcc92663 rpc: Return total fee in mempool (MarcoFalke)

Pull request description:

  This avoids having to loop over the whole mempool to query each entry's fee

ACKs for top commit:
  achow101:
    ACK fa362064e383163a2585ffbc71ac1ea3bcc92663
  glozow:
    ACK fa362064e3 🧸
  jnewbery:
    ACK fa362064e383163a2585ffbc71ac1ea3bcc92663

Tree-SHA512: e2fa1664df39c9e187f9229fc35764ccf436f6f75889c5a206d34fff473fc21efbf2bb143f4ca7895c27659218c22884d0ec4195e7a536a5a96973fc9dd82d08
 2024-01-16 09:29:49 -06:00
fanquake
55d78e8b07
Merge #20346: script: modify security-check.py to use "==" instead of "is" for literal comparison 
b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e swapped "is" for "==" in literal comparison (Tyler Chambers)

Pull request description:

  In Python 3.8+ literal comparisons using "is" instead of "==" produce a SyntaxWarning [source](https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-python-behavior).

  I checked the entire devtools directory, this seems to be the only occurrence.

  This is a small fix, but removes the SyntaxWarning.
  Fixes: #20338

ACKs for top commit:
  hebasto:
    re-ACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e, only squashed since my [previous](https://github.com/bitcoin/bitcoin/pull/20346#pullrequestreview-525934568) review.
  practicalswift:
    re-ACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e: patch still looks correct
  theStack:
    utACK b6121edf70a8d50fd16ddbba0c3168e5e49bfc2e

Tree-SHA512: 82a43495d6552fbaa3b02b58f0930b049d27aa937fe44b47714e3c059f844cc494de20674557371cbccf24fb8873ecb7376fb965ae326847eed2b855ed2d59c6
 2024-01-16 07:57:32 -06:00
MacroFake
736d2bc23c
Merge bitcoin/bitcoin#25126: test: add BIP157 message parsing support (via MESSAGEMAP) 
5dc6d9207778c51c10c16fac4b3663bc7905bafc test: make BIP157 messages default-constructible (MESSAGEMAP compatibility) (Sebastian Falbesoner)
71e4cfefe765c58937b3fd3125782ca8407315d2 test: p2p: add missing BIP157 message types to MESSAGEMAP (Sebastian Falbesoner)

Pull request description:

  The script [message-capture-parser.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/message-capture/message-capture-parser.py) currently doesn't support parsing the BIP157 messages `getcfilters`, `getcfheaders` and `getcfcheckpt`, e.g.
  ```
  $ ./contrib/message-capture/message-capture-parser.py msgs_recv.dat
  ...
      WARNING - Unrecognized message type b'getcfcheckpt' in /home/thestack/bitcoin/msgs_recv.dat
  ...
  ```

  This PR fixes this by adding the missing message type mappings to the [`MESSAGEMAP`](225e5b57b2/test/functional/test_framework/p2p.py (L95-L127)) in the test framework and add default-constructors for the corresponding `msg_`... classes.

  Without the second commit, the following error message would occur:
  ```
    File "/home/thestack/bitcoin/./contrib/message-capture/message-capture-parser.py", line 141, in process_file
      msg = MESSAGEMAP[msgtype]()
  TypeError: __init__() missing 2 required positional arguments: 'filter_type' and 'stop_hash'
  ```

ACKs for top commit:
  dunxen:
    tACK [5dc6d92](5dc6d92077)

Tree-SHA512: d656c4d38a856373f01d7c293ae7d2b27378a9fc248048ebf2a64725ef8b498b3ddf4f420704abdb20d0c68ca548f1777602c5e73b66821a20c97ae618f1d63f
 2024-01-14 11:05:37 -06:00
MacroFake
b3343d73ab
Merge bitcoin/bitcoin#25124: test: Fix intermittent race in p2p_unrequested_blocks.py 
faac67cab02a755b0ce67716c5e5889432b13b83 test: Fix intermittent race in p2p_unrequested_blocks.py (MacroFake)

Pull request description:

  Disconnect may also result in an `OSError`, not only an `AssertionError`. Instead of maintaining a dead code path and enumerating disconnect reasons, just assume disconnection happens every time.

ACKs for top commit:
  jamesob:
    Code review ACK faac67cab0

Tree-SHA512: d2cec003168e421a5faed275cb2e1ef9fc63f9e8514f41d21da17e8964c79e5b453ccd72cd7ec62805f45293cf877be5bc8124ae98a515c0aa42d6e053409653
 2024-01-14 11:05:37 -06:00
MacroFake
431c1e3458
Merge bitcoin/bitcoin#25121: test: compare /mempool/info response with getmempoolinfo RPC 
1df42bc262d994c30d390ea73b232b8d2548899e test: compare `/mempool/info` response with `getmempoolinfo` RPC (brunoerg)

Pull request description:

  This PRs compares `/mempool/info` REST response with `getmempoolinfo` RPC in `interface_rest.py`.
  Similar to #24936 and #24797.

ACKs for top commit:
  theStack:
    ACK 1df42bc262d994c30d390ea73b232b8d2548899e

Tree-SHA512: 2de36d70fa61612e7976f875e55f98e78b1cdb909b48cff18e6a70c55eda34b799e210bcd55361ea947388b7778d867290a73be4f799bb36afd65423ad49c487
 2024-01-14 11:05:36 -06:00
laanwj
a245e52f55
Merge bitcoin/bitcoin#24793: test: Change color of skipped functional tests 
3258bad996262792ba77573d6080dafa3952929c changes color of skipped functional tests (Jacob P. Fickes)

Pull request description:

  changes the color of skipped functional tests (currently grey and can be hard to read/invisible on dark backgrounds) to yellow.

  resolves #24791

ACKs for top commit:
  theStack:
    Tested ACK 3258bad996262792ba77573d6080dafa3952929c
  jarolrod:
    Tested ACK 3258bad996

Tree-SHA512: 3fe5ae0d3b4902b2b6bda6e89ab780feb8bf4b7cb1ce7e8467057b94a1e0a26ddeaf3cac0bc19b06ef10d8bccaac9c495029d42740fbedab8fb0d5fdd7d02eaf
 2024-01-14 11:05:35 -06:00
laanwj
89c1e775c5
Merge bitcoin/bitcoin#24815: lint: convert lint-tests.sh to python 
ae0e06a439e09a7e24dd6198591a588c8df2d529 Converted lint-tests.sh to python (TakeshiMusgrave)

Pull request description:

  Reference issue: https://github.com/bitcoin/bitcoin/issues/24783

ACKs for top commit:
  laanwj:
    Tested ACK ae0e06a439e09a7e24dd6198591a588c8df2d529

Tree-SHA512: a118295b5b6b5199b52d46b54de871d88dd544112e7dd5001a9575d65b093af0aea390f9ad223462a4fc6a201bd8c4debe5e26bfa4860a90c97cfe300477c04a
 2024-01-13 19:32:31 -06:00
MarcoFalke
710ea6e114
Merge bitcoin/bitcoin#24936: test: compare /mempool/contents response with getrawmempool RPC 
bef61496ab5e12e38ac5794cd0836723af070ab5 test: compare `/mempool/contents` response with `getrawmempool` RPC (brunoerg)
5bc5cbaf310f60e89c72e8ecf3f6187c85499027 doc: add reference to `getrawmempool` RPC in `/mempool/contents` REST doc (brunoerg)

Pull request description:

  This PR is similar to #24797, it compares `/mempool/contents` REST response with `getrawmempool` RPC (verbose=True) since they use the same `MempoolToJSON` function.

  Also, adds a reference to `getrawmempool` RPC help to get details about the fields from `/mempool/contents`.

ACKs for top commit:
  0xB10C:
    ACK bef6149

Tree-SHA512: b7e9e9c765ee837986ba167b9234a9b95c9ef0a9ebcc2a03d50f6be6d3aba1480bd77c78111d95df1e4023cde6dfc64bf1e7908d9e5b6f96ca46b76611a4a9b4
 2024-01-13 19:32:31 -06:00
laanwj
31fae25f68
Merge bitcoin/bitcoin#24803: lint: convert submodule linter test to Python 
4a9e36dbaf96f83d0829f8442114a2fa36641776 lint: convert submodule linter test to Python (Eunoia)

Pull request description:

  Refs #24783

ACKs for top commit:
  laanwj:
    Tested ACK 4a9e36dbaf96f83d0829f8442114a2fa36641776

Tree-SHA512: ca25b59acf75cebc79588a6c51dc5c313c8d0bd1d492127815d7b81b36aaffd02815a515d97b355582002f71efc33d46435d0b28fce24497bb99799d9ba57228
 2024-01-13 19:32:31 -06:00
Wladimir J. van der Laan
4293cbdb53
Merge #20003: net: Exit with error message if -proxy is specified without arguments (instead of continuing without proxy server) 
9b4fa0af40cd88ed25dd77962235fbf268bdcaa7 net: Print error message if -proxy is specified without arguments (instead of continuing without proxy server) (practicalswift)

Pull request description:

  Exit with error message if `-proxy` is specified without arguments (instead of continuing without proxy server).

  Continuing without a proxy server when the end-user has specified `-proxy` may result in accidental loss of privacy. (The end-user might think he/she is using a proxy when he/she is not.)

  Before this patch:

  ```
  $ src/bitcoind -proxy
  …
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -listen=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -upnp=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -discover=0
  2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0
  …
  2020-09-23T00:24:33Z init message: Starting network threads...
  ```

  `bitcoind` is now running *without* a proxy server (`GetProxy(…, …) == false`, `HaveNameProxy() == false`, etc.).

  Note that the "-proxy set" log messages above which the end-user might interpret as "good, my traffic is now routed via the proxy".

  After this patch:

  ```
  $ src/bitcoind -proxy
  Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>.
  $ echo $?
  1
  ```

ACKs for top commit:
  laanwj:
    re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7
  kristapsk:
    ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7, I have tested the code.
  hebasto:
    re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7

Tree-SHA512: 4ba7a011991699a54b5bb87ec68367c681231bf5dcd36f8c89ff9ddc2e8d29df453817b7e362597e652ad6b341a22b7274be0fd78d435e5f0fd8058e5221c4ce
 2024-01-10 19:22:59 -06:00
Konstantin Akimov
a9401cca28
chore: update list of circular dependencies 
It is partial de-circularisation of dependencies between that includes net_processing

Classes that still depends on net_processing but should not:
 - llmq/dkgsessionmgr
 - llmq/signing
 - llmq/instantsend

They have asynchronous processing and with current impl that's impossible to do
 2024-01-10 15:12:08 -06:00
Konstantin Akimov
bb1d46c5cb
feat: new util class expected for return errors by more convenient way 
Source: https://github.com/TartanLlama/expected

Also it adds util/expected.h to exclude list of our linters
 2024-01-10 15:12:05 -06:00
fanquake
7a39d5d031
Merge #19172: test: Do not swallow flake8 exit code 
5d77549d8b287eb773db695b88c165ebe3be1005 doc: Add mypy to test dependencies (Hennadii Stepanov)
7dda912e1c28b02723c9f24fa6c4e9003d928978 test: Do not swallow flake8 exit code (Hennadii Stepanov)

Pull request description:

  After #18210 the `flake8` exit code in `test/lint/lint-python.sh` just not used that makes the linter broken.

  This PR:
  - combines exit codes of `flake8` and `mypy` into the  `test/lint/lint-python.sh` exit code
  - documents `mypy` as the test dependency

ACKs for top commit:
  MarcoFalke:
    Approach ACK 5d77549d8b287eb773db695b88c165ebe3be1005, fine with me
  practicalswift:
    ACK 5d77549d8b287eb773db695b88c165ebe3be1005

Tree-SHA512: e948ba04dc4d73393967ebf3c6a26c40d428d33766382a0310fc64746cb7972e027bd62e7ea76898b742a656cf7d0fcda2fdd61560a21bfd7be249cea27f3d41
 2024-01-10 12:07:54 -06:00
fanquake
271acac3a7
Merge #18210: test: type hints in Python tests 
bd7e530f010d43816bb05d6f1590d1cd36cdaa2c This PR adds initial support for type hints checking in python scripts. (Kiminuo)

Pull request description:

  This PR adds initial support for type hints checking in python scripts.

  Support for type hints was introduced in Python 3.5. Type hints make it easier to read and review code in my opinion. Also an IDE may discover a potential bug sooner. Yet, as PEP 484 says: "It should also be emphasized that Python will remain a dynamically typed language, and the authors have no desire to ever make type hints mandatory, even by convention."

  [Mypy](https://mypy.readthedocs.io/en/latest/index.html) is used in `lint-python.sh` to do the type checking. The package is standard so there is little chance that it will be abandoned. Mypy checks that type hints in source code are correct when they are not, it fails with an error.

  **Notes:**

  * [--ignore-missing-imports](https://mypy.readthedocs.io/en/latest/command_line.html#cmdoption-mypy-ignore-missing-imports) switch is passed on to `mypy` checker for now. The effect of this is that one does not need `# type: ignore` for `import zmq`. More information about import processing can be found [here](https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports). This can be changed in a follow-up PR, if it is deemed useful.
  * We are stuck with Python 3.5 until 04/2021 (see https://packages.ubuntu.com/xenial/python3). When Python version is bumped to 3.6+, one can change:
      ```python
      _opcode_instances = []  # type: List[CScriptOp]
      ```
      to
      ```python
      _opcode_instances:List[CScriptOp] = []
      ```
      for type hints that are **not** function parameters and function return types.

  **Useful resources:**

  * https://docs.python.org/3.5/library/typing.html
  * https://www.python.org/dev/peps/pep-0484/

ACKs for top commit:
  fanquake:
    ACK bd7e530f010d43816bb05d6f1590d1cd36cdaa2c - the type checking is not the most robust (there are things it fails to detect), but I think this is worth adopting (in a limited capacity while we maintain 3.5 compat).
  MarcoFalke:
    ACK bd7e530f010d43816bb05d6f1590d1cd36cdaa2c fine with me

Tree-SHA512: 21ef213915fb1dec6012f59ef17484e6c9e0abf542a316b63d5f21a7778ad5ebabf8961ef5fc8e5414726c2ee9c6ae07c7353fb4dd337f8fcef5791199c8987a
 2024-01-10 12:07:54 -06:00
MarcoFalke
49b1a468c7
Merge #19294: test: refactor: Inline adjust_bitcoin_conf_for_pre_17 
fa41b0a6dac7afd77e2b94eca6520ab3d2adc231 pep-8 test/functional/test_framework/util.py (MarcoFalke)
faa841bc979ca306f5ba4d5f7b78fcc427b8e413 test: refactor: Inline adjust_bitcoin_conf_for_pre_17 (MarcoFalke)

Pull request description:

  This removes mental and code complexity as well as attack surface for bikeshedding

ACKs for top commit:
  Sjors:
    utACK fa41b0a6dac7afd77e2b94eca6520ab3d2adc231

Tree-SHA512: 6e3c872e66d98ffaa7aecdfd64aa7dd8fbb51815a8fdaba170ce0772b4c3360084d0ebab4a5feac768ab5df50d04528d7daafc51ba07c15445c1ef94fa3efd34
 2024-01-10 12:07:52 -06:00
MarcoFalke
152246f21a
Merge #19178: Make mininode_lock non-reentrant 
62068381a3b9c065d81300be79abba7aecfdb41b [tests] Make mininode_lock non-reentrant (John Newbery)
c67c1f2c032a8efa141d776a7e5be58f052159ea [tests] Don't call super twice in P2PTxInvStore.on_inv() (John Newbery)
9d80762fa0931fe553fad241e95bcc1515ef0e95 [tests] Don't acquire mininode_lock twice in wait_for_broadcast() (John Newbery)
edae6075aa3b1169c84b65e76fd48d68242a294e [tests] Only acquire lock once in p2p_compactblocks.py (John Newbery)

Pull request description:

  There's no need for mininode_lock to be reentrant.
  Use a simpler non-recursive lock.

ACKs for top commit:
  MarcoFalke:
    ACK 62068381a3b9c065d81300be79abba7aecfdb41b 😃
  jonatack:
    ACK 62068381a3b9c0

Tree-SHA512: dcbc19e6c986970051705789be0ff7bec70c69cf76d5b468c2ba4cb732883ad512b1de5c3206c2eca41fa3f1c4806999df4cabbf67fc3c463bb817458e59a19c
 2024-01-10 12:07:52 -06:00
Konstantin Akimov
fc582937f1
chore: drop circular dependency coinjoin/client <-> wallet 2024-01-10 12:06:02 -06:00
UdjinM6
c25d9aed80
fix: A set of qdata/qwatch related fixes (#5745) 
## Issue being fixed or feature implemented
Fix/tidy up a few `qdata`/`qwatch` related parts, improve performance
for regular non-watching nodes

~based on #5744 atm~

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2024-01-09 21:31:41 -06:00
MarcoFalke
4143e359f7
(Partial) Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs 
a512925e19a70d7f6b80ac530a169f45ffaafa1c [doc] Release notes (Amiti Uttarwar)
50f94b34a33c954f6e207f509c93d33267a5c3e2 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9b509f0b10e4315c0bfa2da0cc0c31c22f [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa83a5436790c1a722a5609ac9d48df235f [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9ca40967d28ae16dfea9cccc6f3a6624a1 [log] Add connection type to log statement (Amiti Uttarwar)

Pull request description:

  After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.

  This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.

  Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093

ACKs for top commit:
  jnewbery:
    Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
  sipa:
    utACK a512925e19a70d7f6b80ac530a169f45ffaafa1c
  guggero:
    Tested and code review ACK a512925e.
  MarcoFalke:
    cr ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c 🌇
  promag:
    Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.

Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
 2024-01-09 08:15:36 -06:00
Wladimir J. van der Laan
ae97ed8a6f
Merge #19368: test: improve functional tests compatibility with BSD/macOS 
3a7e79478ab41af7c53ce14d9fca9815bffe1f73 test: retry when write to a socket fails on macOS (Ivan Metlushko)
8cf9d15b823d91d2a74fc83832fccca2219342c9 test: use pgrep for better compatibility (Ivan Metlushko)

Pull request description:

  Rationale: a few minor changes to make experience of running tests on macOS a bit better
  1.`pidof` is not available on BSD/macOS, while `pgrep` is present on BSD, Linux and macOS
  2. Add retry as a workaround for a weird behavior when writing to a socket (https://bugs.python.org/issue33450). Stacktrace attached

  Man pages:
  https://www.freebsd.org/cgi/man.cgi?query=pgrep&apropos=0&sektion=1&manpath=FreeBSD+6.0-RELEASE&arch=default&format=html
  https://man7.org/linux/man-pages/man1/pgrep.1.html

  Related to #19281

  Stacktrace example:
  ```
  ...
  33/161 - feature_abortnode.py failed, Duration: 63 s

  stdout:
  2020-06-11T10:46:43.947000Z TestFramework (INFO): Initializing test directory /var/folders/2q/d5w9zh614r7g5c8r74ln3g400000gq/T/test_runner_₿_🏃_20200611_174102/feature_abortnode_128
  2020-06-11T10:46:45.199000Z TestFramework (INFO): Waiting for crash
  2020-06-11T10:47:15.921000Z TestFramework (INFO): Node crashed - now verifying restart fails
  2020-06-11T10:47:47.068000Z TestFramework (INFO): Stopping nodes
  [node 1] Cleaning up leftover process

  stderr:
  Traceback (most recent call last):
    File "/Users/xxx/Projects/bitcoin/test/functional/feature_abortnode.py", line 50, in <module>
      AbortNodeTest().main()
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main
      exit_code = self.shutdown()
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 266, in shutdown
      self.stop_nodes()
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 515, in stop_nodes
      node.stop_node(wait=wait)
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_node.py", line 318, in stop_node
      self.stop(wait=wait)
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
      return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 142, in __call__
      response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
    File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
      self.__conn.request(method, path, postdata, headers)
    File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1107, in request
      self._send_request(method, url, body, headers)
    File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1152, in _send_request
      self.endheaders(body)
    File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1103, in endheaders
      self._send_output(message_body)
    File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 936, in _send_output
      self.send(message_body)
    File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 908, in send
      self.sock.sendall(data)
  OSError: [Errno 41] Protocol wrong type for socket
  ```

ACKs for top commit:
  laanwj:
    ACK 3a7e79478ab41af7c53ce14d9fca9815bffe1f73

Tree-SHA512: fefbe40ce94ab29f18bbbed2a434194b1384ffa5279b1d04db7a3708e3dd422bd9e450f1db3f95a1a851fac5a626ab533c6ebcfd7ede96f8ccae9e6f3e9fff92
 2024-01-06 19:30:15 -06:00
MarcoFalke
2d95e9284c
Merge #19014: test: Replace TEST_PREVIOUS_RELEASES env var with test_framework option 
fad798be76dd5e330463c837fda768477d536078 test: Default --previous-releases to false if dir is empty (MarcoFalke)
faf1c3cc58d14f86ba5364e6ee5c8ef29cac2e26 test: Replace TEST_PREVIOUS_RELEASES env var with test_framework option (MarcoFalke)

Pull request description:

  The "auto-detection" feature is kept in place, but making it an option allows to properly document it. For example, on my machine I get:

  ```
  $ ./test/functional/wallet_disable.py --help | grep previous-releases
    --previous-releases   Force test of previous releases (default: False)

ACKs for top commit:
  Sjors:
    re-tACK fad798b

Tree-SHA512: a7377d0d5378be0a50be278d76396cc403583617b5fc43467773eee706df698acf3f4e67651491183b9b43a8e1816b052e4c17b90272b7ec4b6ac134ad811400
 2024-01-06 19:30:14 -06:00
Konstantin Akimov
cf12572c67
fix: withdrawal (asset unlock) txes to use Platform Quorum on RegTest (#5800) 
## Issue being fixed or feature implemented
Asset Unlock tx uses platform's quorum on devnets, testnet, mainnet, but
still quorum type "Test (100)" on Reg Tests
That's part II PR, prior work is here:
https://github.com/dashpay/dash/pull/5618

## What was done?
- Removed `consensus.llmqTypeAssetLocks` which has been kept only for
RegTest - use `consensus.llmqTypePlatform` instead.
- Functional test `feature_asset_locks.py` uses `llmq_type_test = 106`
instead `llmq_type_test = 100` for asset unlock tx
- there's 4 MNs + 3 evo nodes instead 3 MNs as before: evo nodes
requires to have IS to be active


## How Has This Been Tested?
Run unit/functional tests


## Breaking Changes
Asset Unlock tx uses correct quorum "106 llmq_test_platform" on reg test
instead "100 llmq_test"

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
 2024-01-06 19:28:47 -06:00
UdjinM6
25111262cd
fix: ignore triggers from the past when voting (#5798) 
## Issue being fixed or feature implemented
we should not vote on triggers from the past

## What was done?

## How Has This Been Tested?
n/a

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2024-01-06 19:27:26 -06:00
MarcoFalke
5a18a539db
Merge bitcoin/bitcoin#24800: lint: convert lint-python-mutable-default-parameters.sh to Python 
e8e48fa82bdce3f0c1da0693148867befa221de7 Converted lint-python-mutable-default-parameters.sh to python (TakeshiMusgrave)

Pull request description:

  This converts one of the linter scripts to Python. Reference issue: https://github.com/bitcoin/bitcoin/issues/24783

  The approach is to just call git grep using subprocess.run.

  Alternative approaches could be to use Python instead of git grep (I'm not sure how) or use ```pylint --disable=all --enable=W0102```, though that requires installation of pylint.

ACKs for top commit:
  MarcoFalke:
    review ACK e8e48fa82bdce3f0c1da0693148867befa221de7

Tree-SHA512: 7f6f4887dee02c9751b225a6a131fb705868859c4a9af25bb3485cda2358650486b110f17adf89d96a20f212d7d94899922a07aab12c8dc11984cfd5feb7a076
 2024-01-02 11:17:47 -06:00
fanquake
5899f13660
Merge bitcoin/bitcoin#24790: lint: remove qt SIGNAL/SLOT lint 
b72925e7cea11522aca65580c136dbacb2753e83 lint: remove qt SIGNAL/SLOT lint (fanquake)

Pull request description:

  I think we are past the point where we need to lint for this, the CPU
  can probably be better utilized.

ACKs for top commit:
  laanwj:
    ACK b72925e7cea11522aca65580c136dbacb2753e83

Tree-SHA512: 3da6e4811cdd16ff64c7e26f641f7b24f0405cc86cec36666de58691d447eca8662c924df31c6c60b3523c13590bdc62205a3237b1b1794dd8cdef35519309b3
 2024-01-02 11:17:47 -06:00
MarcoFalke
6404d989df
Merge bitcoin/bitcoin#24707: doc: Speed up functional test runs using ramdisk 
17648493df478fa9316cc9ed66fe6bc1c2c820a4 doc: Speed up functional test runs using ramdisk (willcl-ark)

Pull request description:

  Using a ramdisk for the functional tests can give noticable speedups for developers and reviewers.

  Local testing with an 8GB ramdisk saw a full test run using `test/functional/test_runner.py --jobs=100 --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp` reduced from ~280 seconds to ~99 seconds.

  Possible bikeshedding opportunity to be had over whether this might best fit into `doc/productivity.md`, but IMO more people will likely see it (and it will therefore be more useful) if it is here.

  It seems best to select `tmpfs` over `ramfs` as `ramfs` can grow dynamically (good) but cannot be limited in size and might cause the system to hang if you run out of ram (bad), whereas `tmpfs` is size-limited and will overflow into swap.

ACKs for top commit:
  josibake:
    ACK 17648493df
  jamesob:
    ACK 17648493df

Tree-SHA512: b8e0846d4558a7a33fbb7cd190e30c36182db36095e1c1feae8c10a12042cff9d97739964bd9211d8564231dc99b4be5eed806d12a1d11dfa908157d7f26cc67
 2024-01-02 11:17:46 -06:00
MarcoFalke
592c4e30e7
Merge bitcoin/bitcoin#23795: refactor: Remove implicit-integer-sign-change suppressions in validation 
fadd73037e266edb844f0972e82e9213171ef214 refactor: Remove implicit-integer-sign-change suppressions in validation.cpp (MarcoFalke)

Pull request description:

  A file-wide suppression is problematic because it will wave through future violations, potentially bugs.

  Fix that by using per-statement casts.

ACKs for top commit:
  shaavan:
    ACK fadd73037e266edb844f0972e82e9213171ef214
  theStack:
    Code-review ACK fadd73037e266edb844f0972e82e9213171ef214

Tree-SHA512: a8a05613be35382b92d7970f958a4e8f4332432056eaa9d72f6719495134b93aaaeea692899d9035654d0e0cf56bcd759671eeeacfd0535582c0ea048ab58a56
 2023-12-26 22:26:20 -06:00
MarcoFalke
7d601cfa85
Merge bitcoin/bitcoin#23681: test: Remove false coinstatsindex test 
c055f6b216659b844c8dcd4ff2a977f181099678 test: Remove false coinstatsindex test (Fabian Jahr)

Pull request description:

  This test never actually tested the behavior that it describes in the comments. This was discovered in #21590 which seems to speed up muhash which lead to the test failing.

  I can vaguely remember that the described behavior was desired by some reviewers of `coinstatsindex`: That `coinstatsindex` should be aware of stale blocks and able to return statistics on them as well. The index actually does this for blocks that it sees while the index is active, i.e. while running `coinstatsindex` all blocks will be indexed and even when they become stale the index (via `gettxoutsetinfo`) will still return a result for them when given the right hash. But this currently does not work for blocks that the node saw and that became stale _before_ the node activated `coinstatsindex`. While the index syncs initially everything but the active chain is ignored and I don't see any indication that this ever worked differently in the past.

  Introducing this behavior seems non-trivial at first glance so, while I will give this a shot, I think the test should be removed so it does not confuse users and does not block #21590.

Top commit has no ACKs.

Tree-SHA512: b05f5dfeea3e453c8bb7c761501d0d896d4412a3f0c08037955951fae9fe388c63402da401792591e18da8fb67734f47f1a297d573cdb66e0ced451698718067
 2023-12-26 22:26:20 -06:00
MarcoFalke
d2b9631a90
Merge bitcoin/bitcoin#22492: wallet: Reorder locks in dumpwallet to avoid lock order assertion 
9b85a5e2f7e003ca8621feaac9bdd304d19081b4 tests: Test for dumpwallet lock order issue (Andrew Chow)
25d99e6511d8c43b2025a89bcd8295de755346a7 Reorder dumpwallet so that cs_main functions go first (Andrew Chow)

Pull request description:

  When a wallet is loaded which has an unconfirmed transaction in the mempool, it will end up establishing the lock order of cs_wallet -> cs_main -> cs_KeyStore. If `dumpwallet` is used on this wallet, then a lock order of cs_wallet -> cs_KeyStore -> cs_main will be used, which causes a lock order assertion. This PR fixes this by reordering `dumpwallet` and `GetKeyBirthTimes` (only used by `dumpwallet`). Specifically, in both functions, the function calls which lock cs_main are done prior to locking cs_KeyStore. This avoids the lock order issue.

  Additionally, I have added a test case to `wallet_dump.py`. Of course testing this requires `--enable-debug`.

  Fixes #22489

ACKs for top commit:
  MarcoFalke:
    review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 🎰
  ryanofsky:
    Code review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4. Nice to reduce lock scope, and good test!
  prayank23:
    tACK 9b85a5e2f7
  lsilva01:
    Tested ACK 9b85a5e2f7 under the same conditions reported in issue #22489 and the `dumpwallet` command completed successfully.

Tree-SHA512: d370a8f415ad64ee6a538ff419155837bcdbb167e3831b06572562289239028c6b46d80b23d227286afe875d9351f3377574ed831549ea426fb926af0e19c755
 2023-12-24 11:59:46 -06:00
MarcoFalke
9e5ee6ac52
Merge #20316: test: Fix wallet_multiwallet test issue on Windows 
fa00ff0399fe54f42d80daa04140d19bcfe0e2d8 test: Fix wallet_multiwallet test issue on Windows (MarcoFalke)

Pull request description:

  Fixes:

  ```
  Traceback (most recent call last):
    File "test\functional\test_framework\test_framework.py", line 126, in main
      self.run_test()
    File "test/functional/wallet_multiwallet.py", line 120, in run_test
      assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), sorted(in_wallet_dir))
    File "test\functional\test_framework\util.py", line 49, in assert_equal
      raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
  AssertionError: not(['', 'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8'] == ['', 'sub/w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8'])

ACKs for top commit:
  promag:
    ACK fa00ff0399fe54f42d80daa04140d19bcfe0e2d8.

Tree-SHA512: 7a809a352677a216465cef59e866e4881272e302e897cebf7d9645bf87aebeaf54435bb0692bb5c1381c2dd680e8a34e640ea18ca6e2a4087e3233cd9c24ed04
 2023-12-24 11:59:43 -06:00
Odysseas Gabrielides
25cef45858
feat(rpc): gettxchainlocks should return mempool=false when tx not in mempool (#5742) 
## Issue being fixed or feature implemented
Platform (in the scope of Withdrawals) need to be aware if a tx isn't in
mempool when requesting status of a tx using RPC `gettxchainlocks`.
cc @markin-io

## What was done?

- mempool is passed to `GetTransaction` and saving the result for
checking latter.
- If the returned tx_ref is nullptr, then the RPC returns null for the
corresponding tx in the array.

Example: 
`tx1` is mined and chainlocked, `tx2` is in mempool and `tx3` doesn't
exist.
The result now is:
`[
  {
    "height": 830,
    "chainlock": false,
    "mempool": true
  },
  {
    "height": -1,
    "chainlock": false,
    "mempool": true
  },
  {
    "height": -1,
    "chainlock": false,
    "mempool": false
  }
]`

## How Has This Been Tested?


## Breaking Changes


## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-12-24 11:58:14 -06:00
Odysseas Gabrielides
563cc34b4e
feat(rpc): Asset Unlock status by index (#5776) 
## Issue being fixed or feature implemented
Platform in the scope of credit withdrawals, need a way to get the
status of an Asset Unlock by index.

## What was done?
A new RPC was created `getassetunlockchainlocks` that accepts Asset
Unlock indexes array as parameter and return corresponding status for
each index.

The possible outcomes per each index are:
- `chainlocked`: If the Asset Unlock index is mined on a Chainlocked
block.
- `mined`: If no Chainlock information is available, and the Asset
Unlock index is mined.
- `mempooled`: If the Asset Unlock index is in the mempool.
- `unknown`: If none of the above are valid.

Note: This RPC is whitelisted for the Platform RPC user.

## How Has This Been Tested?
Inserted on `feature_asset_locks.py` covering cases where Asset Unlock
txs are in mempool, mined and not present.

## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
 2023-12-22 14:27:00 -06:00
Konstantin Akimov
4083fff0b2
refactor: drop circular dependency governance/object <-> governance/validators 2023-12-21 23:04:43 -06:00
Konstantin Akimov
7e13727738
refactor: drop circular dependency validationinterface <-> governance/object 2023-12-21 23:04:43 -06:00
Konstantin Akimov
70fa626381 refactor: drop dependency validation on llmq/utils 2023-12-21 23:02:31 -06:00
UdjinM6
647f4831c1
chore: update file permissions in tests, add missing executable flag (#5778) 
## Issue being fixed or feature implemented
old mode 100644
new mode 100755

## What was done?
`chmod +x test/functional/*.py`

## How Has This Been Tested?
can now run these test directly e.g. `./test/functional/rpc_quorum.py`

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-12-19 08:03:11 -06:00
UdjinM6
9a99a4abdc
fix(rpc): pass blockhash into TxToJSON so that getspecialtxes could show correct instantlock/chainlock values (#5774) 
## Issue being fixed or feature implemented
`instantlock` and `chainlock` are broken in `getspecialtxes`

kudos to @thephez for finding the issue

## What was done?
pass the hash and also rename the variable to self-describing

## How Has This Been Tested?
run `getspecialtxes` on a node with and without the patch

## Breaking Changes
`instantlock` and `chainlock` will show actual values and not just
`false` all the time now (not sure if that qualifies for "breaking"
though)

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-12-19 07:43:36 -06:00
Odysseas Gabrielides
3bc77a6e1d
feat(rpc): submit chainlock signature if needed RPC (#5765) 
## Issue being fixed or feature implemented
Once Platform is live, there could be an edge case where the CL could
arrive to an EvoNode faster through Platform quorum than regular P2P
propagation.

## What was done?
This PR introduces a new RPC `submitchainlock` with the following 3
mandatory parameters:
- `blockHash`, `signature` and `height`.

Besides some basic tests:
- If the block is unknown then the RPC returns an error (could happen if
the node is stucked)
- If the signature is not verified then the RPC return an error.
- If the node already has this CL, the RPC returns true.
- If the node doesn't have this CL, it inserts it, broadcast it through
the inv system and return true.

## How Has This Been Tested?
`feature_llmq_chainlocks.py` was modified with the following scenario:

1. node0 is isolated from the rest of the network
2. node1 mines a new block and waits for CL
3. Make sure node0 doesn't know the new block/CL (by checking
`getbestchainlock()`)
4. CL is submitted via the new RPC on node0
5. checking `getbestchainlock()` and make sure the CL was processed +
'known_block' is false
6. reconnect node0

## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
 2023-12-18 22:27:19 -06:00
Vijay Das Manikpuri
38b9074275 (partial) Merge #24203: doc: Fix typos pointed out by lint-spelling 2023-12-11 15:48:44 -06:00
MarcoFalke
8b98e1ab7b Merge bitcoin/bitcoin#18795: Test: wallet issue with orphaned rewards 
e4356f6a6c18e5027a064a4d3a5deda27985f584 Testcase for wallet issue with orphaned rewards. (Daniel Kraft)

Pull request description:

  This adds a new test case demonstrating the wallet issue when block rewards are orphaned (#14148).

ACKs for top commit:
  LarryRuane:
    ACK e4356f6a6c18e5027a064a4d3a5deda27985f584
  leonardojobim:
    reACK e4356f6a6c .

Tree-SHA512: e9a2310ee1b3d52cfa302f431ed3d272bbc1b9195439ff318d9eb1006c0b28968dbe840e1600b6ff185e5d7ea57e4dcc837cef16051b5537445e10bc363b8c22
 2023-12-08 21:16:00 +03:00
MarcoFalke
b8267bb5a2 Merge #17556: test: Change feature_config_args.py not to rely on strange regtest=0 behavior 
ff44cae279bef7997f76db18deb1e41b39f05cb6 test: Change feature_config_args.py not to rely on strange regtest=0 behavior (Russell Yanofsky)

Pull request description:

  Update test to simply generate a normal mainnet configuration file instead of using a crazy setup where a regtest=1 config file using an includeconf in the [regtest] section includes another config file that specifies regtest=0, retroactively switching the network to mainnet.

  This setup was fragile and only worked because the triggered InitError happened early enough that none of the ignored [regtest] options mattered (only affecting log output).

  This change was originally made as part of #17493

Top commit has no ACKs.

Tree-SHA512: 3f77305454f04438493dfc2abd78a00434b30869454d1c3f54587b9c1f63239c49c90fb3b4d3a777ad130f2184e0f2dac87cee4cd23c50f1b3496a375943da01
 2023-12-08 21:16:00 +03:00
MarcoFalke
cff1c7b2c3 Merge #21043: net: Avoid UBSan warning in ProcessMessage(...) 
3ddbf22ed179a2db733af4b521bec5d2b13ebf4b util: Disallow negative mocktime (MarcoFalke)
f5f2f9716885e7548809e77f46b493c896a019bf net: Avoid UBSan warning in ProcessMessage(...) (practicalswift)

Pull request description:

  Avoid UBSan warning in `ProcessMessage(...)`.

  Context: https://github.com/bitcoin/bitcoin/pull/20380#issuecomment-770427182 (thanks Crypt-iQ!)

ACKs for top commit:
  MarcoFalke:
    re-ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b only change is adding patch written by me
  ajtowns:
    ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b -- code review only

Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9
 2023-12-08 21:16:00 +03:00
MarcoFalke
1c0cb3e8cc Merge bitcoin/bitcoin#22418: release: Remove gitian 
ab9c34237ab7b056394e0bd1f7cb131ffd95754c release: remove gitian (fanquake)

Pull request description:

  Note that this doesn't yet touch any glibc back compat related code.

ACKs for top commit:
  laanwj:
    Code review ACK ab9c34237ab7b056394e0bd1f7cb131ffd95754c

Tree-SHA512: 8e2fe3ec1097f54bb11ab9136b43818d90eab5dbb0a663ad6a552966ada4bdb49cc12ff4e66f0ec0ec5400bda5c81f3a3ce70a9ebb6fe1e0db612da9f00a51a7
 2023-12-06 12:40:58 -06:00
Wladimir J. van der Laan
3f77d2312f Merge #16525: Dump transaction version as an unsigned integer in RPC/TxToUniv 
e80259f1976545e4f1ab6a420644be0c32261773 Additionally treat Tx.nVersion as unsigned in joinpsbts (Matt Corallo)
970de70bdd3542e75b73c79b06f143168c361494 Dump transaction version as an unsigned integer in RPC/TxToUniv (Matt Corallo)

Pull request description:

  Consensus-wise we already treat it as an unsigned integer (the
  only rules around it are in CSV/locktime handling), but changing
  the underlying data type means touching consensus code for a
  simple cleanup change, which isn't really worth it.

  See-also, https://github.com/rust-bitcoin/rust-bitcoin/pull/299

ACKs for top commit:
  sipa:
    ACK e80259f1976545e4f1ab6a420644be0c32261773
  practicalswift:
    ACK e80259f1976545e4f1ab6a420644be0c32261773
  ajtowns:
    ACK e80259f1976545e4f1ab6a420644be0c32261773 code review -- checked all other uses of tx.nVersion treat it as unsigned (except for policy.cpp:IsStandard anyway), so looks good.
  naumenkogs:
    ACK e80259f

Tree-SHA512: 6760a2c77e24e9e1f79a336ca925f9bbca3a827ce02003c71d7f214b82ed3dea13fa7d9f87df9b9445cd58dff8b44a15571d821c876f22f8e5a372a014c9976b
 2023-12-06 12:33:15 -06:00
MarcoFalke
a800821e9f Merge #18965: tests: implement base58_decode 
60ed33904cf974e8f3c1b95392a23db1fe2d4a98 tests: implement base58_decode (10xcryptodev)

Pull request description:

  implements TODO: def base58_decode

ACKs for top commit:
  ryanofsky:
    Code review ACK 60ed33904cf974e8f3c1b95392a23db1fe2d4a98. Just suggested changes since last review. Thank you for taking suggestions!

Tree-SHA512: b3c06b4df041a6d88033cd077a093813a688e42d0b9aa777c715e5fd69cfba7b1bf984428bd98417d3c15232d3d48bc9c163317564f9e1d562db6611c21e2c10
 2023-12-06 12:33:15 -06:00
Wladimir J. van der Laan
e29a35a997 Merge #18309: zmq: Add support to listen on multiple interfaces 
e66870c5a4c2adbd30dca67d409fd5cd98697587 zmq: Append address to notify log output (nthumann)
241803da211265444e65f254f24dd184f2457fa9 test: Add zmq test to support multiple interfaces (nthumann)
a0b2e5cb6aa8db0563fac7d67a949b9baefe3a25 doc: Add release notes to support multiple interfaces (nthumann)
b1c3f180ecb63f3960506d202feebaa4271058ae doc: Adjust ZMQ usage to support multiple interfaces (nthumann)
347c94f551c3f144c44e00373e4dd61ff6d908b7 zmq: Add support to listen on multiple interfaces (Nicolas Thumann)

Pull request description:

  This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server.
  Currently, if you specify more than one e.g. `zmqpubhashblock` paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. `zmqpubhashblock=0.0.0.0:28332`), which can result in an increased attack surface.
  With this PR a user can specify multiple interfaces to listen on, e.g.
  `-zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332`.

ACKs for top commit:
  laanwj:
    Code review ACK e66870c5a4c2adbd30dca67d409fd5cd98697587
  instagibbs:
    reACK e66870c5a4

Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
 2023-12-06 12:33:15 -06:00
UdjinM6
8a888fb6ee
fix: improve qgetdata/qdata tests (#5744) 
## Issue being fixed or feature implemented
The test is a bit broken and incomplete, some testing scenarios aren't
realistic

## What was done?
pls see individual commits

## How Has This Been Tested?
run it

## Breaking Changes
n/a, tests only

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-12-06 11:48:21 -06:00
MarcoFalke
f7f29d72fa follow-up Merge #14559: appveyor: Enable multiwallet tests - adds missing changes for wallet_multiwallet.py test 
4dca7d0a98 appveyor: Enable multiwallet test (Chun Kuan Lee)

Pull request description:

  Based on #14320

  This PR enable multiwallet test on appveyor. Also re-enable symlink tests on Windows which is available after Windows Vista.

  I disable these tests in #13964 because I suppose that Windows does not support symlink, but I was wrong.

Tree-SHA512: 852cd4dedf36ec9c34aff8926cb34e6a560aea0bb9170c7a2264fc292dbb605622d561568d8df39aeb90d3d2bb700901d218ea7e7c5e21d84827c40d6370b369
 2023-12-06 11:46:53 -06:00
Samuel Dobson
f6f9b9851f Merge #17219: wallet: allow transaction without change if keypool is empty 
92bcd70808b9cac56b184903aa6d37baf9641b37 [wallet] allow transaction without change if keypool is empty (Sjors Provoost)
709f8685ac37510aa145ac259753583c82280038 [wallet] CreateTransaction: simplify change address check (Sjors Provoost)
5efc25f9638866941028454cfa9bae27f1519cb4 [wallet] translate "Keypool ran out" message (Sjors Provoost)

Pull request description:

  Extracted from #16944

  First this PR simplifies the check when generating a change address, by dropping `CanGetAddresses` and just letting `reservedest.GetReservedDestination` do this check.

  Second, when the keypool is empty, instead of immediately giving up, we create a dummy change address and pass that to coin selection. If we didn't need the change address (e.g. when spending the entire balance), then it's all good. If we did need a change address, we throw the original error.

ACKs for top commit:
  fjahr:
    Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  jonasschnelli:
    utACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  achow101:
    ACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  meshcollider:
    Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37

Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
 2023-12-06 11:46:53 -06:00
Wladimir J. van der Laan
9fd9e4cf5a Merge #20567: test: Add option to git-subtree-check to do full check, add help 
34c80d9eee7d21755f2bb80f7c97fd30d2c7b656 test: Add option to git-subtree-check to do full check, add help (Wladimir J. van der Laan)

Pull request description:

  This adds a brief help text to `git-subtree-check.sh` and adds an option to do a full remote check instead of having two different code paths with a successful exit status. Also make it explicit that the CI is not doing this.

ACKs for top commit:
  fjahr:
    tested ACK 34c80d9eee7d21755f2bb80f7c97fd30d2c7b656

Tree-SHA512: 20f672fd3b3c1d633eccf9998fdd738194cdd7d10cc206691f2dcc28bbbf8187b8d06b87814f875a06145b179f5ca1f4f4f9922972be72759cf5ac6e0c11abd1
 2023-12-06 11:40:14 -06:00
Wladimir J. van der Laan
f706a562a7 Merge #19258: doc: improve subtree check instructions 
a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413 doc: improve subtree check instructions (Sjors Provoost)

Pull request description:

  Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.

  This PR also improves the error message if the subtree commit can't be found.

ACKs for top commit:
  laanwj:
    ACK a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413
  fanquake:
    ACK a4a3fc4cd2e6f53cdffcc2962fd152a4e40c7413 - this looks ok.

Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
 2023-12-06 11:40:14 -06:00
MarcoFalke
17b67ba224 Merge #11394: Perform a weaker subtree check in Travis 
487aff421 Check subtree consistency in Travis (Pieter Wuille)
e1d0cc23a Improve git-subtree-check.sh (Pieter Wuille)

Pull request description:

  Apparently many of our subtrees get modified by PRs in this repository, without getting noticed.

  To improve upon this:
  * Make git-subtree-check.sh capable of doing a weaker consistency check (that doesn't need access to external repositories), but which should be sufficient to detect unintended changes. It can be fooled by a fake subtree merge commit, but that would hopefully be obvious to reviewers.
  * Make Travis invoke this subtree check for each of our subtrees.

  Note that Travis is currently expected to fail on this PR, as 2 out of 4 subtrees (`src/secp156k1` and `src/univalue` have been modified directly in master).

Tree-SHA512: 465b680392d3daf38a8c1dda77d6f74b1d1c23324c378774777fb95aa673e119a8f7e3ccc124e41d97b5ac8975f3d79f3015797d2d309666582394364917ec4e
 2023-12-06 11:40:14 -06:00
fanquake
0eba155aa7 Merge #19072: doc: Expand section on Getting Started 
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0caf510d4836a20194892ef9c71426c51 doc: Fix unrelated typos reported by codespell (MarcoFalke)

Pull request description:

  Some random doc changes:

  * Add sections to docs, so that they can be linked to
  * Explain that anyone (even maintainers) are allowed to work on good first issues
  * Expand section on Getting Started slightly

ACKs for top commit:
  hebasto:
    ACK facef3d4131f9980a4516282f11731361559509c
  fanquake:
    ACK facef3d4131f9980a4516282f11731361559509c

Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
 2023-12-06 11:40:14 -06:00
fanquake
2da9982e55 Merge #17829: scripted-diff: Bump copyright of files changed in 2019 
aaaaad6ac95b402fe18d019d67897ced6b316ee0 scripted-diff: Bump copyright of files changed in 2019 (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0
  promag:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 🎉
  fanquake:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 - going to merge this now because the year is over and conflicts are minimal.

Tree-SHA512: 58cb1f53bc4c1395b2766f36fabc7e2332e213780a802762fff0afd59468dad0c3265f553714d761c7a2c44ff90f7dc250f04458f4b2eb8eef8b94f8c9891321
 2023-12-06 11:40:14 -06:00
MarcoFalke
921c93db66 Merge #16973: test: Fix combine_logs.py for AppVeyor build 
d478a472eb0d666e8a762ed8d24fafbabc5f94f3 test: Fix combine_logs.py for AppVeyor build (Martin Zumsande)

Pull request description:

  Fixes #16894

  This fixes the problem of AppVeyor builds not showing `debug.log` if a functional test fails, because the windows separator `\` doesn't work together with the regex in `combine_logs.py`.

  A fix was already attempted in  #16896, however, that PR became inactive and was marked "up for grabs", plus it's a really small change.

  As suggested by jamesob, this PR uses `pathlib`: For the glob and to convert the path to a posix-style string, it leaves the regex as is (in contrast to #16896 which adjusted the regex).

  I tested this locally on Windows and Ubuntu.

Top commit has no ACKs.

Tree-SHA512: 603b4359b6009b6da874c30f69759acda03730ee5747898a0fe957a5fc37ee9ba07858c6aa2169bf4c40521f37e47138e8314d698652ea2760fa0a3f76b890bd
 2023-12-06 11:40:14 -06:00
MarcoFalke
9d4282cb72 partial Merge #15893: Add test for superfluous witness record in deserialization 
cc556e4a30 Add test for superfluous witness record in deserialization (Gregory Sanders)
25b0786581 Fix missing input template by making minimal tx (Gregory Sanders)

Pull request description:

  Adds coverage for changed behavior in https://github.com/bitcoin/bitcoin/pull/14039

ACKs for commit cc556e:
  MarcoFalke:
    utACK cc556e4a30b4a32eab6722f590489d89b2875de3

Tree-SHA512: 3404c8f75e87503983fac5ae27d877309eb3b902f2ec993762911c71610ca449bef0ed98bd17e029414828025b2713e1bd012e63b2a06497e34f1056acaa6321
 2023-12-06 11:40:14 -06:00
MarcoFalke
549a358fe2 Merge #15102: test: Run invalid_txs.InputMissing test in feature_block 
fac4e731a8 test: Run invalid_txs.InputMissing test in feature_block (MarcoFalke)

Pull request description:

Tree-SHA512: 24c3f519ba0cf417b66e0df6f5ddc0430e3f419af4705a9c85096da47ff4d8f51487d65b68f3f993800003b3f936d95d8a0bade846e1b45f95b2bdbecc9ebab7
 2023-12-06 11:40:14 -06:00
MarcoFalke
56af7d6727
Merge bitcoin/bitcoin#22313: test: Add missing sync_all to feature_coinstatsindex 
fafd9165e911bf33d6212ca8a613b71878c82449 test: Add missing sync_all to feature_coinstatsindex (MarcoFalke)

Pull request description:

  Sync the blocks before invalidating them to ensure all nodes are on the right tip. Otherwise nodes[0] might stay on the "stale" block and the test fails (intermittently)

ACKs for top commit:
  jamesob:
    crACK fafd9165e9

Tree-SHA512: ca567b97b839b56c91d52831eaac18d8c843d376be90c9fd8b49d2eb4a46b801a1d2402996d5dfe2bef3e2c9bd75d19ed443e3f42cc4679c5f20043ba556efc8
 2023-12-03 20:45:01 -06:00
MarcoFalke
ec4cbc28d6
Merge #20954: test: Declare nodes type in test_framework.py. 
5353b0c64d32e44fc411464e080d4b00fae7124e Change type definitions for "chain" and "setup_clean_chain" from type comments to Python 3.6+ types. Additionally, set type for "nodes". (Kiminuo)

Pull request description:

  ### Motivation

  When I wanted to understand better https://github.com/bitcoin/bitcoin/pull/19145/files#diff-4bebbd3b112dc222ea7e75ef051838ceffcee63b9e9234a98a4cc7251d34451b test, I noticed that navigation in PyCharm/VS Code did not work for `nodes` variable. I think this is frustrating, especially for newcomers.

  ### Summary

  * This PR modifies Python 3.5 [type comments](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#variables) to Python 3.6+ types and adds a proper type for `nodes` [instance attribute](https://mypy.readthedocs.io/en/stable/class_basics.html#instance-and-class-attributes).
  * This PR does not change behavior.
  * This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.

  ### End result

  1. Open `test/functional/feature_abortnode.py`
  2. Move your caret to: `self.nodes[0].generate[caret here](3)`
  3. Use "Go to definition" [F12] should work now.

  I have tested this on PyCharm (Windows, Ubuntu) and VS Code (Windows, Ubuntu).

  Note: Some `TestNode` methods (e.g. `self.nodes[0].getblock(...)` ) use `__call__` mechanism and navigation does not work for them even with this PR.

ACKs for top commit:
  laanwj:
    ACK 5353b0c64d32e44fc411464e080d4b00fae7124e
  theStack:
    ACK 5353b0c64d32e44fc411464e080d4b00fae7124e

Tree-SHA512: 821773f052ab9b2889dc357d38c59407a4af09e3b86d7134fcca7d78e5edf3a5ede9bfb37595ea97caf9ebfcbda372bcf73763b7f89b0677670f21b3e396a12b
 2023-12-03 20:44:55 -06:00
MarcoFalke
7fbc0f359c Merge bitcoin/bitcoin#14604: tests: Add test and refactor feature_block.py 
55311197c483477b79883da5da09f2bc71acc7cf Added new test for future blocks reacceptance (sanket1729)
511a5af4622915c236cfb11df5234232c2983e45 Fixed inconsistencies between code and comments (sanket1729)

Pull request description:

  This Commit does 3 things:
  1) Adds a test case for checking reacceptance a previously rejected block which
  was too far in the future.
  ~~2) clean up uses of rehash or calc_sha256 where it was not needed~~
  3) While constructing block 44, this commit makes the code consistent with the expected figure in
  the comment just above it by adding a transaction to the block.
  4) Fix comment describing `sign_tx()` function

ACKs for top commit:
  duncandean:
    reACK 5531119
  brunoerg:
    reACK 55311197c483477b79883da5da09f2bc71acc7cf

Tree-SHA512: d40c72fcdbb0b2a0715adc58441eeea08147ee2ec5e371a4ccc824ebfdc6450698bd40aaeecb7ea7bfdb3cd1b264dd821b890276fff8b8d89b7225cdd9d6b546
 2023-12-03 20:32:22 -06:00
MarcoFalke
d667e57dc6 Merge #20466: test: Fix intermittent p2p_fingerprint issue 
fad7be584ffaf8099cc099d9378ba831c9483260 test: Fix intermittent p2p_finerprint issue (MarcoFalke)

Pull request description:

  A single sync_with_ping can't be used to drop a block announcement, as the block might be sent *after* the ping has been responded to.

  Fix that by waiting for the block.

ACKs for top commit:
  theStack:
    ACK fad7be584ffaf8099cc099d9378ba831c9483260

Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
 2023-12-03 20:32:22 -06:00
MarcoFalke
f4ea109e65 (partial) Merge bitcoin/bitcoin#21562: [net processing] Various tidying up of PeerManagerImpl ctor 
fde1bf4f6136638e84cdf9806eedaae08e841bbf [net processing] Default initialize m_recent_confirmed_transactions (John Newbery)
37dcd12d539e4a875581fa049aa0f7fafeb932a4 scripted-diff: Rename recentRejects (John Newbery)
cd9902ac5054c01228d52616bf85f7196364d4ff [net processing] Default initialize recentRejects (John Newbery)
a28bfd1d4cfa523a6abf3832dbfd6183cd546944 [net processing] Default initialize m_stale_tip_check_time (John Newbery)
9190b01d8dcf03b74e9b9e1653688a97ac171b37 [net processing] Add Orphanage empty consistency check (John Newbery)

Pull request description:

  - Use default initialization of PeerManagerImpl members where possible
  - Remove unique_ptr indirection where it's not needed

ACKs for top commit:
  MarcoFalke:
    ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf 👞
  theStack:
    re-ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf

Tree-SHA512: 7ddedcc972df8e933e1fbe5c88b8ea17df89e1e58fc769518512c5540e49dc8eddb3f47e78d1329a6fc5644d2c1d11c981f681fd633f5218bfa4b3e6a86f3d7b
 2023-12-03 20:25:16 -06:00
MarcoFalke
42aea04810 Merge bitcoin/bitcoin#22530: log: sort logging categories alphabetically 
d596dba9877e7ead3fb5426cbe7e608fbcbfe3eb test: assert logging categories are sorted in rpc and help (Jon Atack)
17bbff3b88132c0c95b29b59100456b85e26df75 log, refactor: use guard clause in LogCategoriesList() (Jon Atack)
7c57297319bc386afaf06528778384fe58576ef9 log: sort LogCategoriesList and LogCategoriesString alphabetically (Jon Atack)
f720cfa824f1be863349e7016080f8fb1c3c76c2 test: verify number of categories returned by logging RPC (Jon Atack)

Pull request description:

  Sorting the logging categories seems more user-friendly with the number of categories we now have, allowing CLI users to more quickly find a particular category.

  before
  ```
  $ bitcoin-cli help logging
  ...
  The valid logging categories are: net, tor, mempool, http, bench, zmq, walletdb, rpc, estimatefee, addrman, selectcoins, reindex, cmpctblock, rand, prune, proxy, mempoolrej, libevent, coindb, qt, leveldb, validation, i2p, ipc

  $ bitcoind -h | grep -A8 "debug=<category>"
    -debug=<category>
         ...
         output all debugging information. <category> can be: net, tor,
         mempool, http, bench, zmq, walletdb, rpc, estimatefee, addrman,
         selectcoins, reindex, cmpctblock, rand, prune, proxy, mempoolrej,
         libevent, coindb, qt, leveldb, validation, i2p, ipc.

  $ bitcoin-cli logging [] '["addrman"]'
  {
    "net": false,
    "tor": true,
    "mempool": false,
    "http": false,
    "bench": false,
    "zmq": false,
    "walletdb": false,
    "rpc": false,
    "estimatefee": false,
    "addrman": false,
    "selectcoins": false,
    "reindex": false,
    "cmpctblock": false,
    "rand": false,
    "prune": false,
    "proxy": true,
    "mempoolrej": false,
    "libevent": false,
    "coindb": false,
    "qt": false,
    "leveldb": false,
    "validation": false,
    "i2p": true,
    "ipc": false
  }
  ```

  after

  ```
  $ bitcoin-cli help logging
  ...
  The valid logging categories are: addrman, bench, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb, libevent, mempool, mempoolrej, net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor, validation, walletdb, zmq

  $ bitcoind -h | grep -A8 "debug=<category>"
    -debug=<category>
         ...
         output all debugging information. <category> can be: addrman,
         bench, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb,
         libevent, mempool, mempoolrej, net, proxy, prune, qt, rand,
         reindex, rpc, selectcoins, tor, validation, walletdb, zmq.

  $ bitcoin-cli logging [] '["addrman"]'
  {
    "addrman": false,
    "bench": false,
    "cmpctblock": false,
    "coindb": false,
    "estimatefee": false,
    "http": false,
    "i2p": false,
    "ipc": false,
    "leveldb": false,
    "libevent": false,
    "mempool": false,
    "mempoolrej": false,
    "net": false,
    "proxy": false,
    "prune": false,
    "qt": false,
    "rand": false,
    "reindex": false,
    "rpc": false,
    "selectcoins": false,
    "tor": false,
    "validation": false,
    "walletdb": false,
    "zmq": false
  }
  ```

ACKs for top commit:
  theStack:
    re-ACK d596dba9877e7ead3fb5426cbe7e608fbcbfe3eb

Tree-SHA512: d546257f562b0a288d1b19a028f1a510aaf21bd21da058e7c84653d305ea8662ecb4647ebefd2b97411f845fe5b0b841d40d3fe6814eefcb8ce82df341dfce22
 2023-12-03 20:25:16 -06:00
MarcoFalke
0845e1956e Merge bitcoin/bitcoin#22139: test: add type annotations to util.get_rpc_proxy 
fbeb8c43bc5bce131e15eb9e162ea457bfe2b83e test: add type annotations to util.get_rpc_proxy (fanquake)

Pull request description:

  Split out from #22092 while we address the functional test failure.

ACKs for top commit:
  instagibbs:
    ACK fbeb8c43bc

Tree-SHA512: 031ef8703202ae5271787719fc3fea8693574b2eb937ccf852875de95798d7fa3c39a8db7c91993d0c946b45d9b4d6de570bd1102e0344348784723bd84803a8
 2023-12-03 20:13:09 -06:00
MarcoFalke
fbddd23cd4 Merge bitcoin/bitcoin#22528: refactor: move GetTransaction to node/transaction.cpp 
f685a13bef0418663015ea6d8f448f075510c0ec doc: GetTransaction()/getrawtransaction follow-ups to #22383 (John Newbery)
abc57e1f0882a1a2bb20474648419979af6e383d refactor: move `GetTransaction(...)` to node/transaction.cpp (Sebastian Falbesoner)

Pull request description:

  ~This PR is based on #22383, which should be reviewed first~ (merged by now).

  In [yesterday's PR review club session to PR 22383](https://bitcoincore.reviews/22383), the idea of moving the function `GetTransaction(...)` from src/validation.cpp to src/node/transaction.cpp came up. With this, the circular dependency "index/txindex -> validation -> index/txindex" is removed (see change in `lint-circular-dependencies.sh`). Thanks to jnewbery for suggesting and to sipa for providing historical background.

  Relevant IRC log:
  ```
  17:52 <jnewbery> Was anyone surprised that GetTransaction() is in validation.cpp? It seems to me that node/transaction.cpp would be a more appropriate place for it.
  17:53 <raj_> jnewbery, +1
  17:53 <stickies-v> agreed!
  17:54 <glozow> jnewbery ya
  17:54 <jnewbery> seems weird that validation would call into txindex. I wonder if we remove this function, then validation would no longer need to #include txindex
  17:54 <sipa> GetTransaction predates node/transaction.cpp, and even the generic index framework itself :)
  17:55 <sipa> (before 0.8, validation itself used the txindex)
  17:55 <jnewbery> (and GetTransaction() seems like a natural sibling to BroadcastTransaction(), which is already in node/transaction.cpp)
  17:55 <jnewbery> sipa: right, this is not meant as a criticism of course. Just wondering if we can organize things a bit more rationally now that we have better separation between things.
  17:55 <sipa> jnewbery: sure, just providing background
  17:56 <sipa> seems very reasonable to move it elsewhere now
  ```

  The commit should be trivial to review with `--color-moved`.

ACKs for top commit:
  jnewbery:
    Code review ACK f685a13bef0418663015ea6d8f448f075510c0ec
  rajarshimaitra:
    tACK f685a13bef
  mjdietzx:
    crACK f685a13bef0418663015ea6d8f448f075510c0ec
  LarryRuane:
    Code review, test ACK f685a13bef0418663015ea6d8f448f075510c0ec

Tree-SHA512: 0e844a6ecb1be04c638b55bc4478c2949549a4fcae01c984eee078de74d176fb19d508fc09360a62ad130677bfa7daf703b67870800e55942838d7313246248c
 2023-12-03 20:13:09 -06:00
MarcoFalke
7eb5033d28 Merge bitcoin/bitcoin#22510: test: add test for RPC error 'Transaction already in block chain' 
2ebf2fe0e4727a5a57a03f4283bdf1e263855803 test: check for RPC error 'Transaction already in block chain' (-27) (Sebastian Falbesoner)

Pull request description:

  This PR adds missing test coverage for the RPC error "Transaction already in block chain" (error code `RPC_VERIFY_ALREADY_IN_CHAIN` = `RPC_TRANSACTION_ALREADY_IN_CHAIN` = -27), which is thrown in the function `BroadcastTransaction` (src/node/transaction.cpp).

ACKs for top commit:
  kristapsk:
    ACK 2ebf2fe0e4727a5a57a03f4283bdf1e263855803 (ran linter, looked at changes and ran modified test and checked code in `src/node/transaction.cpp`)
  darosior:
    ACK 2ebf2fe0e4727a5a57a03f4283bdf1e263855803

Tree-SHA512: 8bfbd3ff3da0cb3b8745f69b8ca2377f85fa99f0270750840b60e6ae43b5645c5c59b236993e8b2ad0444ec4171484e4f1ee23fa7e81b79d4222bcb623666fa5
 2023-12-03 20:13:09 -06:00
MarcoFalke
06e467154f Merge bitcoin/bitcoin#22407: rpc: Return block time in getblockchaininfo 
20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 rpc: Return block time in getblockchaininfo (João Barbosa)

Pull request description:

  Return tip time in `getblockchaininfo`, for some use cases this can save a call to `getblock`.

ACKs for top commit:
  naumenkogs:
    ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
  theStack:
    re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
  0xB10C:
    ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
  kristapsk:
    ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
  Zero-1729:
    re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364

Tree-SHA512: 29a920cfff1ef53e0af601c3f93f8f9171f3be47fc84b0fa293cb865b824976e8c1510b17b27d17daf0b8e658dd77d9dc388373395f0919fc4a23cd5019642d5
 2023-12-03 20:13:09 -06:00
MarcoFalke
f947086292 Merge #18855: tests: feature_backwards_compatibility.py test downgrade after upgrade 
489ebfd7a16443d8263c048d55622da297df7c39 tests: feature_backwards_compatibility.py test downgrade after upgrade (Andrew Chow)

Pull request description:

  After upgrading the node, try to go back to the original version to make sure that using a newer node version does not prevent the wallet file from being downgraded again.

ACKs for top commit:
  MarcoFalke:
    ACK 489ebfd7a16443d8263c048d55622da297df7c39

Tree-SHA512: 86231de6514b3657912fd9d6621212166fd2b29b591fc97120092c548babcf1d6f50b5bd103b28cecde395a26809134f01c1a198725596c3626420de3fd1f017
 2023-12-03 20:01:26 -06:00
Konstantin Akimov
5035a1cc4a Merge #18610: scripted-diff: test: replace command with msgtype (naming) 2023-12-03 20:01:26 -06:00
MarcoFalke
738d6b1c70 Merge #19304: test: Check that message sends successfully when header is split across two buffers 
80d4423f997e15780bfa3f91bf4b4bf656b8ea45 Test buffered valid message (Troy Giorshev)

Pull request description:

  This PR is a tweak of #19302.  This sends a valid message.

  Additionally, this test includes logging in the same vein as #19272.

ACKs for top commit:
  MarcoFalke:
    tested ACK 80d4423f997e15780bfa3f91bf4b4bf656b8ea45 (added an assert(false) to observe deterministic coverage) 🌦
  gzhao408:
    ACK 80d4423f99 👊

Tree-SHA512: 3b1aa5ec480a1661917354788923d64595e2886448c9697ec0606a81293e8b4a4642b2b3cc9afb2206ce6f74e5c6d687308c5ad19cb73c5b354d3071ad8496f8
 2023-12-03 20:01:26 -06:00
Konstantin Akimov
b292c41eea partial Merge #18628: test: Add various low-level p2p tests 
Adds missing changes from p2p_invalid_tx.py but one assert is still disabled

fa4c29bc1d2425f861845bae4f3816d9817e622a test: Add various low-level p2p tests (MarcoFalke)

Pull request description:

ACKs for top commit:
  jonatack:
    ACK fa4c29bc1d242

Tree-SHA512: 842821b97359d4747c763398f7013415858c18a300cd882887bc812d039b5cbb67b9aa6f68434575dbc3c52f7eb8c43d1b293a59555a7242c0ca615cf44dc0aa
 2023-12-03 20:01:26 -06:00
Konstantin Akimov
fd2e9857aa fix: follow-up partial bitcoin/bitcoin#25063 - actually load binaries with x86_64-apple-darwin platform 2023-11-24 11:23:46 -06:00
MarcoFalke
d7bdf8e424 Merge bitcoin/bitcoin#26694: test: get_previous_releases.py: M1/M2 macs can't run unsigned arm64 binaries; self-sign when needed 
dc12f2e212dfacbe238cf68eb454b9ec71169bbc test: improve error msg on previous release tarball extraction failure (kdmukai)
7121fd8fa7de50ff67157f81f9e0f267b9795dbb test: self-sign previous release binaries for arm64 macOS (kdmukai)

Pull request description:

  ## The Problem
  If you run `test/get_previous_releases.py -b` on an M1 or M2 mac, you'll get an unsigned v23.0 binary in the arm64 tarball. macOS [sets stricter requirements on ARM binaries](https://news.ycombinator.com/item?id=26996578) so the unsigned arm64 binary is apparently completely unusable without being signed/notarized(?).

  This means that any test that depends on a previous release (e.g. `wallet_backwards_compatibility.py`) will fail because the v23.0 node cannot launch:

  ```
  TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "/Users/kdmukai/dev/bitcoin-core/test/functional/test_framework/test_framework.py", line 563, in start_nodes
      node.wait_for_rpc_connection()
    File "/Users/kdmukai/dev/bitcoin-core/test/functional/test_framework/test_node.py", line 231, in wait_for_rpc_connection
      raise FailedToStartError(self._node_msg(
  test_framework.test_node.FailedToStartError: [node 2] bitcoind exited with status -9 during initialization
  ```

  This can also be confirmed by downloading bitcoin-23.0-arm64-apple-darwin.tar.gz (https://bitcoincore.org/bin/bitcoin-core-23.0/) and trying to run any of the binaries manually on an M1 or M2 mac.

  ## Solution in this PR
  (UPDATED) Per @ hebasto, we can self-sign the arm64 binaries. This PR checks each binary in the previous release's "bin/" and verifies if the arm64 binary is signed. If not, attempt to self-sign and confirm success.

  (note: an earlier version of this PR downloaded the x86_64 binary as a workaround but this approach has been discarded)

  ## Longer term solution
  If possible, produce signed arm64 binaries in a future v23.x tarball?

  Note that this same problem affects the new v24.0.1 arm64 tarball so perhaps a signed v24.x.x tarball would also be ideal?

  That being said, this PR will check all current and future arm64 binaries and self-sign as needed, so perhaps we need not worry about pre-signing the tarball binaries. And I did test a version of `get_previous_releases.py` that includes the new v24.0.1 binaries and it successfully self-signed both v23.0 and v24.0.1, as expected.

  ## Further info:
  Somewhat related to: https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1265164753

  And @ fanquake noted on IRC that you can confirm which binaries are or are not signed via:
  ```
  $ codesign -v -d bitcoin-qt
  bitcoin-qt: code object is not signed at all
  ```

ACKs for top commit:
  hebasto:
    ACK dc12f2e212dfacbe238cf68eb454b9ec71169bbc

Tree-SHA512: 644895f8e97f5ffb3c4754c1db2c48abd77fa100c2058e3c896af04806596fc2b9c807a3f3a2add5be53301ad40ca2b8171585bd254e691f6eb38714d938396b
 2023-11-24 11:23:46 -06:00
MacroFake
d5654db27e partial Merge bitcoin/bitcoin#25063: test: previous releases: add v23.0 
dba123167236a172d2d33861d58aa94a19729671 test: previous releases: add v23.0 (Sjors Provoost)

Pull request description:

  Follows the same pattern as d8b705f1caeb3b4a6790cb26e4e5584ca791d965 (v22.0) and 8a57a06a5062dd8dfdefca4e404d0ddbd2a3da1d (v0.21.0).

  Starting from v23.0 there is a separate macOS release for x86_64 and aarch64.

ACKs for top commit:
  prusnak:
    Approach ACK dba123167236a172d2d33861d58aa94a19729671

Tree-SHA512: 249aeddd5e80e163578581e5c8e9b6579f3694abc3d1fb68dddb7b42d75021ad85266688ec4a365a6631d82a65a19873aff7ba61c0ea59d21f8adbe4b772dc16
 2023-11-24 11:23:46 -06:00
Kittywhiskers Van Gogh
4ac25312cc partial bitcoin#27445: Update src/secp256k1 subtree to release v0.3.1 
excludes:
- 719a74989be3cfbc4422ec07cac199c295d28d05
- 621c17869d3754559c03e4f2bee73885659e0c68
 2023-11-21 07:59:03 -06:00
Odysseas Gabrielides
112564974d
refactor: deprecate non-deterministic IS support (#5553) 
## Issue being fixed or feature implemented
Non-deterministic IS locks aren't used anymore since v18 dip24.
We should drop that support to make code simpler.

## What was done?
Dropped non-deterministic IS code, `evo_instantsend_tests` and
`feature_llmq_is_migration.py` (don't need it anymore), adjusted func
tests.

## How Has This Been Tested?
all tests, synced Testnet

## Breaking Changes

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <545784+knst@users.noreply.github.com>
 2023-11-20 10:17:04 -06:00
fanquake
5a4406ef98 Merge bitcoin/bitcoin#26153: Reduce wasted pseudorandom bytes in ChaCha20 + various improvements 
511aa4f1c7508f15cab8d7e58007900ad6fd3d5d Add unit test for ChaCha20's new caching (Pieter Wuille)
fb243d25f754da8f01793b41e2d225b917f3e5d7 Improve test vectors for ChaCha20 (Pieter Wuille)
93aee8bbdad808b7009279b67470d496cc26b936 Inline ChaCha20 32-byte specific constants (Pieter Wuille)
62ec713961ade7b58e90c905395558a41e8a59f0 Only support 32-byte keys in ChaCha20{,Aligned} (Pieter Wuille)
f21994a02e1cc46d41995581b54222abc655be93 Use ChaCha20Aligned in MuHash3072 code (Pieter Wuille)
5d16f757639e2cc6e81db6e07bc1d5dd74abca6c Use ChaCha20 caching in FastRandomContext (Pieter Wuille)
38eaece67b1bc37b2f502348c5d7537480a34346 Add fuzz test for testing that ChaCha20 works as a stream (Pieter Wuille)
5f05b27841af0bed1b6e7de5f46ffe33e5919e4d Add xoroshiro128++ PRNG (Martin Leitner-Ankerl)
12ff72476ac0dbf8add736ad3fb5fad2eeab156c Make unrestricted ChaCha20 cipher not waste keystream bytes (Pieter Wuille)
6babf402130a8f3ef3058594750aeaa50b8f5044 Rename ChaCha20::Seek -> Seek64 to clarify multiple of 64 (Pieter Wuille)
e37bcaa0a6dbb334ab6e817efcb609ccee6edc39 Split ChaCha20 into aligned/unaligned variants (Pieter Wuille)

Pull request description:

  This is an alternative to #25354 (by my benchmarking, somewhat faster), subsumes #25712, and adds additional test vectors.

  It separates the multiple-of-64-bytes-only "core" logic (which becomes simpler) from a layer around which performs caching/slicing to support arbitrary byte amounts. Both have their uses (in particular, the MuHash3072 code can benefit from multiple-of-64-bytes assumptions), plus the separation results in more readable code. Also, since FastRandomContext effectively had its own (more naive) caching on top of ChaCha20, that can be dropped in favor of ChaCha20's new built-in caching.

  I thought about rebasing #25712 on top of this, but the changes before are fairly extensive, so redid it instead.

ACKs for top commit:
  ajtowns:
    ut reACK 511aa4f1c7508f15cab8d7e58007900ad6fd3d5d
  dhruv:
    tACK crACK 511aa4f1c7

Tree-SHA512: 3aa80971322a93e780c75a8d35bd39da3a9ea570fbae4491eaf0c45242f5f670a24a592c50ad870d5fd09b9f88ec06e274e8aa3cefd9561d623c63f7198cf2c7
 2023-11-19 10:20:12 -06:00
Konstantin Akimov
ba97f49f2f
refactor: re-order headers and forward declarations to improve compile time (#5693) 
## Issue being fixed or feature implemented
Some headers include other heavy headers, such as `logging.h`,
`tinyformat.h`, `iostream`. These headers are heavy and increase
compilation time on scale of whole project drastically because can be
used in many other headers.

## What was done?
Moved many heavy includes from headers to cpp files to optimize
compilation time.
In some places  added forward declarations if it is reasonable.

As side effect removed 2 circular dependencies:
```
"llmq/debug -> llmq/dkgsessionhandler -> llmq/debug"
"llmq/debug -> llmq/dkgsessionhandler -> llmq/dkgsession -> llmq/debug"
```


## How Has This Been Tested?
Run build 2 times before refactoring and after refactoring: `make clean
&& sleep 10s; time make -j18`

Before refactoring:
```
real    5m37,826s
user    77m12,075s
sys     6m20,547s

real    5m32,626s
user    76m51,143s
sys     6m24,511s
```

After refactoring:
```
real    5m18,509s
user    73m32,133s
sys     6m21,590s

real    5m14,466s
user    73m20,942s
sys     6m17,868s
```

~5% of improvement for compilation time. That's not huge, but that's
worth to get merged

There're several more refactorings TODO but better to do them later by
backports:
 - bitcoin/bitcoin#27636
 - bitcoin/bitcoin#26286
 - bitcoin/bitcoin#27238
 - and maybe this one: bitcoin/bitcoin#28200


## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
 2023-11-17 10:04:18 -06:00
UdjinM6
b5d82832da
fix: should not notify about mnlist changes while ConnectBlock isn't done yet (#5711) 
## Issue being fixed or feature implemented
`ConnectBlock` can fail after `ProcessSpecialTxsInBlock`, we shouldn't
be notifying too early. Same for `DisconnectBlock` but that's less of an
issue imo.

## What was done?
Move notifications to the end of `ConnectBlock`/`DisconnectBlock`. There
is no `connman` in `CChainState` and I don't want to pass it in updates
struct so I changed `NotifyMasternodeListChanged` and used `connman`
from `CDSNotificationInterface` instead.

## How Has This Been Tested?
run unit test, run testnet qt wallet

## Breaking Changes

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-16 12:36:46 -06:00
UdjinM6
c2db29439a
fix: rename SPORK_24_EHF to SPORK_24_TEST_EHF, make sure it has no effect on mainnet (#5691) 
## Issue being fixed or feature implemented
Be more explicit about the fact that spork24 is for non-mainnet only,
enforce it in code.

NOTE: I know we have EHF signalling disabled for mainnet in v20 but I
think it still makes sense to make sure spork24 condition won't slip
into mainnet in some future version accidentally.

## What was done?
pls see individual commits

## How Has This Been Tested?

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-13 10:03:46 -06:00
UdjinM6
704c594237
fix: some fixes for block payee validation and corresponding tests (#5684) 
## Issue being fixed or feature implemented
1. we _should not_ skip masternode payments checks below
nSuperblockStartBlock or when governance is disabled
2. we _should_ skip superblock payee checks while we aren't synced yet
(should help recovering from missed triggers)

## What was done?
pls see individual commits. 

## How Has This Been Tested?
run tests, sync w/ and w/out `--disablegovernance`, reindexed on testnet

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-13 10:02:52 -06:00
Odysseas Gabrielides
d5b2c260a4
test: ensure that mining is possible without CL info (#5689) 
## Issue being fixed or feature implemented
With DIP29 added to v20, miners include best CL Signature in CbTx.
The purpose of this test, is to ensure that mining is still possible
when CL information isn't available.
In such case, miners are expected to copy best CL Signature from CbTx of
previous block.

## What was done?
Two scenarios are implemented:

- Add dynamically a node, make sure `getbestchainlock()` fails, let it
mine a block.
- Disable `SPORK_19_CHAINLOCKS_ENABLED`, add dynamically a node, make
sure `getbestchainlock()` fails, let it mine a block.

In both tests, we make sure the block is accepted by everyone and that
the `bestCLSignature` in CbTx is copied from previous block.

## How Has This Been Tested?
`feature_llmq_chainlocks.py`

## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-11-10 08:32:01 -06:00
Konstantin Akimov
216a5f7563
refactor: make MNActivationHeight in Params() indeed constant (#5658) 
## Issue being fixed or feature implemented
Addressed issues and comments from [PR
comment](https://github.com/dashpay/dash/pull/5469#discussion_r1317886678)
and [PR
comment](https://github.com/dashpay/dash/pull/5469#discussion_r1338704082)

`Params()` should be const; global variable `CMNHFManager` is a better
out-come.


## What was done?
The helpers and direct calls of `UpdateMNParams` for each block to
update non-constant member in `Params()` is not needed anymore. Instead
`CMNHFManager` takes cares about status of Signals for each block,
update them dynamically and save in evo db.


## How Has This Been Tested?
Run unit/functional tests.

## Breaking Changes
Changed rpc `getblockchaininfo`. 
the field `ehf` changed meaning: it's now only a flag -1/0; but it is
introduced a new field `ehf_height` now that a height.


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
 2023-11-10 08:31:12 -06:00
Odysseas Gabrielides
c293593be2
test: v20 earlier activation for regtest (#5668) 
## Issue being fixed or feature implemented
Currently, on functional tests v20 activates at height 1440 which is
later than needed.

## What was done?
Reduced the window size of v20 from 480 to 400 which activates v20 at
1200.
Adjusted tests to this change.

Note regarding the window analysis for MN payments in
`feature_llmq_evo.py` (reduced from 256 to 48 blocks):
48 window is enough to analyse 4 MNs and 5 EvoNodes (Weighted count=24)

On my machine using develop:
`python3 feature_llmq_rotation.py 145.45s user 30.00s system 68% cpu
4:16.93 total`

With this PR:
`python3 feature_llmq_rotation.py 119.26s user 24.61s system 62% cpu
3:50.89 total`


## How Has This Been Tested?
all tests


## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-07 08:03:03 -06:00
UdjinM6
c61fe0aacd
fix: actually vote NO on triggers we don't like, some additional cleanups and tests (#5670) 
## Issue being fixed or feature implemented
MNs don't really vote NO on triggers that do not match their local
candidates because:
1. they bail out too early when they see that they are not the payee
2. the hash for objects to vote NO on was picked incorrectly. 

## What was done?
Moved voting out of `CreateGovernanceTrigger` and into its own
`VoteGovernanceTriggers`. Refactored related code to use `optional`
while at it, dropped useless/misleading `IsValid()` call. Added some
safety belts, logging, tests.

## How Has This Been Tested?
Run tests.

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-06 23:45:42 +03:00
Odysseas Gabrielides
a612b31aab
test: getblockchaininfo projected activation_height test (#5665) 
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/5640

## What was done?
Tests that `activation_height` projected by `getblockchaininfo` during
locked_in phase.
Now, this test is only possible with v20 activation since v19, dip0024
are buried and mn_rr uses MNEF.

Enabled this test only in `feature_llmq_rotation.py`.

## How Has This Been Tested?
tests

## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-11-06 13:32:05 -06:00
UdjinM6
66223aed51
fix: FundTransaction should follow the same bip69 rules CreateTransaction does (#5667) 
## Issue being fixed or feature implemented
fixes #5666

kudos to @tinshen for discovering the issue 👍 

## What was done?
add missing logic in FundTransaction

## How Has This Been Tested?
implement/run tests, test rpc manually

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-11-03 10:05:37 -05:00
UdjinM6
25ee1677ca
test: fix feature_governance.py (#5657) 
## Issue being fixed or feature implemented
```
test/functional/feature_governance.py:205:59: F821 undefined name 'p0_amount'
test/functional/feature_governance.py:205:95: F821 undefined name 'p1_amount'
test/functional/feature_governance.py:205:131: F821 undefined name 'p2_amount'
```

## What was done?
add missing `self.`

## How Has This Been Tested?
run linter and `feature_governance.py`

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-31 09:01:05 -05:00
fanquake
51ccdca24d Merge bitcoin/bitcoin#25803: refactor: Drop boost/algorithm/string/replace.hpp dependency 
fea75ad3caa29972db32d3ce7e0fe125ec77a0eb refactor: Drop `boost/algorithm/string/replace.hpp` dependency (Hennadii Stepanov)
857526e8cbb0847a865e9c2509425960d458f535 test: Add test case for `ReplaceAll()` function (Hennadii Stepanov)

Pull request description:

  A new implementation of the `ReplaceAll()` seems enough for all of our purposes.

ACKs for top commit:
  adam2k:
    ACK Tested fea75ad3caa29972db32d3ce7e0fe125ec77a0eb
  theStack:
    Code-review ACK fea75ad3caa29972db32d3ce7e0fe125ec77a0eb

Tree-SHA512: dacfffc9d2bd1fb9f034baf8c045b1e8657b766db2f0a7f8ef7e25ee6cd888f315b0124c54aba7a29ae59186b176ef9868a8b709dc995ea215c6b4ce58e174d9
 2023-10-31 08:40:25 -05:00
UdjinM6
965f5b2063
fix: adjust GetPaymentsLimit to work correctly with historical blocks, adjust sb params on regtest, tweak tests (#5641) 
## Issue being fixed or feature implemented
Noticed a couple of things while I was trying to figure out if an
[issue](https://github.com/dashpay/dash/pull/5627#discussion_r1367153099)
@knst mentioned in #5627 could actually exist:
1. `GetPaymentsLimit()` won't work correctly with historical blocks rn.
We don't use it that way internally but it could be done via rpc and it
should provide correct results.
2. superblock params on regtest are too small to test them properly
3. because of (2) and a huge v20 activation window (comparing to sb
params) `feature_governance.py` doesn't test v20 switching states.
There's also no "sb on v20 activation block" test.

~NOTE: based on #5639 atm~

## What was done?
fix it, pls see individual commits

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-30 18:12:07 +03:00
UdjinM6
fa19c5ffee
fix: adjust LLMQ_TEST_DIP0024 params, mine_cycle_quorum should use correct size (#5655) 
## Issue being fixed or feature implemented
Small dip0024 related cleanups, regtest only.

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-30 10:03:22 -05:00
UdjinM6
7d1e3d4d0d
fix: do not trim values in payment_amounts (#5647) 
## Issue being fixed or feature implemented
sb produced by sentinel:
>"DataString": ... \"payment_amounts\": \"20.00000000|20.00000000\", ...
>...
> "YesCount": 83,

sb produced by core:
>"DataString": ... \"payment_amounts\": \"20.00|20.00\", ...
> "YesCount": 13,

These 2 triggers are for the same block (900552), proposal hashes and
addresses are also the same but the difference in `payment_amounts`
format makes it look like a different trigger for core and this creates
a race.

## What was done?
Use `ValueFromAmount` instead of `FormatMoney` to avoid trimming

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-27 19:59:44 -05:00
fanquake
6e7b402fe9 partial Merge bitcoin/bitcoin#27483: Bump python minimum version to 3.8 
fac395e5eb2cd3210ba6345f777a586a9bec84e3 ci: Bump ci/lint/Dockerfile (MarcoFalke)
fa6eb6516727a8675dc6e46634d8343e282528ab test: Use python3.8 pow() (MarcoFalke)
88881cf7ac029aea660c2413ca8e2a5136fcd41b Bump python minimum version to 3.8 (MarcoFalke)

Pull request description:

  There is no pressing reason to drop support for 3.7, however there are several maintenance issues:

  * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1484988445)
  * Compiling python 3.7 from source is also unsupported on at least macos, according to https://github.com/bitcoin/bitcoin/pull/24017#issuecomment-1107820790
  * Recent versions of lief require 3.8, see https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517561645

  Fix all maintenance issues by bumping the minimum.

ACKs for top commit:
  RandyMcMillan:
    ACK fac395e
  fjahr:
    ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3
  fanquake:
    ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3

Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
 2023-10-23 10:48:39 -05:00
Konstantin Akimov
6b5b8973cc fix: missing changes from Merge #15404: [test] Remove -txindex to start nodes 2023-10-23 10:46:52 -05:00
fanquake
4cd51487c4 fix: missing changes from Merge #16917: tests: Move common function assert_approx() into util.py 
96299a9d6c0a6b9125a58a63ee3147e55d1b086b Test: Move common function assert_approx() into util.py (fridokus)

Pull request description:

  To reduce code duplication, move `assert_approx` into common framework `util.py`.

  `assert_approx()` is used in two functional tests.

ACKs for top commit:
  theStack:
    ACK 96299a9
  practicalswift:
    ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b -- DRY is good and diff looks correct
  fanquake:
    ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b - thanks for contributing 🍻

Tree-SHA512: 8e9d397222c49536c7b3d6d0756cc5af17113e5af8707ac48a500fff1811167fb2e03f3c0445b0b9e80f34935f4d57cfb935c4790f6f5463a32a67df5f736939
 2023-10-23 10:46:52 -05:00
UdjinM6
faba796c73
fix: actually show json for assetlock/unlock txes (#5633) 
## Issue being fixed or feature implemented
The bug was introduced in the original PR #5026 and refactored later
(which is good actually cause we shouldn't mix refactoring and
bug-fixing :) )

## What was done?
fix conditions, add tests

## How Has This Been Tested?
`feature_asset_locks.py`

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-23 10:36:50 -05:00
PastaPastaPasta
c51cec606d
refactor: add gsl::not_null to get compile time / run time pointer guarantees (#5595) 
## Issue being fixed or feature implemented
Current implementation relies either on asserts or sometimes checks then
returning a special value; In the case of asserts (or no assert where we
use the value without checks) it'd be better to make it explicit to
function caller that the ptr must be not_null; otherwise gsl::not_null
will call terminate.

See
https://github.com/microsoft/GSL/blob/main/docs/headers.md#user-content-H-pointers-not_null
and
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-nullptr

I'm interested in a conceptual review; specifically on if this is
beneficial over just converting these ptrs to be a reference?

## What was done?
 *Partial* implementation on using gsl::not_null in dash code


## How Has This Been Tested?
Building

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Signed-off-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-22 09:14:30 -05:00
Konstantin Akimov
63ed462c54
feat: auto generation EHF and spork+EHF activation for MN_RR (#5597) 
Implementation EHF mechanism, part 4. Previous changes are: 
 - https://github.com/dashpay/dash/pull/4577
 - https://github.com/dashpay/dash/pull/5505
 - https://github.com/dashpay/dash/pull/5469

## Issue being fixed or feature implemented
Currently MN_RR is activated automatically by soft-fork activation after
v20 is activated.
It is not flexible enough, because platform may not be released by that
time yet or in opposite it can be too long to wait.
Also, any signal of EHF requires manual actions from MN owners to sign
EHF signal - it is automated here.

## What was done?
New spork `SPORK_24_MN_RR_READY`; new EHF manager that sign EHF signals
semi-automatically without manual actions; and send transaction with EHF
signal when signal is signed to network.
Updated rpc `getblockchaininfo` to return information about of EHF
activated forks.
Fixed function `IsTxSafeForMining` in chainlock's handler to skip
transactions without inputs (empty `vin`).

## How Has This Been Tested?
Run unit/functional tests. Some tests have been updated due to new way
of MN_RR activation: `feature_asset_locks.py`, `feature_mnehf.py`,
`feature_llmq_evo.py` and unit test `block_reward_reallocation_tests`.


## Breaking Changes
New way of MN_RR activation.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
 2023-10-17 22:31:40 -05:00
Odysseas Gabrielides
cecf63e0b7
feat!: exclude fees when calculating platformReward (#5612) 
## Issue being fixed or feature implemented
Calculation of `platformReward` should ignore fees and rely only on
Block subsidy.

cc @QuantumExplorer 

## What was done?
From now on, the following formula is applied:
```
blockReward = blockSubsidy + feeReward
masternodeReward = masternodeShare(blockSubsidy)
platformReward = platformShare(masternodeReward)
masternodeReward += masternodeShare(feeReward)
```


## How Has This Been Tested?


## Breaking Changes
`plaftormReward` differs in networks where `mn_rr` is already active

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-17 22:07:37 -05:00
Odysseas Gabrielides
848ed765e0
feat!: constant subsidy base for blocks in v20 (#5611) 
## Issue being fixed or feature implemented
Currently, the `nSubsidyBase` calculation relies on difficulty. This
leads to variable Block Subsidity.
When Platform will be live, it would constantly require blocks
difficulty in order to calculate the `platformReward` (which relies on
Block Subsidy)

cc @QuantumExplorer 

## What was done?
Starting from v20 activation, `nSubsidyBase` will no longer rely on
difficulty and will be constant to 5.

## How Has This Been Tested?


## Breaking Changes
Block rewards will differ.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-17 15:50:23 -05:00
Odysseas Gabrielides
5ca6382bfa
test: correct calculation of coinbasevalue in feature_asset_locks.py (#5603) 
## Issue being fixed or feature implemented
Fixed a problem forgotten in #5588 in feature_asset_locks.py.

## What was done?
Avoid floating operations when calculating `coinbasevalue`

## How Has This Been Tested?

## Breaking Changes

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-11 08:13:29 -05:00
UdjinM6
30f3f50928
fix: Let CDeterministicMN::ToJson() return correct collateralAddress for spent collaterals (#5607) 
## Issue being fixed or feature implemented
Historical masternode data returned via rpcs like `protx listdiff` can
be broken because some collaterals might be spent already and
`GetUTXOCoin` wasn't able to get any info.

## What was done?
Use `GetTransaction` as a fallback.

## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-09 11:14:51 -05:00
UdjinM6
2004a855d9
fix!: avoid float calculations in PlatformShare (#5604) 
## Issue being fixed or feature implemented
avoid potential discrepancies in block reward calculations

## What was done?
use integers (int64_t) only when dealing with block rewards, no
float/double

## How Has This Been Tested?
run tests

## Breaking Changes
might fork off on devnets that use previous version

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-09 09:15:23 -05:00
UdjinM6
4b046bb608 use deployment nStartTime as a signal expiration mark, adjust tests 
if a signal is mined prior to nStartTime then it means it was mined for one of the previous deployments with the same bit and we can ignore it
 2023-10-06 11:02:15 -05:00
Konstantin Akimov
d83dbd287a fix: fix previous commit with fixes 2023-10-06 11:02:15 -05:00
Konstantin Akimov
5e31bd5545 refactor: multiple fixes, cleanups, improvements and refactorings 2023-10-06 11:02:15 -05:00
Konstantin Akimov
92be5e0be7 fix: now EHF transactions expires after nExpiryEHF blocks 2023-10-06 11:02:15 -05:00
Konstantin Akimov
3973f2b925 feat: update functional tests for Mn EHF - to use same bit more than once 2023-10-06 11:02:15 -05:00
Konstantin Akimov
ef14b53b3d feat: add functional test for unknown and invalid version bits of EHF release 2023-10-06 11:02:15 -05:00
Konstantin Akimov
2c4597db9f feat: improve functional tests for MnEHF to check block reconsideration 2023-10-06 11:02:15 -05:00
Konstantin Akimov
b85a497cca feat: new functional test for feature MnEHF 2023-10-06 11:02:15 -05:00
Konstantin Akimov
33ab3187b2 feat: add CMNHFManager and logic to make hard-forks accordingly received signals 2023-10-06 11:02:15 -05:00
Konstantin Akimov
612faa8868 feat: imlemented new hard-fork mechanism that uses MN Activation Height 
Altough, it's still disabled because no calls of related methods after processing MnEHF tx
 2023-10-06 11:02:15 -05:00
UdjinM6
c814dcaaea
fix: Move CreditPoolDiff checks out of ProcessSpecialTxsInBlock, use correct block reward (#5594) 
## Issue being fixed or feature implemented
The block reward calculation logic in `SetTarget` doesn't work on
superblocks.

## What was done?
Move `CreditPoolDiff` checks out of `ProcessSpecialTxsInBlock` to use
correct block reward.

## How Has This Been Tested?
run tests

## Breaking Changes
n/a, sb blocks should now be processed correctly, non-sb blocks
shouldn't be affected

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-10-04 12:47:21 -05:00
Odysseas Gabrielides
e72eb40024
feat!: Block Reward Reallocation (Doubling Treasury) (#5588) 
## Issue being fixed or feature implemented
Implementation of accepted proposal:
https://www.dashcentral.org/p/TREASURY-REALLOCATION-60-20-20

## What was done?
Once Masternode Reward Location Reallocation activates:
- Treasury is bumped to 20% of block subsidy.
- Block reward shares are immediately set to 75% for MN and 25% miners.
(Previous reallocation periods are dropped)
MN reward share should be 75% of block reward in order to represent 60%
of the block subsidy. (according to the proposal)
- `governancebudget` is returned from `getgovernanceinfo` RPC.

## How Has This Been Tested?
`block_reward_reallocation_tests`

## Breaking Changes


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-03 09:32:53 -05:00
PastaPastaPasta
7ca61abed0
refactor: remove some now unneeded cppcheck suppressions (#5589) 
## Issue being fixed or feature implemented
Unneeded suppressions were present

## What was done?
Removed them

## How Has This Been Tested?
Running linter

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-09-29 07:53:22 -05:00
Kittywhiskers Van Gogh
c3b4b6746a merge bitcoin#20966: save the banlist in a JSON format on disk 2023-09-24 09:50:50 -05:00
Kittywhiskers Van Gogh
5d7367e366 merge bitcoin#22050: remove tor v2 support 2023-09-24 09:50:50 -05:00
Kittywhiskers Van Gogh
4b29a74904 merge bitcoin#20852: allow CSubNet of non-IP networks 2023-09-24 09:50:50 -05:00
Konstantin Akimov
633cc3260f
feat: new rpc `gettxchainlocks' to get transaction statuses by batch (#5578) 
## Issue being fixed or feature implemented
Requested by @QuantumExplorer for platform needs

## What was done?
New rpc `gettransactionsarelocked` that returns list of txes.
it does less heavy calculations and transfer less data by gRPC.


## How Has This Been Tested?
```
$ src/dash-cli gettransactionsarelocked  '["e469de7994b9c1da8efd262fee8843efd7bdcab80c700dc1059c98b28f7c5c1b", "0d9fdf00c9568ff9103742b64e6b8287794633072f8824fa2c475f59e71dbace","0d3f48eebead54d640a7fc5692ddfcba619d8b49347d9a7c04586057c02dec9f"]'

[
  {
    "height": 907801,
    "chainlock": true
  },
  {
    "height": 101,
    "chainlock": true
  },
  {
    "height": -1,
    "chainlock": false
  }
]
```
Limiter tested by this call:
```
src/dash-cli gettransactionsarelocked  '["", "","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","",""]'  | wc
```

## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-09-20 09:07:24 -05:00
merge-script
90285f5327 Merge bitcoin/bitcoin#22790: test: add aarch64-apple-darwin platform entry to get_previous_releases 
f6e4db27ceb67157dc13d13f34351cb87fec2be5 test: add aarch64-apple-darwin platform entry to get_previous_releases (Zero-1729)

Pull request description:

  Over the course of reviewing a PR, I had to edit `test/get_previous_releases.py` (after I ran `git clean -xdff`) to run the backwards compatibility tests (e.g. `wallet_upgradewallet`, `feature_backwards_compatibility`, etc.), as currently on master, running the script as indicated in [`test/README.md`](https://github.com/bitcoin/bitcoin/blob/master/test/README.md), for example, on an M1 machine results in the following error, as the `aarch64-apple-darwin*` platform entry is presently not recognised:

  > Output from an M1 machine running macOS v11.5.2

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2
  Releases directory: releases
  Not sure which binary to download for aarch64-apple-darwin20.6.0
  ```

  As a quick fix, this PR adds the missing `aarch64-apple-darwin*` platform entry. Running the script now results in fetching the old binaries, as expected:

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2

  Releases directory: releases
  Fetching: https://bitcoincore.org/bin/bitcoin-core-0.20.1/bitcoin-0.20.1-osx64.tar.gz
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0 20.9M    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
  100 20.9M  100 20.9M    0     0   136k      0  0:02:37  0:02:37 --:--:-- 95607
  Checksum matched

  …

  Checksum matched
  ```

  After this patch, the backwards compatibility tests also run successfully, as expected.

  **Note**: I am open to other possible solutions.

  ---

  Steps to reproduce:

  > Ensure you take out the binaries in `releases` if they already exist.

  Try running `test/get_previous_releases.py -b v0.15.2` or similar to fetch the old release binaries.

Top commit has no ACKs.

Tree-SHA512: a238d909b70a61be622234bc49b05d2e91a8acfc5ea348d29f2c8a927fb793cb97365e558571e3f46d6a5650c4f3c6e28fa126c6e56b38e1eb98f7c3e3594d0f
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
f878b281ba fix: fillow-up Merge #18774: test: added test for upgradewallet RPC 
- partial dashification
 - disabling this test so far as it does not work anyway
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
fab1031a70 fix: missing changes from Merge #18873: Fix intermittent sync_blocks failures 2023-09-19 08:54:12 -05:00
Sjors Provoost
fe1e8c2d13 fix: missing changes from Merge #18067: wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition 
[test] check for addmultisigaddress regression
 2023-09-19 08:54:12 -05:00