Age | Commit message (Collapse) | Author |
|
24bf17602c620445f76c3b407937751c8a894d37 gui refactor: Inline SplashScreen::ConnectWallet (Russell Yanofsky)
e4f435047121886edb6e6a6c4e4998e44ed2e36a refactor: Move wallet methods out of chain.h and node.h (Russell Yanofsky)
b266b3e0bf29d0f3d5deaeec62d57c5025b35525 refactor: Create interfaces earlier during initialization (Russell Yanofsky)
Pull request description:
Add WalletClient interface so node interface is cleaner and don't need wallet-specific methods.
The new NodeContext::wallet_client pointer will also be needed to eliminate global wallet variables like ::vpwallets in #19101, because createWallet(), loadWallet(), getWallets(), etc methods called by the GUI need a way to get a reference to the list of open wallets if it is no longer a global variable.
ACKs for top commit:
promag:
Code review ACK 24bf17602c620445f76c3b407937751c8a894d37.
MarcoFalke:
ACK 24bf17602c620445f76c3b407937751c8a894d37 š
Tree-SHA512: a70d3776cd6723093db8912028c50075ec5fa0a48b961cb1a945f922658f5363754f8380dbb8378ed128c8c858913024f8264740905b8121a35c0d63bfaed7cf
|
|
variance of result values
3edc4e34fe2f92e7066c1455f5e42af2fdb43b99 bench: Prevent thread oversubscription (Hennadii Stepanov)
ce3e6a7cb21d1aa455513970846e1f70c01472a4 bench: Allow skip benchmark (Hennadii Stepanov)
Pull request description:
Split out from #18710.
Some results (borrowed from #18710):

ACKs for top commit:
fjahr:
Code review ACK 3edc4e34fe2f92e7066c1455f5e42af2fdb43b99
Tree-SHA512: df7413ec9ea326564a8e8de54752c9d1444ff7de34edb03e1e0c2120fc333e4640767fdbe3e87eab6a7b389a4863c02e22ad2ae0dbf139fad6a9b85e00f563b4
|
|
fa0572d0f3b083b4c8e2e883a66e2b198c6779f1 Pass mempool reference to chainstate constructor (MarcoFalke)
Pull request description:
Next step toward #19556
Instead of relying on the mempool global, each chainstate is given a reference to a mempool to keep up to date with the tip (block connections, disconnections, reorgs, ...)
ACKs for top commit:
promag:
Code review ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1.
darosior:
ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1
hebasto:
ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1, reviewed and tested on Linux Mint 20 (x86_64).
Tree-SHA512: 12184d33ae5797438d03efd012a07ba3e4ffa0d817c7a0877743f3d7a7656fe279280c751554fc035ccd0058166153b6c6c308a98b2d6b13998922617ad95c4c
|
|
c4b85ba704a1b5257dc82786a84f676bacb7b027 Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDB (Luke Dashjr)
Pull request description:
Fixes a bug introduced in #19614
The LevelDB-specific fdatasync check was only using `AC_SUBST`, which works for Makefiles, but doesn't define anything for C++. Furthermore, the #define is typically 0 or 1, never undefined.
This fixes both issues by defining it and checking its value instead of whether it is merely defined.
Pulled out of #14501 by fanquake's request
ACKs for top commit:
fanquake:
ACK c4b85ba704a1b5257dc82786a84f676bacb7b027 - thanks for catching and fixing my mistake.
laanwj:
Code review ACK c4b85ba704a1b5257dc82786a84f676bacb7b027
Tree-SHA512: 91d5d426ba000b4f3ee7e2315635e24bbb23ceff16269ddf4f65a63d25fc9e9cf94a3b236eed2f8031cc36ddcf78aeb5916efcb244f415943a8a12f907ede8f9
|
|
GetAddressBalances
b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 wallet, refactor: Remove duplicate map lookups in GetAddressBalances (JoĆ£o Barbosa)
Pull request description:
Now just one lookup in `balances` instead of three.
ACKs for top commit:
achow101:
ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
theStack:
ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
practicalswift:
ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
Tree-SHA512: a73c1b336406a569e3bb10290618c5950b944db58ed0b05ff202d097684bb3ba3a5942c8d30443960052aa16438c054e2d02977b67aa901cce665c4df0ee5602
|
|
|
|
|
|
|
|
|
|
|
|
|
|
This is needed for upcoming commit "sync.h: Make runtime lock checks
require compile-time lock checks" to pass.
|
|
This change prepares for upcoming commit "Do not hide compile-time
thread safety warnings" by replacing AssertLockHeld() with
LockAssertion() where needed.
|
|
handling
ca185cf5a14b16d61814d7172284bc8efcd28b69 doc: Document differences in bitcoind and bitcoin-qt locale handling (practicalswift)
Pull request description:
Document differences in `bitcoind` and `bitcoin-qt` locale handling.
Since this seems to be the root cause to the locale dependency issues we've seen over the years I thought it was worth documenting :)
Note that 1.) `QLocale` (used by Qt), 2.) C locale (used by locale-sensitive C standard library functions/POSIX functions and some parts of the C++ standard library such as `std::to_string`) and 3.) C++ locale (used by the C++ input/output library) are three separate things. This comment is about the perhaps surprising interference with the C locale (2) that takes place as part of the Qt initialization.
ACKs for top commit:
hebasto:
re-ACK ca185cf5a14b16d61814d7172284bc8efcd28b69
Tree-SHA512: e51c32f3072c506b0029a001d8b108125e1acb4f2b6a48a6be721ddadda9da0ae77a9b39ff33f9d9eebabe2244c1db09e8502e3e7012d7a5d40d98e96da0dc44
|
|
|
|
|
|
8e35bf59062b3a823182588e0bf809b3367c2be0 scripted-diff: rename misbehavior members (John Newbery)
1f96d2e673a78220eebf3bbd15b121c51c4cd97b [net processing] Move misbehavior tracking state to Peer (John Newbery)
7cd4159ac834432dadd60a5e8ee817f3cadbee55 [net processing] Add Peer (John Newbery)
aba03359a6e62a376ae44914f609f82a1556fc89 [net processing] Remove CNodeState.name (John Newbery)
Pull request description:
We currently have two structures for per-peer data:
- `CNode` in net, which should just contain connection layer data (eg socket, send/recv buffers, etc), but currently also contains some application layer data (eg tx/block inventory).
- `CNodeState` in net processing, which contains p2p application layer data, but requires cs_main to be locked for access.
This PR adds a third struct `Peer`, which is for p2p application layer data, and doesn't require cs_main. Eventually all application layer data from `CNode` should be moved to `Peer`, and any data that doesn't strictly require cs_main should be moved from `CNodeState` to `Peer` (probably all of `CNodeState` eventually).
`Peer` objects are stored as shared pointers in a net processing global map `g_peer_map`, which is protected by `g_peer_mutex`. To use a `Peer` object, `g_peer_mutex` is locked, a copy of the shared pointer is taken, and the lock is released. Individual members of `Peer` are protected by different mutexes that guard related data. The lifetime of the `Peer` object is managed by the shared_ptr refcount.
This PR adds the `Peer` object and moves the misbehaving data from `CNodeState` to `Peer`. This allows us to immediately remove 15 `LOCK(cs_main)` instances.
For more motivation see #19398
ACKs for top commit:
laanwj:
Code review ACK 8e35bf59062b3a823182588e0bf809b3367c2be0
troygiorshev:
reACK 8e35bf59062b3a823182588e0bf809b3367c2be0 via `git range-diff master 9510938 8e35bf5`
theuni:
ACK 8e35bf59062b3a823182588e0bf809b3367c2be0.
jonatack:
ACK 8e35bf59062b3a823182588e0bf809b3367c2be0 keeping in mind Cory's comment (https://github.com/bitcoin/bitcoin/pull/19607#discussion_r470173964) for the follow-up
Tree-SHA512: ad84a92b78fb34c9f43813ca3dfbc7282c887d55300ea2ce0994d134da3e0c7dbc44d54380e00b13bb75a57c28857ac3236bea9135467075d78026767a19e4b1
|
|
|
|
receiving address' button
4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov)
4227a8e1f3a4f94a5a4cb7adeecf967e14156bde qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov)
Pull request description:
Fix #24
The first commit:
- visual improvement with no behavior change
The second commit:
- removes a bunch of LOCs
- slightly change behavior and makes it standard
With this PR:

