| Age | Commit message (Collapse) | Author |
|---|
|
Instead of setting the long term feerate for each SelectCoinsMinConf
iteration, set it once during CreateTransaction and let it be shared
with each SelectCoinsMinConf through
coin_selection_params.m_long_term_feerate.
Does not change behavior.
|
|
Make sure that all fee calculations use the same feerate.
coin_selection_params.effective_fee is the variable we use for all fee
calculations, so get rid of remaining nFeeRateNeeded usages and just
directly set coin_selection_params.effective_fee.
Does not change behavior.
|
|
During each loop of CreateTransaction, instead of constantly getting a
new feerate, use the feerate that we have already fetched for all
fee calculations. Thix fixes a race condition where the feerate required
changes during each iteration of the loop.
This commit changes behavior as the "Fee estimation failed" error will
now take priority over "Signing transaction failed".
|
|
590bda79e876d9b959083105b8c7c41dd87706eb scripted-diff: Remove setup_clean_chain if default is not changed (Fabian Jahr)
98892f39e3d079c73bff7f2a5d5420fa95270497 doc: Improve setup_clean_chain documentation (Fabian Jahr)
Pull request description:
The first commit improves documentation on setup_clean_chain which is misunderstood quite frequently. Most importantly it fixes the TestShell docs which are simply incorrect.
The second commit removes the instances of `setup_clean_clain` in functional tests where it is not changing the default.
This used to be part of #19168 which also sought to rename`setup_clean_chain`.
ACKs for top commit:
jonatack:
ACK 590bda79e876d9b959083105b8c7c41dd87706eb
Tree-SHA512: a7881186e65d31160b8f84107fb185973b37c6e50f190a85c6e2906a13a7472bb4efa9440bd37fe0a9ac5cd2d1e8559870a7e4380632d9a249eca8980b945f3e
|
|
20677ffa22e93e7408daadbd15d433f1e42faa86 validation: Guard all chainstates with cs_main (Carl Dong)
Pull request description:
```
This avoids a potential race-condition where a thread is reading the
ChainstateManager::m_active_chainstate pointer while another one is
writing to it. There is no portable guarantee that reading/writing the
pointer is thread-safe.
This is also done in way that mimics ::ChainstateActive(), so the
transition from that function to this method is easy.
More discussion:
1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027
2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961
3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522
4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695
```
Basically this PR removes the loaded-but-unfired footgun, which:
- Is multiplied (but still unshot) in the chainman deglobalization PRs (#20158)
- Is shot in the test framework in the au.activate PR (#19806)
ACKs for top commit:
jnewbery:
code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. I've verified by eye that neither of these members are accessed without cs_main.
ryanofsky:
Code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. It is safer to have these new `GUARDED_BY` annotations and locks than not to have them, but in the longer run I think every `LOCK(cs_main)` added here and added earlier in f92dc6557a153b390a1ae1d0808ff7ed5d02c66e from #20749 should be removed and replaced with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` on the accessor methods instead. `cs_main` is a high level lock that should be explicitly acquired at a high level to prevent the chain state from changing. It shouldn't be acquired recursively in low-level methods just to read pointer values atomically.
Tree-SHA512: 68a3a46d79a407b774eab77e1d682a97e95f1672db0a5fcb877572e188bec09f3a7b47c5d0cc1f2769ea276896dcbe97cb35c861acf7d8e3e513e955dc773f89
|
|
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke)
fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)
Pull request description:
This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937
Fixes: #20902
ACKs for top commit:
ajtowns:
ACK fa61b9d1a68820758f9540653920deaeae6abe79
fjahr:
Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79
Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
|
|
51f3752fbeee09d025db33e154bb2efff9e20837 Add release notes for listdescriptors RPC (Ivan Metlushko)
Pull request description:
Original PR is #20226
ACKs for top commit:
jonatack:
ACK 51f3752
jonasschnelli:
ACK 51f3752fbeee09d025db33e154bb2efff9e20837
Tree-SHA512: e8091d01b99a3effcd6c1738e7363a44858ba9bcf6bd99bf60f2025a25db83fc8d61354ab2002365b56071b9f3693c7d534153a259b5ebc91cbcf8d13f6555f1
|
|
f22a3ec1403293f45a06558d105f0da624c5ebed build: make macOS HOST in download-osx generic (fanquake)
Pull request description:
This was missed in #20419, and the update before that, so just make this non-versioned so that we don't have to worry about it. This is fine, because it's just for downloading sources.
ACKs for top commit:
RandyMcMillan:
ACK f22a3ec1403293f45a06558d105f0da624c5ebed
dongcarl:
utACK f22a3ec1403293f45a06558d105f0da624c5ebed
Tree-SHA512: 3a6993a69594a793a5185e4ba48858443a1002a37b96ff881d39ca7719c79432b35d709bd9a9379f8046bdbeb716c5e1598f273a7e7e3f3bf528b6a807abe5ec
|
|
572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 doc: More precise -debug and -debugexclude doc (wodry)
Pull request description:
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.
This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
ACKs for top commit:
laanwj:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
MarcoFalke:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
theStack:
ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0
Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
|
|
This was missed in #20419, and the update before that, so just
make this un-versioned so that we don't have to worry about it.
This is fine, because it's just for downloading sources.
|
|
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev)
381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev)
e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev)
4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev)
f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev)
dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev)
Pull request description:
This PR introduces per-peer message capture into Bitcoin Core. 📓
## Purpose
The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?".
## Functionality
When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue.
The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so:
```
message_capture/203.0.113.7:56072/msgs_recv.dat
message_capture/203.0.113.7:56072/msgs_sent.dat
```
Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option.
## Future Maintenance
I sympathize greatly with anyone who says "the best code is no code".
The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool.
Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small.
## FAQ
"Why not just use Wireshark"
Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark.
Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done.
Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable.
ACKs for top commit:
MarcoFalke:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚
jnewbery:
utACK bff7c66e67aa2f18ef70139338643656a54444fe
theStack:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe
Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
|
|
e99db77a6e73996d33d7108f8336938dd57037a7 Drop boost/preprocessor dependencies (Hennadii Stepanov)
12f5028d4957bce3ba176e80527894b497c04a3b refactor: Move STRINGIZE macro to macros.h (Hennadii Stepanov)
Pull request description:
Use own macros instead of boost's ones.
ACKs for top commit:
laanwj:
Code review ACK e99db77a6e73996d33d7108f8336938dd57037a7
practicalswift:
cr ACK e99db77a6e73996d33d7108f8336938dd57037a7
Tree-SHA512: 7ec15c2780a661e293c990f64c41b5b451d894cc191aa7872fbcaf96da91915a351209b1f1003ab12a7a16cb464e50ac58a028db02beeedfa5f6931752c2d9e2
|
|
Original PR is #20226
|
|
Since these chainstates are:
1. Also vulnerable to the race condition described in the previous
commit
2. Documented as having similar semantics as m_active_chainstate
we should also protect them with ::cs_main.
|
|
b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb build: Add -Werror=mismatched-tags (Hennadii Stepanov)
1485124291368c4a2ca8ea09c18e813f1dbabf5c Fix -Wmismatched-tags warnings (Hennadii Stepanov)
Pull request description:
Warnings were introduced in #20749:
```
./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
class CCheckpointData;
^
./chainparams.h:24:8: note: previous use is here
struct CCheckpointData {
^
./validation.h:43:1: note: did you mean struct here?
class CCheckpointData;
^~~~~
struct
1 warning generated.
```
This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435
ACKs for top commit:
glozow:
utACK https://github.com/bitcoin/bitcoin/pull/21051/commits/b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb 🚗
practicalswift:
cr ACK b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb: patch looks correct
Tree-SHA512: 3ac887ebdbf9a1ae33c1fd5381b3b8d83388ad557ddeb55013acd42bb9752a5bd009e3a0eed52644a023a7a0dda1c159277981af82f58fb0abfe60b84e01bf29
|
|
-BEGIN VERIFY SCRIPT-
git grep -l "self.setup_clean_chain = False" test/functional/*.py | xargs sed -i "/self.setup_clean_chain = False/d";
-END VERIFY SCRIPT-
|
|
|
|
|
|
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.
This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
|
|
|
|
This is a move-only change.
|
|
fa292724598c273867bc6dbf311f1440fe2541ba Remove redundant MakeUCharSpan wrappers (MarcoFalke)
faf4aa2f47c0de4f3a0c5f5fe5b3ec32f611eefd Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke)
fada14b948cac147198e3b685b5dd8cb72dc2911 Treat CDataStream bytes as uint8_t (MarcoFalke)
fa8bdb048e65cae2d26bea3f991717a856e2fb39 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke)
faa96f841fe45bc49ebb6e07ac82a129fa9c40bf Remove unused CDataStream methods (MarcoFalke)
Pull request description:
Using `uint8_t` for raw bytes has a style benefit:
* The signedness is clear from reading the code, as it does not depend on the architecture
Other clean-ups in this pull include:
* Remove unused methods
* Constructor is simplified with `Span`
* Remove `Init()` member in favor of C++11 member initialization
ACKs for top commit:
laanwj:
code review ACK fa292724598c273867bc6dbf311f1440fe2541ba
theStack:
ACK fa292724598c273867bc6dbf311f1440fe2541ba 🍾
Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
|
|
cc30dfbd4bab5fe4acc82dca9f6979e5a3a41e32 doc: Document use of make-tag script to make tags (Wladimir J. van der Laan)
Pull request description:
To make release tags the `make-tag.py` script from the maintainer tools should be used. This ensures that all the various occurrences of the version in different files match the tagged version before proceeding. And move it into a separate section with the other per-release actions.
Also replace other "ping wumpus" references.
ACKs for top commit:
jonatack:
ACK cc30dfbd4bab5fe4acc82dca9f6979e5a3a41e32
Tree-SHA512: c09748a0bea85573b3f04fdb86430a53b683ff4d956edc1f49d471e2e526715cbde7cf6ad83a43a1a3fca4ff3c5af011e3d1b8cb61f5d8e065cfa71ba0138c88
|
|
|
|
dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f refactor: remove boost::thread_group usage (fanquake)
Pull request description:
Post #18710, there isn't much left using `boost::thread_group`, so should just be able to replace it with the standard library. This also removes the last use of `boost::thread_interrupted`.
After this change, last piece of Boost Thread we'd be using is `boost::shared_mutex`. See the commentary [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) as to why it may be non-trivial to swap that for `std::shared_mutex` in the near future.
Closes #17307
ACKs for top commit:
laanwj:
Code review re-ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f
MarcoFalke:
review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f 🔁
jonatack:
Non-expert code review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f, also checked range-diff since last review and that local debug build is clean with gcc 10.2.1-6 on Debian
Tree-SHA512: 5510e2d760cce824234207dc86b1551ca8f21cbf3a2ce753c0254a0d03ffd83c94e449aec202fb7bd76e6fc64df783a6b70a736b0add9ece3734bb9c8ce8fc2f
|
|
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong)
b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong)
0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong)
ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong)
e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong)
5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong)
2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong)
9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong)
0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong)
d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong)
f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong)
e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong)
b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong)
3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong)
eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong)
15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong)
f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
jnewbery:
utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
laanwj:
re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7
ryanofsky:
Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review:
Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
|
|
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
|
|
c943282b5e6312537f885c811d43120ce2f5b766 validation: remove redundant check on pindex (jarolrod)
Pull request description:
This removes a redundant check on `pindex` being a `nullptr`. By the time we get to this step `pindex` is always a `nullptr` as the branch where it has been set would have already returned.
Closes #19223
ACKs for top commit:
Zero-1729:
re-ACK c943282
ajtowns:
ACK c943282b5e6312537f885c811d43120ce2f5b766 - code review only
MarcoFalke:
review ACK c943282b5e6312537f885c811d43120ce2f5b766 📨
theStack:
re-ACK c943282b5e6312537f885c811d43120ce2f5b766
Tree-SHA512: d2dc58206be61d2897b0703ee93af67abed492a80e84ea03dcfbf7116833173da3bdafa18ff80422d5296729d5254d57cc1db03fdaf817c8f57c62c3abef673c
|
|
eligibility on grouping
5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e Rewrite OutputGroups to be clearer and to use scriptPubKeys (Andrew Chow)
f6b305273910db0e46798d361413a7e878cb45f7 Explicitly filter out partial groups when we don't want them (Andrew Chow)
416d74fb1687ae1d47a58c153d09d9afe0b6dc60 Move OutputGroup positive only filtering into Insert (Andrew Chow)
d895e98b594b873f3d34c8ba63e9b55125d51b5a Move EligibleForSpending into GroupOutputs (Andrew Chow)
99b399aba5d27476b61b4865cc39553d03965d57 Move fee setting of OutputGroup to Insert (Andrew Chow)
6148a8acda5e594bb9b3b2d989056f9e03ddbdbd Move GroupOutputs into SelectCoinsMinConf (Andrew Chow)
2acad036575ec998f8bbe4f10f6206b1c8ad3d23 Remove OutputGroup non-default constructors (Andrew Chow)
Pull request description:
Even after #17458, we still deal with setting fees of an `OutputGroup` and filtering the `OutputGroup` outside of the struct. We currently make all of the `OutputGroup`s in `SelectCoins` and then copy and modify them within each `SelectCoinsMinConf` scenario. This PR changes this to constructing the `OutputGroup`s within the `SelectCoinsMinConf` so that the scenario can be taken into account during the group construction. Furthermore, setting of fees and filtering for effective value is moved into `OutputGroup::Insert` itself so that we don't add undesirable outputs to an `OutputGroup` rather than deleting them afterwards.
To facilitate fee calculation and effective value filtering during `OutputGroup::Insert`, `OutputGroup` now takes the feerates in its constructor and computes the fees and effective value for each output during `Insert`.
While removing `OutputGroup`s in accordance with the `CoinEligibilityFilter` still requires creating the `OutputGroup`s first, we can do that within the function that makes them - `GroupOutput`s.
ACKs for top commit:
Xekyo:
Code review ACK: https://github.com/bitcoin/bitcoin/pull/20040/commits/5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
fjahr:
Code review ACK 5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
meshcollider:
Light utACK 5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
Tree-SHA512: 35965b6d49a87f4ebb366ec4f00aafaaf78e9282481ae2c9682b515a3a9f2cbcd3cd6e202fee29489d48fe7f3a7cede4270796f5e72bbaff76da647138fb3059
|
|
5353b0c64d32e44fc411464e080d4b00fae7124e Change type definitions for "chain" and "setup_clean_chain" from type comments to Python 3.6+ types. Additionally, set type for "nodes". (Kiminuo)
Pull request description:
### Motivation
When I wanted to understand better https://github.com/bitcoin/bitcoin/pull/19145/files#diff-4bebbd3b112dc222ea7e75ef051838ceffcee63b9e9234a98a4cc7251d34451b test, I noticed that navigation in PyCharm/VS Code did not work for `nodes` variable. I think this is frustrating, especially for newcomers.
### Summary
* This PR modifies Python 3.5 [type comments](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#variables) to Python 3.6+ types and adds a proper type for `nodes` [instance attribute](https://mypy.readthedocs.io/en/stable/class_basics.html#instance-and-class-attributes).
* This PR does not change behavior.
* This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.
### End result
1. Open `test/functional/feature_abortnode.py`
2. Move your caret to: `self.nodes[0].generate[caret here](3)`
3. Use "Go to definition" [F12] should work now.
I have tested this on PyCharm (Windows, Ubuntu) and VS Code (Windows, Ubuntu).
Note: Some `TestNode` methods (e.g. `self.nodes[0].getblock(...)` ) use `__call__` mechanism and navigation does not work for them even with this PR.
ACKs for top commit:
laanwj:
ACK 5353b0c64d32e44fc411464e080d4b00fae7124e
theStack:
ACK 5353b0c64d32e44fc411464e080d4b00fae7124e
Tree-SHA512: 821773f052ab9b2889dc357d38c59407a4af09e3b86d7134fcca7d78e5edf3a5ede9bfb37595ea97caf9ebfcbda372bcf73763b7f89b0677670f21b3e396a12b
|
|
dee2d6fbf9008d0e0667b3744d847192be6ef6e0 fuzz: Avoid designated initialization (C++20) in fuzz tests (practicalswift)
Pull request description:
Avoid designated initialization (C++20) in fuzz tests.
Context: https://github.com/bitcoin/bitcoin/pull/20197#discussion_r565270556, https://github.com/bitcoin/bitcoin/pull/20936#discussion_r566708730
ACKs for top commit:
MarcoFalke:
cr ACK dee2d6fbf9008d0e0667b3744d847192be6ef6e0
dhruv:
code review ACK dee2d6fbf9008d0e0667b3744d847192be6ef6e0
ajtowns:
utACK dee2d6fbf9008d0e0667b3744d847192be6ef6e0
Tree-SHA512: 5940fab6e97a2b11dd3b1475d2cffa2840dc2e6ec34bd9f9df90f948709cab98fd1c513d5dd104816d33a525a6e9710b8715b02db941e35d84f92bc211f56d1d
|
|
|
|
python rewrite
c86b9a65eb0d6d1e659415880702c4dc889c34e6 contrib: remove verify.sh (Sebastian Falbesoner)
c84838e7afb7b084a56a75e98325563b6de83124 contrib: binary verification script verify.sh rewritten in python (Sebastian Falbesoner)
Pull request description:
The rationale for the PR is the same as for #18132:
> Most of our test scripts are written in python. We don't have enough reviewers for bash scripts and they tend to be clumsy anyway. Especially when it comes to argument parsing.
Note that there are still a lot of things that could be improved in this replacement (e.g. using regexps for version string parsing, adding type annotations, dividing up into more functions, getting a pylint score closer to 10, etc.), but I found the original shell script quite hard to read, so it's possibly still a good first step for an improvement.
~Not sure though if it's worth the reviewers time, and if it's even continued to be used long-term (maybe there are plans to merge it with `get_previous_releases.py`, which partly does the same?), so chasing for Concept ACKs right now.~
ACKs for top commit:
laanwj:
Tested and code review ACK c86b9a65eb0d6d1e659415880702c4dc889c34e6
Tree-SHA512: f7949eead4ef7e5913fe273923ae5c5299408db485146cf996cdf6f8ad8c0ee4f4b30bb6b08a5964000d97b2ae2e7a1bdc88d11c613c16d2d135d80b444e3b16
|
|
To make release tags the `make-tag.py` script from the maintainer tools
should be used. This ensures that all the various occurences of the
version in different files match the tagged version before proceeding.
Also replace other "ping wumpus" references.
|
|
|
|
48c8a9b96453ca429b38fc5d5181a310ae5a93bf net_processing: log txrelay flag from version message (Anthony Towns)
98fab37ca0517bfe58296e47266cd5bd112e90bf net: use peer=N instead of from=N in debug log (Anthony Towns)
12302105bb0bf14721e91b7a3a9d1bf83c8d4154 net_processing: additional debug logging for ignored messages (Anthony Towns)
f7edea3b7c873d6c9bcd50cf528349ef84961a75 net: make debug logging conditional on -debug=net (Anthony Towns)
a410ae8cb09f1b809755316566f9e6bccd41c0c4 net, net_processing: log disconnect reasons with -debug=net (Anthony Towns)
Pull request description:
A few changes to -debug=net logging:
* always log when disconnecting a peer
* only log various connection errors when -debug=net is enabled, since errors from random untrusted peers is completely expected
* log when ignoring a message due to violating protocol (primarily to make it easier to debug other implementations)
* use "peer=123" rather than "from 123" to make grepping logs a bit easier
* log the value of the bip-37 `fRelay` field in version messages both when sending and receiving a version message
ACKs for top commit:
jnewbery:
ACK 48c8a9b96453ca429b38fc5d5181a310ae5a93bf
MarcoFalke:
re-ACK 48c8a9b96453ca429b38fc5d5181a310ae5a93bf only change is rebase 🚓
practicalswift:
re-ACK 48c8a9b96453ca429b38fc5d5181a310ae5a93bf
Tree-SHA512: 6ac530d883dffc4fd7fe20b1dc5ebb5394374c9b499aa7a253eb4a3a660d8901edd72e5ad21ce4a2bf71df25e8f142087755f9756f3497f564ef453a7e9246c1
|
|
faff3991a9be0ea7be31685fb46d94c212c5da34 ci: Fuzz with integer sanitizer (MarcoFalke)
Pull request description:
Otherwise the suppressions file will go out of sync
ACKs for top commit:
practicalswift:
cr ACK faff3991a9be0ea7be31685fb46d94c212c5da34: patch looks correct
Tree-SHA512: 349216d071a2c5ccf24565fe0c52d7a570ec148d515d085616a284f1ab9992ce10ff82eb17962dddbcda765bbd3a9b15e8b25f34bdbed99fc36922d4161d307c
|
|
5d1f260713f9f1d29c0db68f2917dfe7368d4ba0 Improve gui/src/qt README.md (Jarol Rodriguez)
Pull request description:
**Master/Before:** [Render of Master](https://github.com/bitcoin-core/gui/blob/master/src/qt/README.md)
**PR/After:** [Render of PR](https://github.com/bitcoin-core/gui/blob/5d1f260713f9f1d29c0db68f2917dfe7368d4ba0/src/qt/README.md)
**Changes:**
The README.md found in `gui/src/qt` seems to not have gotten any love in a while. This PR fixes some grammatical errors, makes it easier to follow, and modernizes the logic of using Qt Creator.
1. Makes several sections more informative
2. Directories under `Files and Directories` now end with a forward slash denoting that they are a directory
3. Modernize the Qt Creator Logic for the current setup flow
4. Add UNIX Qt Creator Setup Instructions (Ubuntu & Debian)
ACKs for top commit:
RandyMcMillan:
ACK 5d1f260 👍🏼
jonatack:
ACK 5d1f260713f9f1d29c0db68f2917dfe7368d4ba0
Tree-SHA512: bd5cc24b95460f34a1efa489a6acc10d7632c84eabdcdc5729ef077d8303cf037ed664aae033af8883252433ea0999d8ec7d92e8b03d03a873d32b041a94f813
|
|
71430aec4304f71f8a11e6f0fea486e41fe3a9e3 bitcoin-cli: Correct docs (no "generatenewaddress" exists) (Luke Dashjr)
Pull request description:
ACKs for top commit:
jonatack:
ACK 71430aec4304f71f8a11e6f0fea486e41fe3a9e3
Tree-SHA512: 45ee7a51080598141fac4446563bdf99a59bfaa0ee00d9e092511087a968e8535d9208683ed589be1c975531be6d0319d33720f1f0dc1c3635c7d5ea6d726a41
|
|
|
|
543bf745d38ca2f9f7f9f49483772d51154b93a7 gitian-linux: Extend noexec-stack workaround to powerpc (Wladimir J. van der Laan)
00f67c8aa1b8f596f945db30cdc00d54c6e34665 gitian-linux: Build binaries for 64-bit POWER (Luke Dashjr)
63fc2b1782508e750a9254f72b9b8379573a836c gitian: Properly quote arguments in wrappers (Luke Dashjr)
798bc0b29a4ad342010f7cd31dd38eeeb5b709db Support glibc-back-compat on 64-bit POWER (Luke Dashjr)
Pull request description:
Rebase of #14066 by luke-jr.
Let's try to get PowerPC support in in the beginning of the 22.0 cycle so that it gets some testing, and is not a last-minute decision this time, like for last … 2 or 3 major versions.
The symbol/security tooling-related changes have been dropped since they were part of #20434.
Top commit has no ACKs.
Tree-SHA512: df0f8cd320c90f359f8b512c5cb8b59bb277516b57a05482cc8923c656106513b7428e315aaa8ab53e0bd6f80556b07d3639c47f6d9913bcfbfe388b39ef47c4
|
|
|
|
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
LoadExternalBlockFile mainly acts on CChainState.
|
|
Instead use CChainState::ActivateBestChain, which is what the global one
calls anyway.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
[META] This commit should be followed up by removing the comments and
assertions meant only to show that the change is correct.
|
|
|
