Commit Graph

1341 Commits

Author SHA1 Message Date
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
fanquake
7fbcba4503
Merge #19765: doc: Fix getmempoolancestors RPC result doc
333329dbda423b00098ec9f8702d75d24468c56e doc: Fix getmempoolancestor RPC result doc (MarcoFalke)

Pull request description:

ACKs for top commit:
  laanwj:
    ACK 333329dbda423b00098ec9f8702d75d24468c56e

Tree-SHA512: 30a7568ec15d1af0c484b4d479e14ec3609a01b76f17f8285688b0c5e5b0480926bbf6f651da91193b66fd752e83e9707e4b08c52b69f8c670c430da84713359
2024-01-22 19:47:11 -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
Wladimir J. van der Laan
a728478afb
Merge #19660: refactor: Make HexStr take a span
0a8aa626dd69a357e1b798b07b64cf4177a464a3 refactor: Make HexStr take a span (Wladimir J. van der Laan)

Pull request description:

  Make `HexSt`r take a span of bytes, instead of an awkward pair of templated iterators. This simplifies most of the uses.

ACKs for top commit:
  elichai:
    Code review ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
  hebasto:
    re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
  jonatack:
    re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3

Tree-SHA512: 6e178ece5cbac62119c857a10299b1e85422938084c3f03063e17119a5129e0c28016e05a6fabaa4c271a7e0a37c7cd89fa47c435ee19b38a5acfe80d00de992
2024-01-19 10:34:32 -06:00
MarcoFalke
bd1e2b0003
Merge #19585: rpc: RPCResult Type of MempoolEntryDescription should be OBJ.
ae4958be95a1158de9992a8e43ce032d87c74f13 rpc: RPCResult Type of MempoolEntryDescription should be OBJ. If multiple entries are possible, wrapping Type should be OBJ_DYN. fixes #19579 (Chris L)

Pull request description:

  If multiple entries are possible, wrapping Type should be OBJ_DYN.

  fixes #19579

Top commit has no ACKs.

Tree-SHA512: 59cf9f6e9729a69a867e924d8306e0cd6b70a3d702fc5a4111345874bb1224ee51ac3f70cea61b25cfe6bde7f65cb02528d52acc20dda4eda692eddf34f217e8
2024-01-19 10:34:31 -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
d7ea4757df
Merge #19282: RPC: Rephrase generatetoaddress help, and use PACKAGE_NAME
0f8f51544558d204a4e9d0607b0f375150529637 RPC: Rephrase generatetoaddress help, and use PACKAGE_NAME (Luke Dashjr)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 357c6e0bd1b144213ca6cf0bfd649c7a482c2d6d5e98a254d20c8365d228dc71ae1b78aca4918fdbf065f8894ef82f8a475902d605204275bb99fe77d4b42fae
2024-01-16 15:05:07 -06:00
MarcoFalke
673111c448
Merge #19233: Make SetMiscWarning() accept bilingual_str argument
d49612f98add29066817b7c808b76c2d728948e5 Make SetMiscWarning() accept bilingual_str argument (Hennadii Stepanov)
d1ae7c0355662481a7d181a0a458284936d53eb1 Make GetWarnings() return bilingual_str (Hennadii Stepanov)
38e33aa481cefbe12c50f344bae190c0d95fb489 refactor: Make GetWarnings() bilingual_str aware internally (Hennadii Stepanov)

Pull request description:

  This is one more step for consistent usage of `bilingual_str`.

  No new translation messages are defined.

ACKs for top commit:
  laanwj:
    Code review ACK d49612f98add29066817b7c808b76c2d728948e5
  MarcoFalke:
    ACK d49612f98add29066817b7c808b76c2d728948e5 🌂

Tree-SHA512: 7413cb94a85291209c182845f6873350bb9e9ce940647d416c462a136603832fec8a63d792341bf634f07629767c78bc206d3a318cf10c7e87241c114c2496e9
2024-01-16 15:05:05 -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
MarcoFalke
03d743466b
Merge #20941: rpc: document RPC_TRANSACTION_ALREADY_IN_CHAIN exception
74d23bf7fbb6169ec658c36af57cd1f937823d9c rpc: document RPC_TRANSACTION_ALREADY_IN_CHAIN exception (Jarol Rodriguez)