ACKs for top commit:
Saibato:
Concept tACK https://github.com/bitcoin-core/gui/pull/39/commits/4227a8e1f3a4f94a5a4cb7adeecf967e14156bde https://github.com/bitcoin-core/gui/pull/39/commits/4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9
promag:
Tested ACK 4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 on macos.
Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
|
|
pre-0.2.9 nodes
7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messages (RaĆŗl MartĆnez (RME))
Pull request description:
The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before.
This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot).
Credits for discovering this go to sipa in https://github.com/bitcoin/bitcoin/pull/19628#discussion_r475907453
Thanks for the attention!
ACKs for top commit:
sipa:
utACK 7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce
vasild:
ACK 7b6d0f1
Tree-SHA512: c5fab59dda2acafe143f607a4c5b636a54ac76fba651cad1ad1b09c94e88ab39503a31c2244c8f2664da68456c2a870c601d8894139c55cde9ece8161913ed2e
|
|
d3e8adfada889a3c9fba930086eda609509aca07 util: remove c-string interfaces for DecodeBase58{Check} (Sebastian Falbesoner)
Pull request description:
This micro-PR gets rid of base58 function interfaces that are redundant in terms of c-string / std::string variants; the c-string interface for `DecodeBase58Check` is completely unused outside the base58 module, while the c-string interface for `DecodeBase58` is only used in unit tests, where an implicit conversion to std::string is not problematic.
ACKs for top commit:
practicalswift:
ACK d3e8adfada889a3c9fba930086eda609509aca07 -- patch looks correct
laanwj:
Code review ACK d3e8adfada889a3c9fba930086eda609509aca07
Tree-SHA512: 006a4a1e23b11385f60820c188b8e6b1634a182ca36e29a6580f72150214c65a3fdb273ec439165f26ba88a42d2bf5bab1cf3666a9eaee222fb4e1c00aeba433
|
|
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
|
|
annotations
9034f6e30ee6d3db71049de4500e7ede648557fb Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR annotations (Hennadii Stepanov)
Pull request description:
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#guarded-var-and-pt-guarded-var:
> `GUARDED_VAR` and `PT_GUARDED_VAR`
> Use of these attributes has been deprecated.
ACKs for top commit:
MarcoFalke:
ACK 9034f6e30ee6d3db71049de4500e7ede648557fb They seem to be deprecated for a long time already https://releases.llvm.org/4.0.0/tools/clang/docs/ThreadSafetyAnalysis.html#guarded-var-and-pt-guarded-var
Tree-SHA512: d86f55fe57c28d91eda4a0ad727e36a5b35ba4b50a557c59b83cf0c5291cc5ad37b6f4ba6daeba3c1aba143faadaea6bb21c723f4d221856d6e6c42d228e8aa2
|
|
The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before.
This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot).
Credits for discovering this go to sipa.
|
|
df536883d263781c2abe944afc85f681cda635ed chain: Remove UB CChain comparison (Carl Dong)
Pull request description:
Comparing two empty `CChain`s is currently undefined behaviour, and resulted in false assertion failures when comparing identical empty `CChain`s in local testing.
Let's just remove this comparison operator since it doesn't seem to be used anywhere.
ACKs for top commit:
practicalswift:
ACK df536883d263781c2abe944afc85f681cda635ed -- patch is guaranteed to be correct :)
MarcoFalke:
cr ACK df536883d263781c2abe944afc85f681cda635ed
Tree-SHA512: db10bac364fc965b56abf7a5bac48018786b14806ffe107e3e8eb24d5004a29331f3387dfe3409a3452a6750d3329e3f354265d787ebb3abfccabe77b28a54d5
|
|
|
|
|
|
|
|
|
|
cc26fab48d76a813d798657b18ae1af08a301150 tests: Add fuzzing harness for CNode (practicalswift)
Pull request description:
Add fuzzing harness for `CNode`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
Top commit has no ACKs.
Tree-SHA512: e6330e5de5b2eb44d3bd91a885e69ebb625bfd1cb2499338aeb3997ff0268848434e651126fe68a8cadd7235c391e61a40d6408ee26e457faf73572e0c375f6b
|
|
It was unused, and had UB
|
|
b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8 gui: Clarify block height label (Hennadii Stepanov)
Pull request description:
Prefer "block height" instead of "number of blocks".
This was done while testing https://github.com/bitcoin/bitcoin/pull/16981.
ACKs for top commit:
michaelfolkson:
ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8. I don't think there are any other obvious examples in the GUI where "block height" should replace "number of blocks" except for translations.
MarcoFalke:
cr ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8
Tree-SHA512: ec3b48c1af5d613ed657ad51f2caddea774376736ecc02343d54518986e35ec37f1745b059814b5be92b5e5c2bb2970d17159b24c6e88b9316803d4de5327c31
|
|
Suggested https://github.com/bitcoin/bitcoin/pull/19099#discussion_r450522201
|
|
Add WalletClient interface so node interface is cleaner and don't need
wallet-specific methods.
The new NodeContext::wallet_client pointer will also be needed to eliminate
global wallet variables like ::vpwallets, because createWallet(), loadWallet(),
getWallets(), etc methods called by the GUI need a way to get a reference to
the list of open wallets if it is no longer a global variable.
Also tweaks splash screen registration for load wallet events to be delayed
until after wallet client is created.
|
|
Add AppInitInterfaces function so wallet chain and chain client interfaces are
created earlier during initialization. This is needed in the next commit to
allow the gui splash screen to be able to register for wallet events through a
dedicated WalletClient interface instead managing wallets indirectly through
the Node interface. This only works if the wallet client interface is created
before the splash screen needs to use it.
|
|
listunspent
a99a3c0bd6d5476503c015e23be569295fdd190c rpc: Validate provided keys for query_options parameter in listunspent (pasta)
Pull request description:
At Dash, one of our developers was working with the `listunspent` RPC command, but instead of saying "minimumAmount" he said "minimmumAmount" as such the RPC wasn't working as expected.
In https://github.com/dashpay/dash/pull/3507 we implemented a check so that `listunspent` returns an error if an unrecognized option is given. I figured I might as well adapt the code and throw up a PR here.
Cheers!
ACKs for top commit:
adaminsky:
ACK `a99a3c0bd`
meshcollider:
Seems fine to me. utACK a99a3c0bd6d5476503c015e23be569295fdd190c
Tree-SHA512: 9fccf14979849879a51b352afa3e1932ce4a6cfc2ee97b8d405ec6e65673fe94e302795e3ec0b440e6d252f13acda620e1f6a0e86c3fa918883c3fb4600a372c
|
|
|
|
b8405b833ad28351c80fb10f6f896f974013fd9e wallet: IsChange requires cs_wallet lock (JoĆ£o Barbosa)
d8441f30ff57e4ae98cff6694c995eaffc19c51a wallet: IsMine overloads require cs_wallet lock (JoĆ£o Barbosa)
a13cafc6c6998baedf3c5766259c21fcd763b99e wallet: GetWalletTx requires cs_wallet lock (JoĆ£o Barbosa)
Pull request description:
This change removes some unlock/lock and lock/lock cases regarding `GetWalletTx` and `IsMine` overloads.
ACKs for top commit:
laanwj:
Code review ACK b8405b833ad28351c80fb10f6f896f974013fd9e
ryanofsky:
Code review ACK b8405b833ad28351c80fb10f6f896f974013fd9e. Just new commit since last review changing IsChange GetChange locks and adding annotations
Tree-SHA512: 40d37c4fe5d10a1407f57d899d5822bb285633d8dbfad8afcf15a9b41b428ed9971a9a7b1aae84318371155132df3002699a15dab56e004527d50c889829187d
|
|
fixes issue #19678 UBSan implicit-integer-sign-change
Credit to Eugene (Crypt-iQ) for finding and reporting the issue
and to Vasil Dimov (vasild) for the original suggestion
|
|
|
|
|
|
This change decreases the variance of benchmark results.
|
|
and otherwise log that an unknown INV type was received.
In INV processing, when handling transaction type inv messages,
ToGenTxid() expects that we constructed the CInv ourselves or
that we verified that it is for a transaction type CInv.
Therefore, change this `else` branch into an `else if (inv.GenMsgTx())`
to make this safer and log any INVs that fall through.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The nFetchFlags code can be removed here because GetFetchFlags() can only add
the MSG_WITNESS_FLAG, which is added to the CInv::type field. That CInv is only
passed to AlreadyHave() or ToGenTxid(), and neither of those functions do
anything different depending on whether the CInv type is MSG_TX or
MSG_WITNESS_TX.
Co-authored by: John Newbery <john@johnnewbery.com>
|