💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141838493)
per the functional test README:

```
- If more than one name from a module is needed, use lexicographically sorted multi-line imports
in order to reduce the possibility of potential merge conflicts.
```

so this should be

```suggestion
from test_framework.segwit_addr import (
bech32_decode,
convertbits,
encode_segwit_address,
)
```

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141903161)
styling nit: two blank lines between functions, per PEP8 guidlines

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141899143)
since you are already using the `encode_segwit_address` function, it might be better to also use the `decode_segwit_address` function here instead of duplicating the logic.

something like:

```suggestion
hrp = address[:2]
version, payload = decode_segwit_address(hrp, address)
return version, bytearray(payload)
```

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141903949)
styling nit: two blank lines between functions, per PEP8 guidelines

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141909051)
styling nit: should be lexicographically sorted and add a comma at the end of each line (even if it is the last one)

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141905124)
also, if you end up taking my suggestion to use `decode_segwit_address`, you can remove the `bech32_decode, convertbits` imports

💬 flack commented on pull request "refactor: wallet, do not translate init options names":
(https://github.com/bitcoin/bitcoin/pull/25666#discussion_r1141928781)
Strictly speaking, the middle sentence is a bit redundant (and also contains the option name)

💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1476003813)
Rebased b22cf8d563a469e29c9942b4fd9f93351d8b9766 -> cc662d09386e50eb92a7ccbaa1edff62fd8cdd44 ([splitSystemFs_4](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_4) -> [splitSystemFs_5](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_4..splitSystemFs_5)) to fix conflicts and update the commit message to exclude the scripted diff.

📝 MarcoFalke opened a pull request: "test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/27280)
null

💬 MarcoFalke commented on pull request "test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/27280#issuecomment-1476011366)
Can be tested with:

```diff
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 70a802dc58..1d1c6a64b6 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -52,7 +52,7 @@ class InitStressTest(BitcoinTestFramework):
assert_equal(200, node.getblockcount())

lines_to_terminate_after = [
- b'Validating signatures for all blocks',
+ b'.Validating signatures for all blocks',

...

🤔 dergoegge requested changes to a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)
Concept ACK

Prefer this over #27276 as well I think.

Maybe add the peer id to the log locations as well?

💬 dergoegge commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141510303)
I think this `LogPrintf` call isn't safe because the work on the header has not actually been checked, that first happens in `ProcessNewBlockHeaders` below (by "safe" i mean that this location could be used to fill our disk with log messages).

At this point the header is only claiming to have enough work (i.e. `prev_block->nChainWork + CalculateHeadersWork({cmpctblock.header}) >= GetAntiDoSWorkThreshold()`) but the actual PoW check happens later on.

You can move this log location below the
...

💬 fanquake commented on pull request "Log successful AcceptBlockHeader()":
(https://github.com/bitcoin/bitcoin/pull/27276#issuecomment-1476018573)
Closing in favour of #27278.

✅ fanquake closed a pull request: "Log successful AcceptBlockHeader()"
(https://github.com/bitcoin/bitcoin/pull/27276)

👍 alexsio27444 approved a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)

💬 josibake commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#discussion_r1141953882)
grammar nit:

```suggestion
// This can only happen if the feerate is 0, the requested destinations are value of 0 (e.g OP_RETURN),
// and there are no pre-selected inputs. This will result in a 0-input transaction, which is consensus-invalid
```

👍 alexsio27444 approved a pull request: "refactor: Add BlockManager getters"
(https://github.com/bitcoin/bitcoin/pull/26900)

👍 alexsio27444 approved a pull request: "net, refactor: net_processing, add `ProcessCompactBlockTxns`"
(https://github.com/bitcoin/bitcoin/pull/26969)

⚠️ fanquake opened an issue: "test: `interface_bitcoin_cli.py --descriptors` failure under `--valgrind`"
(https://github.com/bitcoin/bitcoin/issues/27281)
Seen on a aarch64 Alpine box. Master @ 40e1c4d4024b8ad35f2511b2e10bf80c5531dfde. Binaries compiled with Clang 15.0.7. Valgrind `valgrind-3.21.0.GIT`.

```bash
79/262 - interface_bitcoin_cli.py --descriptors failed, Duration: 34 s

stdout:
2023-03-20T10:34:23.174000Z TestFramework (INFO): PRNG seed is: 6631947331891546746
2023-03-20T10:34:23.175000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20230320_101936/interface_bitcoin_cli_187
2023-03-20T10:34:29.054000Z
...

💬 ajtowns commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141958384)
Could just make this:

```c++
const auto level{(IsIBD() ? BCLog::Level::Info : BCLog::Level::Warning)};
LogPrintf(BCLog::VALIDATION, level, "saw new ...", hash.ToString());
```

(or `None` rather than `Warning` maybe)