Pull request description:

  It is not documented in the `RPCHelpMan` of `sendrawtransaction` that if you attempt to send a transaction which already exists in a block, an `RPC_TRANSACTION_ALREADY_IN_CHAIN` exception will be raised. It is best to make developers aware of this so that it can be properly caught and avoid any headaches.

  Closes #5638

ACKs for top commit:
  jonatack:
    ACK 74d23bf7fbb6169ec658c36af57cd1f937823d9c

Tree-SHA512: d1d5fc242574377c8a76b4ef7b12239996424d8bee186533b5a8fe337bbeb3186e51dbdd28c5eafb982601e44e17b68a7f52db5dd7bc647429f6f95e2de289f6
2024-01-16 09:29:49 -06:00
fanquake
d6fdca6207
Merge #20787: Use C++17 std::array deduction for OUTPUT_TYPES, ALL_FEE_ESTIMATE_HORIZONS
aaaa9878405f3f38f4f61c00feca110d7f9ca481 refactor: Use C++17 std::array deduction for ALL_FEE_ESTIMATE_HORIZONS (MarcoFalke)
fa39cdd072c91eac70cda04b8b26681611f94cb7 refactor: Use C++17 std::array deduction for OUTPUT_TYPES (MarcoFalke)

Pull request description:

  With the new C++17 array deduction rules, an array encompassing all values in an enum can be specified in the same header file that specifies the enum. This is useful to avoid having to repeatedly enumerate all enum values in the code. E.g. the RPC code, but also the fuzz code.

ACKs for top commit:
  theStack:
    cr ACK aaaa9878405f3f38f4f61c00feca110d7f9ca481 ⚙️
  fanquake:
    ACK aaaa9878405f3f38f4f61c00feca110d7f9ca481

