05b56d1c937b7667ad51400d2f9fb674af72953f [wallet] Remove CMerkleTx serialization logic (John Newbery)
783a76f23ba4f33b6e6f609eaf3bf41afd9bcd6f [wallet] Flatten CWalletTx class hierarchy (John Newbery)
b3a9d179f23f654b8fba63924bbca5fd31ad4bb0 [wallet] Move CMerkleTx functions into CWalletTx (John Newbery)
Pull request description:
CMerkleTx is only used as a base class for
CWalletTx. It was previously also used for vtxPrev which
was removed in 93a18a3650.
This PR moves all of the CMerkleTx members and logic
into CWalletTx. The CMerkleTx class is kept for deserialization
and serialization of old wallet files.
This makes the refactor in #15931 cleaner.
ACKs for top commit:
laanwj:
ACK 05b56d1c937b7667ad51400d2f9fb674af72953f. Looks good to me.
Tree-SHA512: 3d3a0069ebb536b12a328f1261e7dc55158a71088d445ae4b4ace4142c432dc296f58c8183b1922e54a60b8cc77e9d17c3dce7478294cd68693594baacf2bab3
29ee4c417d97dca29c4ef53b6c1a55caa902787a Specify AM_CPPFLAGS for ZMQ. (Daniel Kraft)
Pull request description:
When building the ZMQ static library, add `AM_CPPFLAGS` to the library `CPPFLAGS`. Otherwise, we may miss important flags that are specified elsewhere. For instance, if `--enable-debug` is passed and
`-DDEBUG_LOCKORDER` set, then that would not apply to the ZMQ library before (causing potential for hard-to-find bugs).
ACKs for top commit:
laanwj:
utACK 29ee4c417d97dca29c4ef53b6c1a55caa902787a
Tree-SHA512: 64085d71ed3f435a6e4df6dc42bda8b6159a4d292d0547c5b38c09d6ac95e976ad1728cd65278bffdd57363f60a58eb762b1171dafbe055cf94ffcd4f66da877
faa88d0b5c7f6e317d0e35daef28d320801f1bb5 doc: update labels in CONTRIBUTING.md (MarcoFalke)
Pull request description:
None of the examples in the "trivial" area are acceptable pull requests, unless they are acceptable in a different area (like "doc" or "log").
Fix that by removing the "trivial" area.
ACKs for top commit:
jonatack:
ACK faa88d0b5c7f6e317d0e35daef28d320801f1bb5
fanquake:
ACK faa88d0b5c7f6e317d0e35daef28d320801f1bb5 - agree that trivial was pretty useless and that the meaning was unclear. Other changes look fine. Surprised the white space linter hasn't been having a field day in this file.
Tree-SHA512: 6208bcc7c84ad0ca6aeaa2de1901c9da8971aac332b5e7a1194ea7b24fb2d887f988aa22fdfa818e89cbcfd8cb8595ce312525f88c81c5ade484fd7c9bd13d1b
4057b7acb7125739537078d026ad96bb21708e3c wallet: Recognize -disablewallet option early (Hennadii Stepanov)
Pull request description:
This PR makes early check for the `-disablewallet` option.
If `-disablewallet=1`, objects `PaymentServer` and `WalletController` are nor created.
ACKs for top commit:
jonasschnelli:
utACK 4057b7acb7125739537078d026ad96bb21708e3c
laanwj:
ACK 4057b7acb7125739537078d026ad96bb21708e3c
Tree-SHA512: 74633cd1eacd0914c73712e6dff190255b5378595cfee7eaeb91e17671fc9120928034739f4ae1c53b86f46c4b400390877241384376b2fc534de326d3ab0944
07e01d6258831fd2dfef959a405b3dfe4e88c3c1 rpc: sendrawtransaction unconditionality/privacy note (Jon Atack)
Pull request description:
In sendrawtransaction RPCHelpMan, mention unconditionality and privacy as per http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-25.html#l-522
before
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submits raw transaction (serialized, hex-encoded) to local node and network.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
after
```
$ bitcoin-cli help sendrawtransaction
sendrawtransaction "hexstring" ( maxfeerate )
Submit a raw transaction (serialized, hex-encoded) to local node and network.
Note that the transaction will be sent unconditionally to all peers, so using this
for manual rebroadcast may degrade privacy by leaking the transaction's origin, as
nodes will normally not rebroadcast non-wallet transactions already in their mempool.
Also see createrawtransaction and signrawtransactionwithkey calls.
(...)
```
ACKs for top commit:
promag:
ACK 07e01d6258831fd2dfef959a405b3dfe4e88c3c1.
laanwj:
ACK 07e01d6258831fd2dfef959a405b3dfe4e88c3c1
Tree-SHA512: 427b3ca29384eef271eb496b7b14e883220863543a536ddeb31940aaffd52ea0b607d929d50f2b7958514105ef7823fa05c1ee381d4a432808753c06bd97af58
248e22bbc0d7bc40ae3584d53a18507c46b0e553 depends: disable unused Qt features (fanquake)
Pull request description:
Related to #16354. Kept separate from #16370, because:
> QT is a monster 😂 - dongcarl in #bitcoin-builds
I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows.
I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in #16354.
ACKs for top commit:
sipsorcery:
tACK 248e22bbc0d7bc40ae3584d53a18507c46b0e553 (Windows 10 test only)
laanwj:
ACK 248e22bbc0d7bc40ae3584d53a18507c46b0e553
Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0
fa56b21c744577eb5d2085fd6104cbc3dc62d677 doc: Update bips 35, 37 and 111 status (MarcoFalke)
Pull request description:
Follow-up to
* #16152: Disable bloom filtering by default
ACKs for top commit:
laanwj:
ACK fa56b21c744577eb5d2085fd6104cbc3dc62d677, thanks for keeping this file up to date
fanquake:
ACK fa56b21c744577eb5d2085fd6104cbc3dc62d677
Tree-SHA512: 50c17067abbba096e8604b8abccbd0474ecc2dca973935751720c79a023a2d517bb025bb6c36d4fb0d29b7338322af7ae1c0d5a9aaf2712000d9d336c898c204
* refactor: break circular dependencies(-13, +2)
introduces specialtxman, which handles validation of special transactions, specialtx is now simply the primitive underlying type. This breaks a lot of the circular depends
Also removes an unneeded `#include <masternode/payments.h>` in net_processing.cpp, which resolves a circular dependency. (we know it's okay to remove b/c masternode/payments.h isn't included in any header files, and removing it doesn't break compilation)
* format: make clang-format happy
* remove unrelated change
* remove some unneeded includes to `evo/deterministicmns.h`, explicitly include some previously implicitly included includes.
Resolves two circular dependencies
* refactor: remove circular depend, unused include
107e030723552cf272dc8da01bb682032a457a3d build: make protobuf optional in depends (fanquake)
ff6122f32b21fa00e9308e098b33b9657debc1d7 doc: clarify protobuf build requirements (fanquake)
Pull request description:
As mentioned by dongcarl in https://github.com/bitcoin/bitcoin/pull/15584#issuecomment-521780972, make building `protobuf` optional in depends. With this change it will only be built if you pass `PROTOBUF=1`.
ACKs for top commit:
laanwj:
code review ACK 107e030723552cf272dc8da01bb682032a457a3d
Sjors:
tACK 107e030 on macOS 10.14. When I build depends with `PROTOBUF=1` then `./configure` has `bip70` enabled.
Tree-SHA512: 49bc247a6879aaf55b943a3d0b930544ddef1e69a481955a8bebe0b02c9ad0fe168b93025f34168334cef34bb567478eb98eacab62ba909f2f64fb21119c71b8
c4b0c08f7c91bcef48dd023982ff132795575247 Update tx-size-small comment with relevant CVE disclosure (Gregory Sanders)
Pull request description:
Code first introduced under https://github.com/bitcoin/bitcoin/pull/11423 with essentially no description and no discussion.
ACKs for top commit:
MarcoFalke:
ACK c4b0c08f7c91bcef48dd023982ff132795575247
fanquake:
ACK c4b0c08f7c91bcef48dd023982ff132795575247
Tree-SHA512: 95d5c92998b8b1e944c477dbaee265b62612b6e815099ab31d9ff580b4dff777abaf7f326a284644709f918aa1510412d62310689b1250ef6e64de7b19ca9f71
59e387705c7e55ec40400301346354fa2d0c613f test: add invalid tx templates for use in functional tests (James O'Beirne)
Pull request description:
This change adds a list of `CTransaction`-generating templates which each correspond to a specific type of invalid transaction. We then use this list to test for a wider variety of invalid tx types in `p2p_invalid_tx.py` and `feature_block.py`.
Consolidating all invalid tx types will allow us to more easily cover all tx reject cases from a variety of tests without repeating ourselves. Validation logic doesn't differ much between mempool and block acceptance, but there *is* a difference and we should be sure we're testing both comprehensively.
Right now, I've only added templates covering the tx reject types listed below but if this approach seems worthwhile I will expand the list to be fully comprehensive.
```
bad-txns-in-belowout
bad-txns-inputs-duplicate
bad-txns-too-many-sigops
bad-txns-vin-empty
bad-txns-vout-empty
bad-txns-vout-negative
```
Tree-SHA512: 05407f4a953fbd7c44c08bb49bb989cefd39a2b05ea00f5b3c92197a3f05e1b302f789e33832445734220e1c333d133aba385740b77b84139b170c583471ce20
2222c96deec0f636dee6e49efb745f29b06a40a5 test: Add notes on how to generate data/wallets/high_minversion (MarcoFalke)
Pull request description:
I forgot to do this in #16796
ACKs for top commit:
ryanofsky:
ACK 2222c96deec0f636dee6e49efb745f29b06a40a5
Tree-SHA512: 5f24ffa641b97eac4febad42ade7228b14fa72335c918a10880c5dec86a3ecc3075a31526f275188e07fea95b8e2c6320c64f716099f604b00e13d5366fcee37
c0b5d9710322a614a50ab5da081558cf6a38ad2a Test that joinpsbts randomly shuffles the inputs (Andrew Chow)
6f405a1d3b38395e35571b68aae55cae50e0762a Shuffle inputs and outputs after joining psbts (Andrew Chow)
Pull request description:
`joinpsbts` currently just adds the inputs and outputs in the order of that the PSBTs were provided. This makes it extremely easy to identify which outputs belong to which inputs. This PR changes that so that all of the inputs and outputs are shuffled in the joined transaction.
ACKs for top commit:
instagibbs:
utACK c0b5d97103
jonatack:
ACK c0b5d9710322a614a50ab5da081558cf6a38ad2a modulo suggestions for later.
Tree-SHA512: 14a0b7aae07d92e6d2c76a3a3b228b481e1964cb7d34f97515bdda18e2ea05a9f97c5a22affc143b86ae8b95c3cb239849fb54219d65512bc2112264dca915c8
6aab7649d30b19d136a27f1287fd2c8b00fb460c doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)
Pull request description:
Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.
Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.
ACKs for top commit:
fanquake:
ACK 6aab7649d30b19d136a27f1287fd2c8b00fb460c - Thanks for following up. Hopefully we now never have to deal with whitespace again.
Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
b6233a4985519fea2ca35ae6d45877bb2df697f9 bitcoin-wallet: Add a missing closing parenthesis in the help (darosior)
Pull request description:
ACKs for top commit:
kristapsk:
utACK b6233a4985519fea2ca35ae6d45877bb2df697f9
fanquake:
ACK b6233a4985519fea2ca35ae6d45877bb2df697f9
Tree-SHA512: acf18633fdca4bd73838fcaa0ebe4121dd0b5308daa77c4458ec4c98a9e8aa6d9d6580a48c884147438af14e670b0606c1e76f72d1d7efd221c4da419061beed
* introduce handle_potential_conflicts.py to only warn on true conflicts. Fail CI when there is a conflict
Signed-off-by: Pasta <pasta@dashboost.org>
* remove unneeded input assign
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* use dashpay instead of PastaPastaPasta
Signed-off-by: Pasta <pasta@dashboost.org>
* Update .github/workflows/handle_potential_conflicts.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* more linter fixes
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* Make numerous coinjoin functions constexpr
* remove empty function
Signed-off-by: Pasta <pasta@dashboost.org>
* remove InitStandardDenominations calls
Signed-off-by: Pasta <pasta@dashboost.org>
* use default constructors, add constexpr to CCoinJoinStatusUpdate constructor
Signed-off-by: Pasta <pasta@dashboost.org>
* remove default constructor
Signed-off-by: Pasta <pasta@dashboost.org>
* refactor: use a vector instead of map for llmqs
this is a valuable refactor for a number of reasons.
it forces the removal of more verbose `Params().GetConsensus().llmqs.count` and instead call to `Params().HasLLMQ()`
`llmqs` is now stored in contiguous memory (which hopefully means better lookup time / iteration time)
std::vector is much more constexpr friendly, and normally is better optimized
Signed-off-by: Pasta <pasta@dashboost.org>
* use copy_if
Signed-off-by: Pasta <pasta@dashboost.org>
* fixes
Signed-off-by: Pasta <pasta@dashboost.org>
Make fMixing atomic as it has concurrent access
Add tsan suppression for zmq namespace
Suppress deadlock false positive in ConnectTip
Switch ubsan target to linux32
Add new test job for linux64_cxx17 target without any sanitizers
Increase rpc time out for block reward reallocation test
Fix heap use after free in CConnman::GetExtraOutboundCount()
Different builds for linux32 and linux64 tsan and ubsan
Increase timeout for llmq_signing functional test
* refactor: numerous changes to avoid passing around a const ref to shared_ptr of CDeterministicMNC when not needed.
Introduces ForEachMNShared, a version of ForEachMN that uses a shared_ptr, and may extend the lifetime of the underlying shared_ptr. This is not preferred, should prefer ForEachMN. See docs.
Adjusts ForEachMN to pass a reference. This is preferred for use over ForEachMNShared. See docs. A reference should be used since in usage we assume it's non-null anyway. Additionally, it allows us to know that the lifespan of the dmn is not being being extended (if lifespan needs to be extended, should use ForEachMNShared.
IsMNValid, IsMNPoSeBanned, UpdateMN, UpdateMN, AddUniqueProperty, DeleteUniqueProperty, UpdateUniqueProperty now take a const reference to CDeterministicMN instead of a const reference to shared_ptr<CDeterministicMN>. All of these functions previously assumed (or would've crashed) a non-null ptr, and non extended lifetime, as such converting to ref is appropriate.
CompareByLastPaid ptr overload now takes raw ptr instead of a const ref to shared. Since we simply dereference them, a raw ptr makes the most sense. This also avoids a potential expensive and implicit raw ptr -> shared ptr conversion if the function was called with raw ptrs.
rpcevo BuildDMNListEntry now takes a const ref for reasons as stated above
Signed-off-by: Pasta <pasta@dashboost.org>
* make stuff const
Signed-off-by: Pasta <pasta@dashboost.org>
* refactor/llmq: use ranges count_if
Signed-off-by: Pasta <pasta@dashboost.org>
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
fab0c820fa4c0c3227eec85c64310a3bf938a149 rpc: Clarify that block count means height excl genesis (MarcoFalke)
Pull request description:
There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.
However, it really returns the height, which is `0` for the genesis block.
So clarify that and also remove the misleading "longest blockchain" comment.
Finally, fix the wallet test that incorrectly used this rpc.
ACKs for top commit:
instagibbs:
utACK fab0c820fa
promag:
ACK fab0c82, sorry for the misconception.
Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
99e88a372 rpc: Remove dependency on interfaces::Chain in SignTransaction (Antoine Riard)
Pull request description:
Assuming wallet RPCs and node RPCs will go into different processes, signrawtransactionwithkey doesn't need to access Coins via interfaces::Chain, it may use directly utility in node/coins.cpp
Obviously will need rebase after #15638
Tree-SHA512: 42ee8fcbcd38643bbd82210db6f68249bed5ee036a4c930a1db534d0469a133e287b8869c977bf0cc79a7296dde04f72adb74d24e1cd20f4a280f4c2b7fceb74
9b085f4863eaefde4bec0638f1cbc8509d6ee59a Mention new descriptor RPCs in descriptors.md (Pieter Wuille)
28d78de00bf49c6efc19f0edeef2a03d039752a1 Mention new PSBT RPCs in psbt.md (Pieter Wuille)
Pull request description:
The documentation in `psbt.md` and `descriptors.md` does not list new and updated RPCs (`analyzepsbt`, `utxoupdatepsbt`, `joinpsbts`, `deriveaddresses`, `getdescriptorinfo`, `listunspent`). Fix this.
It'd be good to have this in 0.18 (only documentation).
ACKs for commit 9b085f:
fanquake:
utACK 9b085f4
Tree-SHA512: ee16907e8c15351a530f11fc0a585c50835a7bf5aec997ac0e897949d9b9e41a28ddebbeaba69753fee7d2de75e518091518185085fcd1f6ada94b7231097b2e
318b1f7af1 [wallet] Close bdb when flushing wallet. (John Newbery)
Pull request description:
bdb would not be closed when closing the wallet in wallet-tool. Fix this by calling wallet->flush with true.
Tree-SHA512: f722e527e4806eca5254221e944f57853d11bf89a9264309fa558a6cc2b23feefb7bb2963e87b4fad9cfb31ac4cffe563688988e0614a481a8ff1d393aceb132
fa4c8679ed94f215ce895938f7c3c169a2ce101e rpc: Avoid creating non-standard raw transactions (MarcoFalke)
Pull request description:
Multiple OP_RETURN outputs in a transaction are not standard and unlikely to be relayed, so avoid creating them.
Apart from that, the logic was broken in that it duplicated the same hex-data for each data output: Closes#14868.
Tree-SHA512: b08d08062b5622e8a7b497e490ccaf53b06e844c863fda3bf3f932a98684a809e8341aeb98232059a795afb32d8770a6c5591a66f8e6ee372b672af245607887
* use pull_request_target instead of pull_request
Signed-off-by: Pasta <pasta@dashboost.org>
* use pasta based action 0.1.1
Signed-off-by: Pasta <pasta@dashboost.org>
* optimize: adjust ci dockerfile for faster building
* remove old comment
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* remove unneeded semicolons
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* format: make each installed package it's own line to minimize conflicts
* sort the installed packages (and a fix)
Signed-off-by: Pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* optimize: somehow optimize circular-dependencies.py
Signed-off-by: pasta <pasta@dashboost.org>
* optimize: use parallel if available to lint in parallel
Signed-off-by: pasta <pasta@dashboost.org>
* suggestions
* more suggestions
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* src/evo/evodb.cpp:57:29: warning: Assert statement calls a function which may have desired side effects: 'IsClean'. [assertWithSideEffect]
* src/llmq/quorums.cpp:635:37: note: Null pointer dereference
src/llmq/quorums.cpp:635:37: warning: Either the condition 'pFrom==nullptr' is redundant or there is possible null pointer dereference: pFrom. [nullPointerRedundantCheck]
src/llmq/quorums.cpp:636:81: note: Assuming that condition 'pFrom==nullptr' is not redundant
* fix a bunch of cppcheck warnings
* cppcheck: run on many more files. Enable all checks except a few ignored ones.
ignored
```
"Consider using std::transform algorithm instead of a raw loop."
"Consider using std::accumulate algorithm instead of a raw loop."
```
* ci: build specific version of cppcheck instead of install from apt
* ci: use cppcheck 2.4, remove commented out line, fix symlink
cppcheck 2.6 is latest, however causes issues
```
src/spork.cpp:135:51: warning: Analysis failed. If the code is valid then please report this failure. [cppcheckError]
```
cppcheck 2.5 appears to get into an infinite loop
* no need to check presence before insertion
* use if-init, remove redundant check
* remove redundant check
* don't remove cmake? fix macOs depends build?
* cppcheck: one per line, alphabetize
* remove duplicate cmake install
