Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 TheCharlatan approved a pull request: "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks"
(https://github.com/bitcoin/bitcoin/pull/27028)
re-ACK faa0839837e6d2ba5d0ab08fddd497a257d7e075
💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1482587371)
review-beg: @willcl-ark
💬 MarcoFalke commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1482607575)
Just picked the latest release, see https://github.com/MarcoFalke/DrahtBot/commit/0da5d6aacd9b4296af9d1ccdefe8aecf6b60147d

Haven't tried `guix pull`, because I am not sure if it persists in my workflow.
💬 MarcoFalke commented on issue "Running RPC call importmulti with a xpub key with very large range results in a timeout error":
(https://github.com/bitcoin/bitcoin/issues/17797#issuecomment-1482610945)
Closing for now, because bdb will be phased out and this doesn't seem to be happening with descriptor wallets? Let us know if this is still an issue and should be reopened.
MarcoFalke closed an issue: "Running RPC call importmulti with a xpub key with very large range results in a timeout error"
(https://github.com/bitcoin/bitcoin/issues/17797)
💬 jonatack commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1147473618)
Here's another idea. If the server version is less than current master, return the current -addrinfo, and otherwise return something like:

```
{
"With quality/recency filtering (original -addrinfo)": {
"addresses_known": {
"ipv4": 1,
"ipv6": 0,
"onion": 14673,
"i2p": 1129,
"cjdns": 7,
"total": 15810
}
},
"All addresses (no filtering)": {
"addresses_known": {
"ipv4": 1,
"ipv6": 0,
"onion": 14673,
"i2
...
👍 willcl-ark approved a pull request: "test: Support decoding segwit address in address_to_scriptpubkey()"
(https://github.com/bitcoin/bitcoin/pull/27269)
ACK d17808299

I also tested two of the BIP173 [Test Vectors](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#test-vectors):

```python
check_bech32_decode(bytes.fromhex('0020000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433'), 1)
check_bech32_decode(bytes.fromhex('00201863143c14c5166804bd19203356da136c985678cd4d27a1b8c6329604903262'), 1)
```

...which also passed. In doing so I noticed the hardcoding of human readable part `hrp` in _address.py_#L196 was li
...
💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147454419)
I think this is totally fine as is here for now, but in the future we may want to put these all in a list/enum
💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147486821)
Instead of hardcoding the hrp here, why not have `check_bech32_decode()` take `hrp` as a parameter, then we can add `bc1` test vectors instead of being limited to `tb1`?
💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147460292)
I may be mistaken but I believe that actually this [_should_](https://peps.python.org/pep-0008/#blank-lines) have 1 line gap, as it's a member function? But its OK, I wouldn't change and invalidate the ACKS you've got already! (I configured my editor to show PEP8 diagnostics so I can view them directly as I'm editing).
🚀 fanquake merged a pull request: "test: Support decoding segwit address in address_to_scriptpubkey()"
(https://github.com/bitcoin/bitcoin/pull/27269)
🚀 fanquake merged a pull request: "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks"
(https://github.com/bitcoin/bitcoin/pull/27028)
⚠️ MarcoFalke opened an issue: "ci: Cleanup ci/test/01_base_install.sh"
(https://github.com/bitcoin/bitcoin/issues/27321)
### Motivation

Currently `ci/test/01_base_install.sh` has the following issues:

* Two names to alias `bash -c`: `CI_EXEC_ROOT`, and `CI_EXEC`
* The aliases are not needed, as explained in the comment in the file

Thus, they should be removed, or at least de-duplicated. The whole file is already run in bash, so the only place where `bash -c "..."` is needed is in the `apt install` step to properly pass the packages string with spaces.

### Possible solution

* Remove or de-duplicate `CI_EX
...
💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1147543379)
Alas that does not work if the condition includes function calls.
💬 willcl-ark commented on issue "core stops to run with `Failed to read block` error":
(https://github.com/bitcoin/bitcoin/issues/27142#issuecomment-1482781323)
Do you think it's something you will try again in the future? It would be good to get reproduction steps if possible...
📝 jonatack opened a pull request: "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet"
(https://github.com/bitcoin/bitcoin/pull/27322)
While deprecating the remaining wallet RPC "warning" fields in #27279, the CI reported `undefined reference to IsDeprecatedRPCEnabled in libbitcoin_wallet` compiler errors when this function was called from wallet RPCs.

Fix this by moving the function from `rpc/server` to `rpc/util`, which by looking at `doc/design/librairies.md` and `src/Makefile.am` looks like the right place for it. Happy to change approach per reviewer feedback.

Cherry-picked from #27279, where the relevant CI is now
...
💬 jonatack commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1482789635)
Same error reported by CI in https://cirrus-ci.com/task/5776249860128768?logs=functional_tests#L2682.

```
[default_wallet] AddToWallet 87b8816da11c3e05844802cf2ca4ac0fdcd605d0b53ae046f470264aec116878
test 2023-03-23T23:35:57.593000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_fra
...
💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1147575925)
I can imagine some use cases where having consistent netgroup identifiers would be advantageous. Unless `NetGroupManager::GetGroup()` adds too much overhead, they seem preferable.
💬 furszy commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1482795409)
@jonatack https://github.com/bitcoin/bitcoin/pull/27318
💬 MarcoFalke commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1482807752)
lgtm

Maybe explain that this moves it from `node` to `common`?