💬 ponury1990 commented on issue "Bitcoin core full node with S3 bucket":
(https://github.com/bitcoin/bitcoin/issues/27168#issuecomment-1445904156)
Ja to bym chciał w końcu poczuć ze mam coś a nie tylko się patrzyć nie da sie chociaż 1 BTC na portfel przelać trzeba się prosić o swoje szczęscie ?

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1118460580)
I'm not sure this is how APS works. When we remove negative UTXOs we assess the whole group and not each UTXO individually, so you don't need to regroup anything after filtering negative groups.

💬 hebasto commented on issue "Bitcoin core full node with S3 bucket":
(https://github.com/bitcoin/bitcoin/issues/27168#issuecomment-1445967141)
NACK on any feature which allows to delegate a block storage to a third-party.

It will create an opportunity for an adversary to alter block data, which being served to other peers will create a spam network traffic. That is, essentially, an attack on the Bitcoin network.

---

In addition to the [mentioned](https://github.com/bitcoin/bitcoin/issues/27168#issuecomment-1445884569) option, one can use `-blocksdir` to manage block storage location.

✅ fanquake closed an issue: "Bitcoin core full node with S3 bucket"
(https://github.com/bitcoin/bitcoin/issues/27168)

💬 MarcoFalke commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1118563747)
Not sure about leaking test-only code into real code. Have you tried to reset the categories instead?

💬 Ayms commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1446096405)
@glozow Thanks, @1440000bytes I saw your comments that you have deleted apparently (maybe a misunderstanding about my intentions), for my information who are the current 4 maintainers?

I don't see where I could be dishonest in this thread and why I would be trying to force/bypass some processes, I am a bit used to this since many years here and elsewhere, my only concerns are: what is the issue with this trivial change compared to other standard storage methods? And not doing it might lead p
...

💬 fanquake commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#issuecomment-1446104588)
Guix Build:
```bash
6d03c85b270c0239b3f480dbdbdd3e2465ac666228b0c9a1a3e71005bb9fef73 guix-build-5da7c0b3e346/output/aarch64-linux-gnu/SHA256SUMS.part
df27c28a6421f571c9f2b176dc2fb3d70a74898d9f85a278a5487d734767264e guix-build-5da7c0b3e346/output/aarch64-linux-gnu/bitcoin-5da7c0b3e346-aarch64-linux-gnu-debug.tar.gz
bfe7ae61f2b60049f31b9e67e3b3144779a20506ba8874946499e56f558317d4 guix-build-5da7c0b3e346/output/aarch64-linux-gnu/bitcoin-5da7c0b3e346-aarch64-linux-gnu.tar.gz
423401caa7cc886b
...

💬 MarcoFalke commented on issue "Intermittent issue in p2p_ibd_stalling.py self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)":
(https://github.com/bitcoin/bitcoin/issues/27112#issuecomment-1446104867)
Ok, thank you. It was running all tests in parallel in valgrind, which may explain why things went slow.

✅ MarcoFalke closed an issue: "Intermittent issue in p2p_ibd_stalling.py self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)"
(https://github.com/bitcoin/bitcoin/issues/27112)

👍 stickies-v approved a pull request: "Handle invalid hex encoding in ParseHex"
(https://github.com/bitcoin/bitcoin/pull/25227)
ACK fad0c892c34c30cf8f50e832425210e24d45837e

An API that fails on invalid input instead of accepting the partial valid part is more robust imo. I don't see any places where this behaviour change will cause issues, but there are a lot of callsites.

---

Looking at the callsites of `ParseHex()`, it looks like there's a significant number of places where we first check `IsHex()` and then `ParseHex()`, iterating over the string twice when I think this can just be replaced with a single `TryP
...

💬 stickies-v commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118568346)
nit: for all these, would prefer having consistency between using a temporary var for the `ParseHex` and `TryParseHex` tests
```suggestion
BOOST_CHECK_EQUAL(ParseHex(with_embedded_null), 0);
```

💬 stickies-v commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118546663)
nit: add description of expected behaviour (and in 0000509239d4e699f57b392531f242ad6933c982 it would be "Truncated input is ignored")
```suggestion
// Truncated input makes the parsing fail entirely
```

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118650899)
Can you explain this?

Currently it is consistent in that a all cases use `result = ParseHex(...)`. This is the case before this pull request and not changed.

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118651283)
(Also, your suggestion wouldn't compile as is, I am pretty sure)

💬 stickies-v commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118658103)
The below diff captures my entire suggestion (and compiles/tests). In these locations, we use a temp var for `ParseHex` but not for `TryParseHex`. It's just a style/minor thing (hence the nit), but I think having consistency between both makes it easier for the reviewer to see the similarities between both tests.

<details>
<summary>git diff</summary>

```diff
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 65468a113..74e151b99 100644
--- a/src/test/util_tests.cpp
+
...

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#issuecomment-1446262046)
force pushed to change tests. can be re-reviewed with range-diff

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118687591)
Thanks, done.

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#discussion_r1118687793)
thanks, done

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1118693568)
Also ran both variants in nanobench and there is no difference.

🚀 glozow merged a pull request: "contrib: Improve verify-commits.py to work with maintainers leaving"
(https://github.com/bitcoin/bitcoin/pull/27058)