💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626177967)
you can ignore this if you don't like it
```Suggestion
/** m_buffer + m_offset points to first object in queue. m_offset = 0 if m_capacity is 0; otherwise
```

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626321220)
Putting bounds on the capacity growth seems useful here and a few other spots, even if it doesn't necessarily have to be this tight.
```
if (old_cap > old_size) {
assert(real[idx].capacity() == old_cap);
} else {
assert(real[idx].capacity() > old_cap);
assert(real[idx].capacity() <= 2 * (old_cap + 1));
}
```

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626291871)
`s/Index/BufferIndex/` reduces my mental load fwiw

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626341902)
Calling `compare_fn` here seems more complete than picking and choosing when to call it before the final check
```Suggestion
if (!real.empty()) compare_fn(real[idx], sim[idx]);
if (non_full && command-- == 0) {
```

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626395824)
double checking: `first_part == 0` implies `m_size == 0`?

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626387016)
gave an attempt
```Suggestion
/** Returns the number of populated objects from m_offset to end of allocated memory. */
size_t FirstPart() const noexcept { return std::min(m_capacity - m_offset, m_size); }
```

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626448996)
seems ever so slightly more direct, even though this seems correct as is
```Suggestion
if (&other == this) return *this;
```

💬 theuni commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2148290427)
> `if (this == &other) return *this;`

Huh, that worked. But this (the one I tried) doesn't
```c++
if (*this == other) return *this;
```

💬 theuni commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2148299043)
Oh, duh, that's a totally different comparison. Ofc that didn't do what I wanted :)

💬 maflcko commented on pull request "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure":
(https://github.com/bitcoin/bitcoin/pull/30174#discussion_r1626530422)
Yeah, I think it shouldn't matter here. An alternative would be to increase the magic number `120` sufficiently (maybe by a factor of 10 or so).

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626544077)
Indeed!

💬 willcl-ark commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2148338315)
I think an upstream PR may be the best approach here, So I opened https://github.com/becheran/mlc/pull/89

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2148351046)
Not sure if globbing is the right fix. Conceptually, no folder or file should be scanned that is not tracked in git.

Though, I wonder if there is an easy way to ask git to print all files/folders that are "dirty", ignoring the gitignore. An alternative may be to teach mlc to read understand the gitignore files. However, that will also be incomplete, because devs may add random other folders themselves.

So for now the only solution would be to call `mlc` in a loop over `git ls-files -- '*.m
...

💬 maflcko commented on pull request "test: Set mocktime in p2p_disconnect_ban.py to avoid intermittent test failure":
(https://github.com/bitcoin/bitcoin/pull/30174#issuecomment-2148366625)
> nit: If another push happens, these look like typos:

Thanks, fixed.

> Did you run into this specific failure scenario, or simply being proactive?

It happened once with valgrind, which sometimes takes some time to load. You can test with:

```diff
diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py
index 72a42a658e..aa4ae7ea27 100755
--- a/test/functional/p2p_disconnect_ban.py
+++ b/test/functional/p2p_disconnect_ban.py
@@ -103,6 +103,7 @@
...

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626579091)
Done.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626579194)
Done.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626579310)
Done.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626580105)
I didn't take this suggestion. The current should be invoking `compare_fn` whenever an element is destructed or destructively overwritten; that's not necessarily number `idx` though.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626580268)
Done.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1626580385)
Done.