8f5dc8800aeb524eee2fa2451cd22883b7b2bfec test: display command line options passed to send_cli() in debug log (Jon Atack)
Pull request description:
as per https://github.com/bitcoin/bitcoin/pull/18691#discussion_r411382589, and revert two cli calls changed in #18691 from rpc commands back to command line options (these were the only occurrences).
ACKs for top commit:
MarcoFalke:
ACK 8f5dc8800aeb524eee2fa2451cd22883b7b2bfec
Tree-SHA512: fcb3eca00aa4099066028c90d5e50a02e074366e09a17f5f5b937d9f7562dd054ff65681aa0ad4c94f6de1e98b1e2b9ac4cd084ddc297010253989a80483b1b9
## Issue being fixed or feature implemented
This is an implementation of DIP0027 "Credit Asset Locks".
It's a mechanism to fluidly exchange between Dash and credits.
## What was done?
This pull request includes:
- Asset Lock transaction
- Asset Unlock transaction (withdrawal)
- Credit Pool in coinbase
- Unit tests for Asset Lock/Unlock tx
- New functional test `feature_asset_locks.py`
RPC: currently locked amount (credit pool) is available through rpc call
`getblock`.
## How Has This Been Tested?
There added new unit tests for basic checks of transaction validity
(asset lock/unlock).
Also added new functional test "feature_asset_locks.py" that cover
typical cases, but not all corner cases yet.
## Breaking Changes
This feature should be activated as hard-fork because:
- It adds 2 new special transaction and one of them [asset unlock tx]
requires update consensus rulels
- It adds new data in coinbase tx (credit pool)
## 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
**To release DIP 0027**
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Refusing to process `dsq` will result in node not being able to process
`dstx`es later.
## What was done?
## 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)_
## Issue being fixed or feature implemented
V19 is active on mainnet/testnet now, no need to check activation bits
anymore. This PR also bumps `MinBIP9WarningHeight` to
post-v19-activation height which should stop `unknown new rules
activated (versionbit 8)` warning from appearing.
## What was done?
Bury v19, bump `MinBIP9WarningHeight`
## How Has This Been Tested?
Run tests, reindex on mainnet/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)_
5aadd4be1883386a04bef6a04e9a1142601ef7a7 Convert amounts from float to decimal (Prayank)
Pull request description:
> decimal is preferred in accounting applications
https://docs.python.org/3.8/library/decimal.html
Decimal type saves an exact value so better than using float.
~~3 variables declared with type as 'Decimal' in [test/functional/mempool_accept.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_accept.py): fee, fee_expected, output_amount~~
~~Not required to convert to string anymore for using the above variables as decimal~~
+ fee, fee_expected, output_amount
~~+ 8 decimal places~~
+ Using value of coin['amount'] as decimal and removed 'int'
+ Removed unnecessary parentheses
+ Remove str() and use quotes
Fixes https://github.com/bitcoin/bitcoin/issues/20011
ACKs for top commit:
guggero:
ACK 5aadd4be1883386a04bef6a04e9a1142601ef7a7
Tree-SHA512: 5877cf3837e5b65bec0fc8909de141a720bfa02a747513e21d20f3c41ec0cfecc524d2c347a96596b0a1a97900da2acf08b799f26b11d537e4dcddc6ce45f38e
d76925478efd35e6fd835370639f2139b28381e4 [doc] Clarify semantic of peer's m_protect w.r.t to outbound eviction logics (Antoine Riard)
ac71fe936da290adf5a3155fe8db5f78b485f1f1 [doc] Clarify scope of eviction protection of outbound block-relay peers (Antoine Riard)
Pull request description:
Block-relay-only peers were introduced by #15759. According to its
author, it was intented to make them only immune to outbound peer
rotation-based eviction and not from all eviction as modified comment
leans to think of.
Clearly indicate that outbound block-relay peers aren't protected
from eviction by the bad/lagging chain logic.
Fix#19863
ACKs for top commit:
naumenkogs:
ACK d76925478efd35e6fd835370639f2139b28381e4
jonatack:
ACK d76925478efd35e6fd835370639f2139b28381e4
Tree-SHA512: 597fbd62838a6e39276024165b11514cad20a2e9d33cf9202d261cbadcb62b2df427c858e0cb57e585840d4c1d4600104aa53916bb868541f2580e4eed9b4b52
fab94534b64593be1620c989bf69eb02e1be9b1b doc: Document that wallet salvage is experimental (MarcoFalke)
Pull request description:
See #20151
ACKs for top commit:
practicalswift:
ACK fab94534b64593be1620c989bf69eb02e1be9b1b: user safety first
hebasto:
ACK fab94534b64593be1620c989bf69eb02e1be9b1b, maybe capitalize into "WARNING"?
meshcollider:
Trivial ACK fab94534b64593be1620c989bf69eb02e1be9b1b
Tree-SHA512: 94912c491facc485293e4333066057933d706d84c7172f615296e7ba998c583c8bd07e751e6f00cd6576e7791007ace321f959181f7bf6a4e15e10d7ec8a1b7e
0956e46bff7f0b6da65a4de6d4f8261fe9d7055c test: use zero-argument super() shortcut (Python 3.0+) (Sebastian Falbesoner)
Pull request description:
This mini-PR replaces all calls to `super(...)` with arguments with the zero-argument shortcut `super()` where applicable. See [PEP 3135](https://www.python.org/dev/peps/pep-3135/#specification):
> The new syntax:
>
> super()
>
> is equivalent to:
>
> super(__class__, <firstarg>)
>
> where __class__ is the class that the method was defined in, and <firstarg> is
> the first parameter of the method (normally self for instance methods, and cls
> for class methods).
ACKs for top commit:
fanquake:
ACK 0956e46bff7f0b6da65a4de6d4f8261fe9d7055c
Tree-SHA512: 4ac66fe7ab2be2e8a514e5fcfc41dbb298f21b23ebb7b7b0310d704b0b3cef8adf287a8d80346d1ea9418998c597b4f0ff1f66148d0d806bb43db6607e0fe1cf
fc289b7898fb90d4800675b69c0bb9b42df5599f wallet: Refactor WalletRescanReserver to use wallet reference (João Barbosa)
Pull request description:
Simple refactor to `WalletRescanReserver` to use wallet reference instead of pointer.
Complements #18259.
ACKs for top commit:
MarcoFalke:
ACK fc289b7898fb90d4800675b69c0bb9b42df5599f
Tree-SHA512: b03e33f2d9df2870436aa3284137fd022dd89ea96a1b170fa27f8685ad4f986e6c4ba5975a84966c30d18430a4014d7d8740a1dff2f985c9ef8226ed18e69db9
a2324e4d3f47f084b07a364c9a360a0bf31e86a0 test: Improve naming and logging of avoid_reuse tests (Fabian Jahr)
1abbdac6777bc5396d17a6772c8176a354730997 wallet: Prefer full destination groups in coin selection (Fabian Jahr)
Pull request description:
Fixes#17603 (together with #17843)
In the case of destination groups of >10 outputs existing in a wallet with `avoid_reuse` enabled, the grouping algorithm is adding left-over outputs as an "incomplete" group to the list of groups even when a full group has already been added. This leads to the strange behavior that if there are >10 outputs for a destination the transaction spending from that will effectively use `len(outputs) % 10` as inputs for that transaction.
From the original PR and the code comment I understand the correct behavior should be the usage of 10 outputs. I opted for minimal changes in the current code although there maybe optimizations possible for cases with >20 outputs on a destination this sounds like too much of an edge case right now.
ACKs for top commit:
jonatack:
Re-ACK a2324e4
achow101:
ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0
kallewoof:
ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0
meshcollider:
Tested ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 (verified the new test fails on master without this change)
Tree-SHA512: 4743779c5d469fcd16df5baf166024b1d3c8eaca151df1e8281b71df62b29541cf7bfee3f8ab48d83e3b34c9256e53fd38a7b146a54c79f9caa44cce3636971a
d67055e00dd90f504384e5c3f229fc95306d5aac Upgrade or rewrite encrypted key checksums (Andrew Chow)
c9a9ddb4142af0af5f7b1a5ccd13f8e585007089 Set fDecryptionThoroughlyChecked based on whether crypted key checksums are valid (Andrew Chow)
a8334f7ac39532528c5f8bd3b0eea05aa63e8794 Read and write a checksum for encrypted keys (Andrew Chow)
Pull request description:
Adds a checksum to the encrypted key record in the wallet database so that encrypted keys can be checked for corruption on wallet loading, in the same way that unencrypted keys are. This allows for us to skip the full decryption of keys upon the first unlocking of the wallet in that session as any key corruption will have already been detected. The checksum is just the double SHA256 of the encrypted key and it is appended to the record after the encrypted key itself.
This is backwards compatible as old wallets will be able to read the encrypted key and ignore that there is more data in the stream. Additionally, old wallets will be upgraded upon their first unlocking (so that key decryption is checked before we commit to a checksum of the encrypted key) and a wallet flag set indicating that. The presence of the wallet flag lets us skip the full decryption as if `fDecryptionThoroughlyChecked` were true.
This does mean that the first time an old wallet is unlocked in a new version will take much longer, but subsequent unlocks will be instantaneous. Furthermore, corruption will be detected upon loading rather than on trying to send so wallet corruption will be detected sooner.
Fixes#12423
ACKs for top commit:
laanwj:
code review ACK d67055e00dd90f504384e5c3f229fc95306d5aac
jonatack:
Code review ACK d67055e00dd90f504384e5c3f229fc95306d5aac
meshcollider:
Code review ACK d67055e00dd90f504384e5c3f229fc95306d5aac
Tree-SHA512: d5c1c10cfcb5db9e10dcf2326423565a9f499290b81f3155ec72254ed5bd7491e2ff5c50e98590eb07842c20d7797b4efa1c3475bae64971d500aad3b4e711d4
25dac9fa65243ca8db02df22f484039c08114401 doc: add release notes for explicit fee estimators and bumpfee change (Karl-Johan Alm)
05227a35545d7656450874b3668bf418c73813fb tests for bumpfee / estimate_modes (Karl-Johan Alm)
3404c1b753432c4859a4ca245f01c240610a00cb policy: optional FeeEstimateMode param to CFeeRate::ToString (Karl-Johan Alm)
6fcf4484302d13bd7739b617470d8c8e31974908 rpc/wallet: add two explicit modes to estimate_mode (Karl-Johan Alm)
b188d80c2de9ebb114da5ceea78baa46bde7dff6 MOVEONLY: Make FeeEstimateMode available to CFeeRate (Karl-Johan Alm)
5d1a411eb12fc700804ffe5d6e205234d30edd5f fees: add FeeModes doc helper function (Karl-Johan Alm)
91f6d2bc8ff4d4cd1b86daa370ec9d2d9662394d rpc/wallet: add conf_target as alias to confTarget in bumpfee (Karl-Johan Alm)
69158b41fc488e4f220559da17a475eff5923a95 added CURRENCY_ATOM to express minimum indivisible unit (Karl-Johan Alm)
Pull request description:
This lets users pick their own fees when using `sendtoaddress`/`sendmany` if they prefer this over the estimators.
ACKs for top commit:
Sjors:
re-utACK 25dac9fa65: rebased, more fancy C++,
jonatack:
ACK 25dac9fa65243ca8db02df2 I think this should be merged after all this time, even though it looks to me like there are needed follow-ups, fixes and test coverage to be added (see further down), which I don't mind helping out with, if wanted.
fjahr:
Code review ACK 25dac9fa65243ca8db02df22f484039c08114401
Tree-SHA512: f31177e6cabf3187a43cdfe93477144f8e8385c7344613743cbbd16e8490d53ff5144aec7b9de6c9a65eb855b55e0f99d7f164dee4b6bf3cfea4dce51cf11d33
fab860aed4878b831dae463e1ee68029b66210f5 fuzz: Stop nodes in process_message* fuzzers (MarcoFalke)
6666c828e072a5e99ea0c16394ca3e5b9de07409 fuzz: Give CNode ownership to ConnmanTestMsg in process_message fuzz harness (MarcoFalke)
Pull request description:
Background is that I saw an integer overflow in net_processing
```
#30629113 REDUCE cov: 25793 ft: 142917 corp: 3421/2417Kb lim: 4096 exec/s: 89 rss: 614Mb L: 1719/4096 MS: 1 EraseBytes-
net_processing.cpp:977:25: runtime error: signed integer overflow: 2147483624 + 100 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:977:25 in
net_processing.cpp:985:9: runtime error: signed integer overflow: -2147483572 - 100 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:985:9 in
```
Telling from the line numbers, it looks like `nMisbehavior` wrapped around.
Fix that by calling `StopNodes` after each exec, which should clear the node state and thus `nMisbehavior`.
ACKs for top commit:
practicalswift:
ACK fab860aed4878b831dae463e1ee68029b66210f5
Tree-SHA512: 891c081d5843565d891aec028b6c27ef3fa39bc40ae78238e81d8f784b4d4b49cb870998574725a5159dd03aeeb2e0b9bc3d3bb51d57d1231ef42e3394b2d639
## Issue being fixed or feature implemented
Here's TODO that seems out-dated
```
/// TODO: all 4 functions do not belong here really, they should be refactored/moved somewhere (main.cpp ?)
```
This changes are extracted from this PR:
https://github.com/dashpay/dash/pull/5342
## What was done?
This changes hides some methods from global namespace (making local
static function), hiding other functions to the namespace
## How Has This Been Tested?
Run unit/functional 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
## What was done?
write in logs of TxMempool tx's hashes instead whole txes
## How Has This Been Tested?
Run unit/functional 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
## Issue being fixed or feature implemented
Legacy IS messages are gone long time ago, no need to keep them in code.
## What was done?
Drop `MSG_LEGACY_TXLOCK_REQUEST`/`LEGACYTXLOCKREQUEST`
## 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)_
5 minute profiling shows previous usage around ~7% and current usage
around ~2%
## Issue being fixed or feature implemented
Due to us rapidly receiving multiple duplicates of DSQueue's, we start
processing them before it's added the the vector of processed ones, we
probably at one point tried to minimize locked time, but that's not
productive here
## What was done?
Expand the locked scope to ensure we don't double process.
## How Has This Been Tested?
Ran full node for 5-10 minutes
## 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: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Implementation of Randomness Beacon Part 3.
Starting from v20 activation fork, members for quorums are sorted using
(if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using
Blockhash instead)
The actual new way to shuffle is already implemented in
https://github.com/dashpay/dash/pull/5366.
SPV clients also need to calculate members, but they only know block
headers.
Since Coinbase is in the actual block, then they lack the required
information to correctly calculate quorum members.
## What was done?
- Message `MNLISTIDFF` is enriched with a new field `quorumsCLSigs`.
This field holds the Chainlock Signature required for each set of
indexes corresponding to quorums in field `newQuorums`.
- Protocol version has been bumped to `70230`.
- Clients with protocol version greater or equal to `70230` will receive
the new field `quorumsCLSigs`.
- The same field is returned in `protx diff` RPC.
Note:
- Field `quorumsCLSigs` will populated only after v20 activation
- If for one or more quorums, no non-null CL sig was found in CbTx then
a null signature is returned in `quorumsCLSigs`.
## How Has This Been Tested?
- Functional test mininode's protocol version was bumped to `70230`.
- `feature_llmq_rotation.py` checks that `quorumsCLSigs` match in both
P2P and RPC messages.
## 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: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
## Issue being fixed or feature implemented
It splits from https://github.com/dashpay/dash/pull/5150/ by
@PastaPastaPasta request.
## What was done?
See commits
## How Has This Been Tested?
Run unit/functional 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
b1f59d55d920d2b35269b474762f94fec87bfb16 RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC endpoint and wallet_name parameter specify a wallet (Luke Dashjr)
Pull request description:
Just documentation clarifications from #20448
ACKs for top commit:
MarcoFalke:
review ACK b1f59d55d920d2b35269b474762f94fec87bfb16
jonatack:
re-ACK b1f59d55d920d2b35269b474762f94fec87bfb16 per `git diff e8303a0 b1f59d5`
Tree-SHA512: ac068b0aa7ceed49496367fdd9425b59dbba18b56e89b26afc22a6c8ece51f0b92a169cacd55740b1cadab2b32f4f8e8700e609066ab7e59d3b53c7891da585e
c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7 Updated outdated help command for getblocktemplate (fixes#19625) (Jake Leventhal)
Pull request description:
**Summary of Changes**
* Removed coinbasetxn from the help outputs
* Added the missing name for transactions in the help outputs
* Added help outputs for longpollid and default_witness_commitment
* Added more clarity to capabilities, rules, and coinbaseaux
**Rationale**
The outputs from the help command for `getblocktemplate` are outdated and don't reflect the actual results from `getblocktemplate` (see #19625 for more details)
Fixes#19625.
ACKs for top commit:
laanwj:
ACK c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7
fjahr:
utACK c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7
Tree-SHA512: ee443af4bc3b2838dfd92e2705f344256ee785ae720e505fffea9b0ec5b75930e3b1374bae59b36d5da57c85c9aefe4d62504b028b893d6f2914dccf1e34c658
