Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 karask commented on issue "Cannot disable RBF with walletrbf configuration option":
(https://github.com/bitcoin/bitcoin/issues/27532#issuecomment-1523304198)
Ah, missed that. Thanks!
karask closed an issue: "Cannot disable RBF with walletrbf configuration option"
(https://github.com/bitcoin/bitcoin/issues/27532)
💬 ryanofsky commented on issue "CI failure "buster-backports InRelease: The following signatures couldn't be verified because the public key is not available"":
(https://github.com/bitcoin/bitcoin/issues/27531#issuecomment-1523323865)
Thanks. The thing I don't understand is why rebasing fixes the problem when rerunning the task doesn't fix the problem.

This failure doesn't seem to have anything to do with the contents of the PR, so I wouldn't expect pushing a rebased version of the PR to change anything. How is rebasing actually different than rerunning? Does a github push clear some cirrus cache that would otherwise not be cleared? I think I've been confused about this behavior in other contexts so would appreciate any po
...
👋 glozow's pull request is ready for review: "mempool: keep CPFP'd transactions when loading from mempool.dat"
(https://github.com/bitcoin/bitcoin/pull/27476)
achow101 closed an issue: "gettransaction does not contain the field "abandoned" for abandoned receiving tx"
(https://github.com/bitcoin/bitcoin/issues/25130)
🚀 achow101 merged a pull request: "rpc, wallet: add abandoned field for all categories of transaction in ListTransaction"
(https://github.com/bitcoin/bitcoin/pull/25158)
💬 achow101 commented on pull request "rpc: Return block hash & height in getbalances, gettransaction and getwalletinfo":
(https://github.com/bitcoin/bitcoin/pull/26094#issuecomment-1523381226)
There's a test failure in `wallet_orphanedreward.py`.

```
256/263 - wallet_orphanedreward.py failed, Duration: 17 s

stdout:
2023-04-26T12:57:28.311000Z TestFramework (INFO): PRNG seed is: 5570008732445289618
2023-04-26T12:57:28.313000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20230426_085557/wallet_orphanedreward_19
2023-04-26T12:57:40.283000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/andy/bitcoin/bitcoin/m
...
💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1523392935)
Push to 60cd07b781bfe46262a5d66c97e257e0dd378f5c:
- rebase on master
- add PR release note
- change how we detect "old" directory use from looking for `blocks/` to looking for `chainstate/`
💬 MarcoFalke commented on issue "CI failure "buster-backports InRelease: The following signatures couldn't be verified because the public key is not available"":
(https://github.com/bitcoin/bitcoin/issues/27531#issuecomment-1523395787)
Yeah, in Cirrus there is a setting to merge the cirrus.yaml of the pull request with the one in the target branch. It is enabled, however it doesn't merge the git contents. Thus, there is a workaround step to achieve that: https://github.com/bitcoin/bitcoin/blob/91ccb62faab21b2b52b089cc04f3a5c1bf6989cc/.cirrus.yml#L27

However, the workaround does not work for Cirrus Docker Env files. Ideally this is fixed by Cirrus CI. I don't see another workaround than either rebasing or removing the Env fe
...
💬 willcl-ark commented on issue "Relative paths named in the -conf parameter reset when parsing datadir in named config":
(https://github.com/bitcoin/bitcoin/issues/19990#issuecomment-1523439189)
Will this issue be closed by https://github.com/bitcoin/bitcoin/pull/27302 ?
💬 MarcoFalke commented on pull request "Remove now-unnecessary poll, fcntl includes from net(base).cpp":
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1523481640)
Can you add this to iwyu CI to double check and aid reviewers?
💬 MarcoFalke commented on pull request "test: add coverage for rpc error when trying to rescan beyond pruned data":
(https://github.com/bitcoin/bitcoin/pull/25937#issuecomment-1523489631)
lgtm ACK cca4f82b828669ae23f6ac64fb83e068b81ae189

See also https://marcofalke.github.io/b-c-cov/total.coverage/src/wallet/rpc/transactions.cpp.gcov.html
🤔 brunoerg reviewed a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511#pullrequestreview-1402102237)
Concept ACK
💬 MarcoFalke commented on pull request "script: remove unused bitwise `CScriptNum` operators":
(https://github.com/bitcoin/bitcoin/pull/27096#issuecomment-1523501332)
Idk, this is probably too hard to review to be worth it. You are removing an overload, so reviewers need to check how this interacts with overload resolution. Maybe just close and reopen/mention it the next time the code is touched an reviewers are primed on it?
💬 MarcoFalke commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1523503942)
Maybe even with gcc 13.1, now that it is about to be released?
💬 MarcoFalke commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-1523507720)
On Windows CI this will eat the bench CPU without terminating?
💬 MarcoFalke commented on pull request "test: test banlist database recreation":
(https://github.com/bitcoin/bitcoin/pull/26794#issuecomment-1523512721)
lgtm ACK 4bdcf571584cbe44ad5533a3c991d3b0b4b2c84f
💬 MarcoFalke commented on pull request "Move `CopyrightHolders()` and `LicenseInfo()` into `libbitcoin_common`":
(https://github.com/bitcoin/bitcoin/pull/26688#issuecomment-1523517315)
I think this is a bugfix/requirement for https://github.com/bitcoin/bitcoin/pull/26504, so I am not sure why it was closed but the other not?
💬 ryanofsky commented on issue "Relative paths named in the -conf parameter reset when parsing datadir in named config":
(https://github.com/bitcoin/bitcoin/issues/19990#issuecomment-1523524847)
> Will this issue be closed by #27302 ?

Yes specifically it is fixed by the third commit 0319de5cbedd1a8f8766cfec61151c58b3fb27ef from #27302. That commit changes the `ArgsManager::GetConfigFilePath` definition so the "The specified config file %s does not exist" warning is no longer triggered:

https://github.com/bitcoin/bitcoin/blob/91ccb62faab21b2b52b089cc04f3a5c1bf6989cc/src/init/common.cpp#L125-L130
👍 brunoerg approved a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511#pullrequestreview-1402136223)
tACK c505611f08a850acb97dea9cc03b36aa468929ca

just tested it with my "clearnet only" node, results:
```sh
➜ bitcoin-core-dev git:(27511-stratospher) ✗ ./src/bitcoin-cli getaddrmaninfo
{
"ipv4": {
"new": 28765,
"tried": 277,
"total": 29042
},
"ipv6": {
"new": 7245,
"tried": 67,
"total": 7312
},
"onion": {
"new": 0,
"tried": 0,
"total": 0
},
"i2p": {
"new": 0,
"tried": 0,
"total": 0
},
"cjdns": {
"ne
...