Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 ryanofsky commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1111053194)
re: https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107231839

Agree new behavior is better than old behavior:

- Having absolute path of the file that could not be loaded provides more information to users
- Internally, new code avoids confusing side effect of `GetConfigFile` actually creating the datadir directory instead of just returning a path string
💬 ryanofsky commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1110294446)
In commit "util: add ArgsManager datadir helper functions" (f9ee6df4c08a8ef63900359b6c3564db1d4f5073)

Maybe add comment about why this is creating wallet directories. Could say something like:

"wallets" subdirectories are created in all new datadirs, because wallet code will create new wallets in the "wallets" subdirectory only if exists already, otherwise it will create them in the top-level datadir where they could interfere with other files. Wallet init code currently avoids creating "
...
👍 ryanofsky approved a pull request: "Convert ArgsManager::GetDataDir to a read-only function"
(https://github.com/bitcoin/bitcoin/pull/27073)
Light code review ACK 64b812e9a6df8fb4ec96b978a08dc9bde2ada2f0. I want to look a little more closely at this and test a little more to be sure it works as expecte, but this seems like a very nice change. Avoids confusing side effect of functions like GetDataDirNet, AbsPathForConfigVal, and GetConfigFile creating directories in the background.
💬 Riahiamirreza commented on issue "Improve porting documentation for legacy-only wallet RPCs":
(https://github.com/bitcoin/bitcoin/issues/25363#issuecomment-1435744687)
How can I update error messages for one of the remaining rpc's error messages?
Do they still need improvement?
💬 furszy commented on issue "Improve porting documentation for legacy-only wallet RPCs":
(https://github.com/bitcoin/bitcoin/issues/25363#issuecomment-1435756155)
@Riahiamirreza seems that #25680 aims to fix this issue. Maybe you could go there and help reviewing.
And to #27034 if you like the `importaddress` compatibility idea.
👍 ponury1990 approved a pull request: "rpc: make importaddress compatible with descriptors wallet"
(https://github.com/bitcoin/bitcoin/pull/27034)
💬 llazzaro commented on pull request "lint: enable E722 do not use bare except":
(https://github.com/bitcoin/bitcoin/pull/25867#issuecomment-1435779314)
z@MarcoFalke @aureleoules new commit [61bb4e7](https://github.com/bitcoin/bitcoin/commit/61bb4e783b3acc62b121a228f6b14c2462e23315)
👋 KolbyML's pull request is ready for review: "Add Import to Wallet GUI"
(https://github.com/bitcoin-core/gui/pull/650)
📝 TheCharlatan opened a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125)
The libbitcoin_kernel library should not rely on the `ArgsManager`, but rather use option structs that can be passed to the various classes it uses. This PR removes reliance on the `ArgsManager` from the `blockstorage.*` files. Like similar prior work, it introduces an options struct to the `BlockManager` that can be populated with `ArgsManager` values.

Some related prior work: https://github.com/bitcoin/bitcoin/pull/26889 https://github.com/bitcoin/bitcoin/pull/25862 https://github.com/bitco
...
🚀 fanquake merged a pull request: "refactor: remove windows-only compat.h usage in random"
(https://github.com/bitcoin/bitcoin/pull/26814)
💬 fanquake commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1435999830)
```bash
validation.cpp:80:13: error: using decl 'ReadBlockFromDisk' is unused [misc-unused-using-decls,-warnings-as-errors]
using node::ReadBlockFromDisk;
^
/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/validation.cpp:80:13: note: remove the using
using node::ReadBlockFromDisk;
```
:lock: fanquake locked an issue: "Received BTC not showing in ( btc core ) address nor private key"
(https://github.com/bitcoin/bitcoin/issues/27115)
:lock: fanquake locked an issue: "Why is Bitcoin Core structured as a wallet? I know that it began as a wallet, but is it possible to make it like a GUI block explorer instead?"
(https://github.com/bitcoin/bitcoin/issues/27110)
:lock: fanquake locked an issue: "bitcoin failed to build with "EXEC : error : invalid triplet: arm64ec-windows-static " with MSVC on Windows arm64ec"
(https://github.com/bitcoin/bitcoin/issues/27059)
📝 fanquake locked a pull request: "Blockstorage: Dont access gArgs to get blocks_dir"
(https://github.com/bitcoin/bitcoin/pull/27103)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 fanquake locked a pull request: "Fix minor typo"
(https://github.com/bitcoin/bitcoin/pull/27102)
<!--
Just a small typo fix
-->
📝 fanquake locked a pull request: "Check for the awk script before executing it"
(https://github.com/bitcoin/bitcoin/pull/27095)
Hi ! I am new to the Bitcoin contributions. I was taking a look at the possible TODOs and I found a TODO in the file `ci/retry/retry` where, awk script was being used and we need to check if it already exist or not. This patch completes the TODO and would possibly be my very first contribution to the Bitcoin Protocol.

Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear
...
📝 fanquake locked a pull request: "correct and error on the read"
(https://github.com/bitcoin/bitcoin/pull/27089)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
📝 fanquake locked a pull request: "ci: Improve `ccache` cache hit rates"
(https://github.com/bitcoin/bitcoin/pull/27083)
Changes that are not affecting binaries, e.g., #27070 or #27072, are expected to show up to 100% `ccache` hit rate. But that is not the case:
- https://cirrus-ci.com/build/4720733276864512
- https://cirrus-ci.com/build/5915469295648768

Zero hit rate in the "macOS 10.15" is well [known](https://github.com/bitcoin/bitcoin/issues/21552). But a [fix](https://github.com/bitcoin/bitcoin/pull/24620) was [considered](https://github.com/bitcoin/bitcoin/pull/24620#issuecomment-1100850691) not "a good
...
📝 fanquake locked a pull request: "mempool: Increase mempool default size"
(https://github.com/bitcoin/bitcoin/pull/27079)
> Bitcoin Core 0.12 will have a strict maximum size on the mempool. The
default value is 300 MB and can be configured with the `-maxmempool`
parameter. Whenever a transaction would cause the mempool to exceed
its maximum size, the transaction that (along with in-mempool descendants) has
the lowest total feerate (as a package) will be evicted and the node's effective
minimum relay feerate will be increased to match this feerate plus the initial
minimum relay feerate. The initial minimum rel
...