💬 edilmedeiros commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1592905348)
Since you are proposing random input vectors, what do you think about adding a log message with the generated test vector?

`BOOST_TEST_MESSAGE("Test input: '" << encoded << "'");`

looks good on my terminal. You probably can figure out a better message.

💬 edilmedeiros commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1592956103)
Same thing as above.

```
test/base58_tests.cpp:45: Entering test case "base58_DecodeBase58"
test/base58_tests.cpp:60: info: check '["",""]
Decoding failed for test #0: ' has passed
test/base58_tests.cpp:64: info: check '["",""]
Mismatch for test #0: expected , got ' has passed
test/base58_tests.cpp:60: info: check '["61","2g"]
Decoding failed for test #1: 2g' has passed
test/base58_tests.cpp:64: info: check '["61","2g"]
Mismatch for test #1: expected 61, got 61' has passed
test/base
...

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#issuecomment-2099233436)
Thanks for the review @edilmedeiros, some of the existing BOOST_CHECK_MESSAGE messages are either in [indicative](https://github.com/bitcoin/bitcoin/blob/master/src/test/bloom_tests.cpp#L51) or [subjunctive](https://github.com/bitcoin/bitcoin/blob/master/src/test/bloom_tests.cpp#L31) grammatical moods (even in the same file, as you can see).
I'm fine with both, but if you think the subjunctive is more readable, I'll rephrase.

👍 hebasto approved a pull request: "Fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2044111050)
ACK fb9f150759b22772dd48983a2be1ea397245e289.

💬 edilmedeiros commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#issuecomment-2099261379)
> Thanks for the review @edilmedeiros, some of the existing BOOST_CHECK_MESSAGE messages are either in [indicative](https://github.com/bitcoin/bitcoin/blob/master/src/test/bloom_tests.cpp#L51) or [subjunctive](https://github.com/bitcoin/bitcoin/blob/master/src/test/bloom_tests.cpp#L31) grammatical moods (even in the same file, as you can see). I'm fine with both, but if you think the subjunctive is more readable, I'll rephrase.

I have no personal preference about it, but this is not my point.
...

✅ hebasto closed an issue: "Signmessage doesn't work with segwit addresses"
(https://github.com/bitcoin/bitcoin/issues/10542)

🚀 hebasto merged a pull request: "Fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819)

💬 cbergqvist commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1593070661)
Without any additional changes. With `--enable-debug` and building with Clang.

I've retested on ecded92737c4173a30a43e612a21c6e1f24d1605, here is the combined log for `test/functional/p2p_seednode.py`:
[29605_timeout.combined_log.gz](https://github.com/bitcoin/bitcoin/files/15240761/29605_timeout.combined_log.gz)

It seems like I only have to bump 20 -> 23 seconds to prevent frequent timeouts. I've got IPv6 turned off but otherwise not much weird stuff going on AFAIK. Maybe something stand
...

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1593114513)
Ah, now I get what you meant - I've changed the messages to shoulds, thanks for the hint!
Running it with `make && src/test/test_bitcoin --run_test=base58_tests --log_level=all -- -printtoconsole=1` now prints:
```
test/base58_tests.cpp:108: info: check 'Decoding `111111111111111J4kMuvqbHmtTVvULAvzkZCa3m2n9zHaYBL3KCE5y7phvY7737aYsgLjg5343Awj7jbBuYvioVwvkP4HvWsb3T6F8w3WEax4HkrTx8GXbZc1R1m4BWT48R8d2rU3557aNGpQ5pFyaFHYzTzDiNRTVmoYG7pns87AuFahyrTUNRufZe4gLf5Mb1JhWDCd438qHu8pBFWsVJrGkw` as `00000
...

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1593116395)
Thanks, I've cherry picked this, in the current impl it's called `max_ret_len` - fixed

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1593117357)
Fixed, thanks!
Added you as a Co-author, please check that the email is correct.

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1593117445)
Good idea, done

💬 iotamega commented on issue "Possible to Ban Clients by Name?":
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2099367681)
Vouch, spoke to Secret Service this week. They are aware of the issue and a case is open for it. Your local office can assist.

💬 mzumsande commented on issue "Memory leak with `rest/block` REST endpoint and `getblock` RPC when verbosity >=2":
(https://github.com/bitcoin/bitcoin/issues/30052#issuecomment-2099375700)
> I could not reproduce an OOM, although I did observe resource usage increasing a little before eventually plateauing.

I see the same kind of behavior with Ubuntu with REST, the memory usage saturates after a few calls. So it looks like the same issue as in #25229 - at least for me.

💬 maciejsszmigiero commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#discussion_r1593153350)
Added a relevant constant.

💬 maciejsszmigiero commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2099395788)
> If there's no drawbacks, why not go even larger?

I used 128 MiB as the new size for commonality with `MAX_BLOCKFILE_SIZE` already used by the block storage and because it gives me a nice low disk cache flush rate of about 1 flush / 2 seconds that no longer impacts the overall system performance.

But just to be sure, changed the patch to use `std::max()` around this `max_file_size` option so if at some point LevelDB decides to increase its default above 128 MiB we won't be lowering it acc
...

🤔 edilmedeiros requested changes to a pull request: "test: use assert_greater_than util"
(https://github.com/bitcoin/bitcoin/pull/30019#pullrequestreview-2044207210)
Concept ACK.

I have run all the tests that have changed, it seems ok.

Thanks for the contribution. But, since you are already refactoring this out, I believe it would be way better to not leave any more `assert` behind that could be changed to `assert_equal` or `assert_greater_than`. Otherwise, looks like we are not improving the code base as there will be more than one style of asserts in some files.

See the diffs in the comments to find some suggestions.

To find others, grep each
...

💬 edilmedeiros commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#discussion_r1593096616)
Change these to `assert_equal` is an easy gain here.

💬 edilmedeiros commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#discussion_r1593146698)
```diff
diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py
index cf48826e6a..415e4f1b95 100755
--- a/test/functional/feature_rbf.py
+++ b/test/functional/feature_rbf.py
@@ -15,6 +15,7 @@ from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than,
+ assert_greater_than_or_equal,
assert_raises_rpc_error,
)
from test_framework.wallet import MiniWallet
@@ -446,9 +447,9
...

💬 edilmedeiros commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#discussion_r1593156322)
```diff
index ff67207c4e..2a4cc9dfba 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -85,6 +85,7 @@ from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than,
+ assert_greater_than_or_equal,
softfork_active,
assert_raises_rpc_error,
)
@@ -376,14 +377,14 @@ class SegWitTest(BitcoinTestFramework):
self.test_node.send_message(msg_headers())


...