Tree-SHA512: b71bd98f3ca07ddfec385735538ce89a4952e418b52dc990fb160187ccef1fc7ebc139d42988b6f7b48df24823af61f803b83d47fb7a3b82475f0c0b109bffb7
2024-01-16 07:57:34 -06:00
PastaPastaPasta
ca77d06a25
refactor: make GetTxPayload return an Optional T instead of taking in a T& return (#5733)
## Issue being fixed or feature implemented
We should avoid return by reference; especially return by reference with
a bool flag indicating validity.

## What was done?
Instead we use a std::optional

## How Has This Been Tested?
Unit tests pass

## Breaking Changes
Should be none

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [ ] 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>
2024-01-11 21:43:01 -06:00
Jonas Schnelli
5758a3368d
Merge #18152: qt: Use SynchronizationState enum for signals to GUI
a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc refactor: Remove Node:: queries from GUI (Hennadii Stepanov)
06d519f0b43ed16252428e935d3aeb5a38f582e0 qt: Add SynchronizationState enum to signal parameter (Hennadii Stepanov)
3c709aa69d5bb5a1564c339a0e6a16bac8f02c98 refactor: Remove Node::getReindex() call from GUI (Hennadii Stepanov)
1dab574edf57ccd6cdf5ec706ac328c62142d7a2 refactor: Pass SynchronizationState enum to GUI (Hennadii Stepanov)
2bec309ad6d0f2543948d64ed26f7d9a903f67e5 refactor: Remove unused bool parameter in RPCNotifyBlockChange() (Hennadii Stepanov)
1df77014d8bb733d7d89e36b28671cb47f436292 refactor: Remove unused bool parameter in BlockNotifyGenesisWait() (Hennadii Stepanov)

Pull request description:

  This PR is a followup of #18121 and:
  - addresses confusion about GUI notification throttling conditions (**luke-jr**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#discussion_r378552386), **ryanofsky**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#discussion_r378975960))
  - removes `isInitialBlockDownload()` call from the GUI back to the node (on macOS). See:  **ryanofsky**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#pullrequestreview-357730284)

ACKs for top commit:
  jonasschnelli:
    Core Review ACK a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc (modulo [question](https://github.com/bitcoin/bitcoin/pull/18152#pullrequestreview-414140601)).
  ryanofsky:
    Code review ACK a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc. Only changes since last review were rebase and tweaking SynchronizationState enum declaration as suggested (thanks!)

Tree-SHA512: b6a712a710666e763aeee0d5440de1391a4c6c8f7fa661888773e1ba59e9e0f83654ee384d4edc704031be7eb25616e5eca2a6e26058d3efb7f64c47f9ed7316
2024-01-10 19:15:47 -06:00
UdjinM6
dc5152bd74
refactor: drop global coinJoinWalletManager 2024-01-10 12:06:04 -06:00
UdjinM6
60240b1fde
refactor: pass wallet name instead of wallet itself when possible 2024-01-10 12:06:03 -06:00
UdjinM6
b208e911c6
refactor: rename CJClientManager 2024-01-10 12:06:01 -06:00
Konstantin Akimov
28b12e2647
fix: follow-up bitcoin#15638 to move AnalyzePSBT related code 2024-01-10 12:05:57 -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
fanquake
2865a2d142
Merge #19316: [net] Cleanup logic around connection types
01e283068b9e6214f2d77a2f772a4244ebfe2274 [net] Remove unnecessary default args on CNode constructor (Amiti Uttarwar)
bc5d65b3ca41eebb1738fdda4451d1466e77772e [refactor] Remove IsOutboundDisconnectionCandidate (Amiti Uttarwar)
2f2e13b6c2c8741ca9d825eaaef736ede484bc85 [net/refactor] Simplify multiple-connection checks (Amiti Uttarwar)
7f7b83deb2427599c129f4ff581d4d045461e459 [net/refactor] Rework ThreadOpenConnections logic (Amiti Uttarwar)
35839e963bf61d2da0d12f5b8cea74ac0e0fbd7b [net] Fix bug where AddrFetch connections would be counted as outbound full relay (Amiti Uttarwar)
4972c21b671ff73f13a1b5053338b6abbdb471b5 [net/refactor] Clarify logic for selecting connections in ThreadOpenConnections (Amiti Uttarwar)
60156f5fc40d56bb532278f16ce632c5a8b8035e [net/refactor] Remove fInbound flag from CNode (Amiti Uttarwar)
7b322df6296609570e368e5f326979279041c11f [net/refactor] Remove m_addr_fetch member var from CNode (Amiti Uttarwar)
14923422b08ac4b21b35c426bf0e1b9e7c97983b [net/refactor] Remove fFeeler flag from CNode (Amiti Uttarwar)
49efac5cae7333c6700d9b737d09fae0f3f4d7fa [net/refactor] Remove m_manual_connection flag from CNode (Amiti Uttarwar)
d3698b5ee309cf0f0cdfb286d6b30a256d7deae5 [net/refactor] Add connection type as a member var to CNode (Amiti Uttarwar)
46578c03e92a55925308363ccdad04dcfc820d96 [doc] Describe different connection types (Amiti Uttarwar)
442abae2bac7bff85886143df01e14215532b974 [net/refactor] Add AddrFetch connections to ConnectionType enum (Amiti Uttarwar)
af59feb05235ecb85ec9d75b09c66e71268c9889 [net/refactor] Extract m_addr_known logic from initializer list (Amiti Uttarwar)
e1bc29812ddf1d946bc5acca406a7ed2dca064a6 [net/refactor] Add block relay only connections to ConnectionType enum (Amiti Uttarwar)
0e52a659a2de915fc3dce37fc8fac39be1c8b6fa [net/refactor] Add feeler connections to ConnectionType enum (Amiti Uttarwar)
1521c47438537e192230486dffcec0228a53878d [net/refactor] Add manual connections to ConnectionType enum (Amiti Uttarwar)
26304b4100201754fb32440bec3e3b78cd3f0e6d [net/refactor] Introduce an enum to distinguish type of connection (Amiti Uttarwar)
3f1b7140e95d0f8f958cb35f31c3d964c57e484d scripted-diff: Rename OneShot to AddrFetch (Amiti Uttarwar)

Pull request description:

  **This is part 1 of #19315, which enables the ability to test `outbound` and `block-relay-only` connections from the functional tests.** Please see that PR for more information of overall functionality.

  **This PR simplifies how we manage different connection types.** It introduces an enum with the various types of connections so we can explicitly define the connection type. The existing system relies on a series of independent flags, then has asserts scattered around to ensure that conflicting flags are not enabled at the same time. I find this approach to be both brittle and confusing. While making these changes, I found a small bug due to the silent assumptions.

  This PR also proposes a rename from `OneShot` to `AddrFetch`. I find the name `OneShot` to be very confusing, especially when we also have `onetry` manual connections. Everyone I've talked to offline has agreed that the name is confusing, so I propose a potential alternative. I think this is a good opportunity for a rename since I'm creating an enum to explicitly define the connection types.
  (some context for the unfamiliar: `oneshot` or `addrfetch` connections are short-lived connections created on startup. They connect to the seed peers, send a `getaddr` to solicit addresses, then close the connection.)

  Overview of this PR:
  * rename `oneshot` to `addrfetch`
  * introduce `ConnectionType` enum
  * one by one, add different connection types to the enum
  * expose the `conn_type` on CNode, and use this to reduce reliance on flags (& asserts)
  * fix the bug in counting different type of connections
  * some additional cleanup to simplify logic and make expectations explicit/inclusive rather than implicit/exclusive.

ACKs for top commit:
  jnewbery:
    utACK 01e283068b9e6214f2d77a2f772a4244ebfe2274
  laanwj:
    Code review ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274, the commits are pretty straightforward to follow, and I think this is a move in the right direction overall
  vasild:
    ACK 01e283068
  sdaftuar:
    ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274.
  fanquake:
    ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274 - I don't have as much experience with the networking code but these changes look fairly straight forward, the new code seems more robust/understandable and the additional documentation is great. I'm glad that a followup branch is already underway. There might be some more review comments here later today, so keep an eye on the discussion, however I'm going to merge this now.
  jb55:
    wow this code was messy before... ACK 01e283068b9e6214f2d77a2f772a4244ebfe2274

Tree-SHA512: 7bb644a6ed5849913d777ebc2ff89133ca0fbef680355a9a344e07496a979e6f9ff21a958e8eea93dcd7d5c343682b0c7174b1a3de380a4247eaae73da436e15
2024-01-09 08:15:35 -06:00
MarcoFalke
061f2b57c8
Merge #17946: Fix GBT: Restore "!segwit" and "csv" to "rules" key
412d5fe8791c417bf46fc55a5bb8d59be98a33db QA: feature_segwit: Check that template "rules" includes "!segwit" as appropriate (Luke Dashjr)
2abe8cc3b760219cfa434e4c96e9f8d3611d0037 Bugfix: Include "csv","!segwit" in "rules" (Luke Dashjr)

Pull request description:

  #16060 removed CSV & segwit from versionbits, breaking the "rules" key returned by GBT.

  Without this, miners don't know they're mining segwit blocks, and should fall back to pre-segwit block creation.

ACKs for top commit:
  sipa:
    ACK 412d5fe8791c417bf46fc55a5bb8d59be98a33db
  jnewbery:
    Tested ACK 412d5fe8791c417bf46fc55a5bb8d59be98a33db.

Tree-SHA512: 825d72e257dc0dd4941f2fe498d8d4f4f2a21b9505cd21a8f9eb7fb5d6d7dd9219347928cf90bb57a777920ce24295859763e64fa8a22ebb58fc2380f80f5615
2024-01-09 08:13:04 -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
b2ad5302ce
refactor: simplify comparator in rpc/governance 2023-12-21 23:04:44 -06:00
Konstantin Akimov
64a153d634
refactor: untie governance/object and wallet implementation 2023-12-21 23:04:40 -06:00
Konstantin Akimov
c3c9ccf261 refactor: drop global variable fDIP0001ActiveAtTip - partial implementation
Impossible to drop it completelly right now because:
 - net doesn't know any details about chain - can't check status of fork
 - the functional test feature_maxuploadtarget.py assume block size 1Mb
 - DIP0001 can't be activated from regtest early block2 because big txes are
 not allowed after DIP0001

refactor: drop global variable fDIP0001ActiveAtTip - attempt 2
2023-12-21 23:02:31 -06:00
Konstantin Akimov
1ed7f0cca4 refactor: use DeploymentActiveAfter instead llmq::utils in rpc/ 2023-12-21 23:02:31 -06:00
PastaPastaPasta
acd2acd592
refactor: pass large structure by const reference on every RPC call (#5780) 2023-12-21 12:01:39 -06:00
UdjinM6
6fe36cc1cb
fix: Improve quorum caching (again) (#5761)
## Issue being fixed or feature implemented
1. `scanQuorumsCache` is a special one and we use it incorrectly.
2. Platform doesn't really use anything that calls `ScanQuorums()`
directly, they specify the exact quorum hash in RPCs so it's
`GetQuorum()` that is used instead. The only place `ScanQuorums()` is
used for Platform related stuff is `StartCleanupOldQuorumDataThread()`
because we want to preserve quorum data used by `GetQuorum()`. But this
can be optimised with its own (much more compact) cache.
3. RPCs that use `ScanQuorums()` should in most cases be ok with smaller
cache, for other use cases there is a note in help text now.

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests, run a node (~in progress~ looks stable)

## 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
2023-12-20 09:54:00 -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
MarcoFalke
09973f2bee Merge #20932: refactor: Replace fs::absolute calls with AbsPathJoin calls
da9caa1cedd69702aea44cb44b2fd0a2d6d56916 Replace fs::absolute calls with AbsPathJoin calls (Kiminuo)
66576c4fd532ac18b8b355ea93d25581a2c15654 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo)

Pull request description:

  This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.

  This PR doesn't change behavior aside from adding an assert and fixing a test bug.

ACKs for top commit:
  jonatack:
    Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 only doxygen improvements since my last review per `git diff d867d7a da9caa1`
  MarcoFalke:
    review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 📯
  ryanofsky:
    Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916. Just comment and test tweaks since previous review.

Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
2023-12-08 21:16:00 +03:00
MarcoFalke
d6d3e0905c Merge bitcoin/bitcoin#19362: rpc/blockchain: Reset scantxoutset progress before inferring descriptors
8c4129b4540f4f739413ed9a6fbfc78afc252f42 rpc: reset scantxoutset progress on finish (Pavol Rusnak)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/19361 by moving resetting the `g_scan_progress` variable **before** inferring the descriptors

ACKs for top commit:
  achow101:
    Code review ACK 8c4129b4540f4f739413ed9a6fbfc78afc252f42

Tree-SHA512: 2b81e5c930b9c201a1b9cc742c37e0617582326acd9adbd6b14991d33c05d51b1d5ff6a26f3accab7d274dc8c6a48bc8cbccf0811b60c1f76dd805f9b31482c0
2023-12-08 21:16:00 +03:00
Wladimir J. van der Laan
da20789983 Merge bitcoin-core/gui#162: Add network to peers window and peer details
e262a19b0ba07d8d2334551f49ca1577ab2999fc gui: display network in peer details (Jon Atack)
913695307360d3582f74ffe225cb122b2e48023d gui: rename peer tab column headers, initialize in .h (Hennadii Stepanov)
05c08c696a1bc45ab28abe6f82bebbdbd6239c23 gui: add network column in peers tab/window (Jon Atack)
e0e55060bf697fe8b9ed43ff62383a0451e9c0ce gui: fix broken doxygen formatting in src/qt/guiutil.h (Jon Atack)
0d5613f9ded846aa20a95c757672e9560c664c4c gui: create GUIUtil::NetworkToQString() utility function (Jon Atack)
af9103cc792e17f35249f1d4cb30f0d6958ceb75 net, rpc: change CNodeStats::m_network from string to Network (Jon Atack)

Pull request description:

  and rename peers window column headers from NodeId and Node/Service to Peer Id and Address.

  ![Screenshot from 2020-12-27 14-45-31](https://user-images.githubusercontent.com/2415484/103172228-efec8600-4849-11eb-8cee-04a3d2ab1273.png)

ACKs for top commit:
  laanwj:
    ACK e262a19b0ba07d8d2334551f49ca1577ab2999fc

Tree-SHA512: 709c2a805c109c2dd033aca7b6b6dc94ebe2ce7a0168c71249e1e661c9c57d1f1c781a5b9ccf3b776bedeb83ae2fb5c505637337c45b1eb9a418cb1693a89761
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
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
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
fanquake
da7ca00f03 Merge #18814: rpc: Relock wallet only if most recent callback
9f59dde9740d065118bdddde75ef9f4e4603a7b1 rpc: Relock wallet only if most recent callback (João Barbosa)
a2e6db5c4f1bb52a8814102b628e51652493d06a rpc: Add mutex to guard deadlineTimers (João Barbosa)

Pull request description:

  This PR fixes an early relocking race condition from #18811 where old relock callback runs after new wallet unlock code and nRelockTime update but before rpcRunLater call, causing early relock and incorrect nRelockTime time

  Issue introduced in #18487.
  Fixes #18811.

ACKs for top commit:
  MarcoFalke:
    ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1
  ryanofsky:
    Code review ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1. No changes since last review except squashing commits.
  jonatack:
    ACK 9f59dde9740d065118bdddde75ef

Tree-SHA512: 2f7fc03e5ab6037337f2d82dfad432495cc337c77d07c968ee2355105db6292f24543c03456f5402e0e759577a4327758f9372f7ea29de6d56dc3695fda9b379
2023-12-05 12:47:47 -06:00
MarcoFalke
52c8ef2feb
Merge bitcoin/bitcoin#22263: refactor: wrap CCoinsViewCursor in unique_ptr
7ad414f4bfa74595ee5726e66f3527045c02a977 doc: add comment about CCoinsViewDBCursor constructor (James O'Beirne)
0f8a5a4dd530549d37c43da52c923ac3b2af1a03 move-only(ish): don't expose CCoinsViewDBCursor (James O'Beirne)
615c1adfb07b9b466173166dc2e53ace540e4b32 refactor: wrap CCoinsViewCursor in unique_ptr (James O'Beirne)

Pull request description:

  I tripped over this one for a few hours at the beginning of the week, so I've sort of got a personal vendetta against `CCoinsView::Cursor()` returning a raw pointer.

  Specifically in the case of CCoinsViewDB, if a raw cursor is allocated and not freed, a cryptic leveldb assertion failure occurs on CCoinsViewDB destruction (`Assertion 'dummy_versions_.next_ == &dummy_versions_' failed.`).

  This is a pretty simple change.

  Related to: https://github.com/bitcoin/bitcoin/issues/21766
  See also: https://github.com/google/leveldb/issues/142#issuecomment-414418135

ACKs for top commit:
  MarcoFalke:
    review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 🔎
  jonatack:
    re-ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 modulo suggestion
  ryanofsky:
    Code review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977. Two new commits look good and thanks for clarifying constructor comment

Tree-SHA512: 6471d03e2de674d84b1ea0d31e25f433d52aa1aa4996f7b4aab1bd02b6bc340b15e64cc8ea07bbefefa3b5da35384ca5400cc230434e787c30931b8574c672f9
2023-12-03 20:45:01 -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
ad3086c629 Merge bitcoin/bitcoin#22383: rpc: Prefer to use txindex if available for GetTransaction
78f4c8b98eada337346ffb206339c3ebae4ff43b prefer to use txindex if available for GetTransaction (Jameson Lopp)

Pull request description:

  Fixes #22382

  Motivation: prevent excessive disk reads if txindex is enabled.

  Worth noting that this could be argued to be less of a bug and more of an issue of undefined behavior. If a user calls GetTransaction with the wrong block hash, what should happen?

ACKs for top commit:
  jonatack:
    ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b
  theStack:
    Code review ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b
  LarryRuane:
    tACK 78f4c8b98eada337346ffb206339c3ebae4ff43b
  luke-jr:
    utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b
  jnewbery:
    utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b
  rajarshimaitra:
    Code review ACK 78f4c8b98e
  lsilva01:
    Code Review ACK and Tested ACK 78f4c8b98e on Ubuntu 20.04

Tree-SHA512: af7db5b98cb2ae4897b28476b2fa243bf7e6f850750d9347062fe8013c5720986d1a3c808f80098e5289bd84b085de03c81a44e584dc28982f721c223651bfe0
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
d70ba2c0f7 Merge bitcoin/bitcoin#19438: Introduce deploymentstatus
e48826ad87b4f92261f7433e84f48dac9bd9e5c3 tests: remove ComputeBlockVersion shortcut from versionbits tests (Anthony Towns)
c5f36725e877d8eb492383844f8ef7535466b366 [refactor] Move ComputeBlockVersion into VersionBitsCache (Anthony Towns)
4a69b4dbe0d7f504811b67c399da7e6d11e4f805 [move-only] Move ComputeBlockVersion from validation to versionbits (Anthony Towns)
0cfd6c6a8f929d5567ac41f95c21548f115efee5 [refactor] versionbits: make VersionBitsCache a full class (Anthony Towns)
8ee3e0bed5bf2cd3c7a68ca6ba6c65f7b9a72cca [refactor] rpc/blockchain.cpp: SoftForkPushBack (Anthony Towns)
92f48f360da5f425428b761219301f509826bec4 deploymentinfo: Add DeploymentName() (Anthony Towns)
ea68b3a5729f5d240e968388c4f88acffeb27228 [move-only] Rename versionbitsinfo to deploymentinfo (Anthony Towns)
c64b2c6a0f79369624ae96b2e3d579d50aae4de6 scripted-diff: rename versionbitscache (Anthony Towns)
de55304f6e7a8b607e6b3fc7436de50910747b0c [refactor] Add versionbits deployments to deploymentstatus.h (Anthony Towns)
2b0d291da8f479739ff394dd92801da8c40b9f8e [refactor] Add deploymentstatus.h (Anthony Towns)
eccd736f3dc231ac0306ca763c3b72cf8247230a versionbits: Use dedicated lock instead of cs_main (Anthony Towns)
36a4ba0aaaa9b35185d7178994e36bc02cca9887 versionbits: correct doxygen comments (Anthony Towns)

Pull request description:

  Introduces helper functions to make it easy to bury future deployments, along the lines of the suggestion from [11398](https://github.com/bitcoin/bitcoin/pull/11398#issuecomment-335599326) "I would prefer it if a buried deployment wouldn't require all code paths that check the BIP9 status to require changing".

  This provides three functions: `DeploymentEnabled()` which tests if a deployment can ever be active, `DeploymentActiveAt()` which checks if a deployment should be enforced in the given block, and `DeploymentActiveAfter()` which checks if a deployment should be enforced in the block following the given block, and overloads all three to work both with buried deployments and versionbits deployments.

  This adds a dedicated lock for the versionbits cache, which is acquired internally by the versionbits functions, rather than relying on `cs_main`. It also moves moves versionbitscache into deploymentstatus to avoid a circular dependency with validation.

ACKs for top commit:
  jnewbery:
    ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3
  gruve-p:
    ACK e48826ad87
  MarcoFalke:
    re-ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3 🥈

Tree-SHA512: c846ba64436d36f8180046ad551d8b0d9e20509b9bc185aa2639055fc28803dd8ec2d6771ab337e80da0b40009ad959590d5772f84a0bf6199b65190d4155bed
2023-12-01 09:08:50 -06:00
fanquake
f0d8627c87 Merge #18742: miner: Avoid stack-use-after-return in validationinterface
7777f2a4bb1f9d843bc50a4e35085cfbb2808780 miner: Avoid stack-use-after-return in validationinterface (MarcoFalke)
fa5ceb25fce2200edf6b8ebfa6d4f01ed6774b95 test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue (MarcoFalke)
fa770ce7fe67685c43780e219d8232efbee0bb8e validationinterface: Rework documentation, Rename pwalletIn to callbacks (MarcoFalke)
fab6d060ce5f580db538070beec1c5518c8c777c test: Add unregister_validation_interface_race test (MarcoFalke)

Pull request description:

  When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race:

  * The validationinterface destructing itself
  * The validationinterface getting dereferenced for execution

  [1] 64139803f1/src/validationinterface.cpp (L82-L83)

  This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface.

  This issue has been fixed in commit ab31b9d6fe7b39713682e3f52d11238dbe042c16, but the fix has not been applied to the miner.

  Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414

ACKs for top commit:
  promag:
    Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780.
  laanwj:
    Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780

Tree-SHA512: 8087119243c71ba18a823a63515f3730d127162625d8729024278b447af29e2ff206f4840ee3d90bf84f93a2c5ab73b76c7e7044c83aa93b5b51047a166ec3d3
2023-11-26 15:09:55 -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
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
d910b3e465
chore(rpc): renamed from bitcoin to coins in help text (#5659)
## Issue being fixed or feature implemented


## What was done?
Renamed `bitcoin` to `coins` in help texts of mining RPCs.

## How Has This Been Tested?


## 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
- [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-11-03 09:06:02 -05:00
Konstantin Akimov
a0c8c9f0a5
fix: possible assert call if nHeight in CDeterministicMNList is higher then Tip (#5590)
## Issue being fixed or feature implemented
fix: possible assert call if nHeight in CDeterministicMNListDiff is
higher than Tip
    
Example of new log:
```
2023-09-28T17:35:50Z GetProjectedMNPayeesAtChainTip WARNING pindex is nullptr due to height=914160 chain height=914159
```
    
instead assert call:
```
...
     #6  0x00007ffff7a33b86 in __assert_fail (assertion=0x55555783afd2 "pindex", file=0x5555577f2ed8 "llmq/utils.cpp", line=730,
            function=0x5555577f2448 "bool llmq::utils::IsMNRewardReallocationActive(const CBlockIndex*)") at ./assert/assert.c:101
     #7  0x0000555555ab7daf in llmq::utils::IsMNRewardReallocationActive (pindex=<optimized out>) at llmq/utils.cpp:730
     #8  0x00005555559458ad in CDeterministicMNList::GetProjectedMNPayees (this=this@entry=0x7fffffffc690, pindex=0x0, nCount=<optimized out>, nCount@entry=2147483647)
            at evo/deterministicmns.cpp:231
     #9  0x000055555594614f in CDeterministicMNList::GetProjectedMNPayeesAtChainTip (this=this@entry=0x7fffffffc690, nCount=nCount@entry=2147483647) at evo/deterministicmns.cpp:216
     #10 0x00005555558c9f51 in MasternodeList::updateDIP3List (this=this@entry=0x55555908cfd0) at qt/masternodelist.cpp:194
     #11 0x00005555558ca9a0 in MasternodeList::updateDIP3ListScheduled (this=0x55555908cfd0) at qt/masternodelist.cpp:157
     #12 0x000055555684a60f in void doActivate<false>(QObject*, int, void**) ()
     #13 0x00005555568525b1 in QTimer::timerEvent(QTimerEvent*) ()
     #14 0x0000555556844ce5 in QObject::event(QEvent*) ()
     #15 0x0000555556ac3252 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
     #16 0x000055555681e6b8 in QCoreApplication::sendEvent(QObject*, QEvent*) ()
     #17 0x000055555686de2a in QTimerInfoList::activateTimers() ()
     #18 0x000055555686be84 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #19 0x00005555569bf8a2 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #20 0x000055555681caf6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #21 0x0000555556825f8a in QCoreApplication::exec() ()
...
```

## What was done?
ClientModel returns now a pair: MNList and CBlockIndex; so, we always
know the which one has been used even if current chain is switched.

## How Has This Been Tested?
Run on my localhost from `c034ff0c2606142ba3e8894bc74f693b87374e5c` -
aborted with backtrace like above.
With both of commit - no assert more.


## 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: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-10-27 19:53:27 -05:00
Odysseas Gabrielides
1717d2f607
feat(rpc): return activation_height in getblockchaininfo for BIP9 softforks (#5624)
## Issue being fixed or feature implemented
When expecting a hard fork, we manually calculate activation heights.

## What was done?
Returning expected activation height for BIP9 softporks in `locked_in`
status in `getblockchaininfo` RPC.

## 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-10-23 10:56:10 -05:00
Konstantin Akimov
b96f3e794b fix: missing changes from "merge bitcoin#16127: more thread safety annotation coverage" 2023-10-23 10:46:52 -05:00
UdjinM6
5d8ffe56dc
fix: add rpc help text for "ehf" field (#5628)
## Issue being fixed or feature implemented
#5597 follow-up 

## What was done?
add missing filed description

## How Has This Been Tested?
`help getblockchaininfo`

## 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-20 10:04:55 -05:00