💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633223401)
> This was the case before this PR, right?
No, a peek read before this PR would return 1 byte and a normal read following that would also only return that one byte (with the exception of sometimes padding with a bunch of zeros). This is what causes the fuzz input to never be interpreted as one continuous sequence of data, which is why mutations that insert pieces of data (e.g. from a dictionary) are ineffective.
> Where can I see that use case? Is there some code that does not work without
...
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633223401)
> This was the case before this PR, right?
No, a peek read before this PR would return 1 byte and a normal read following that would also only return that one byte (with the exception of sometimes padding with a bunch of zeros). This is what causes the fuzz input to never be interpreted as one continuous sequence of data, which is why mutations that insert pieces of data (e.g. from a dictionary) are ineffective.
> Where can I see that use case? Is there some code that does not work without
...
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107775468)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107775468)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.
📝 glozow opened a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260)
Backports:
- #30151
And final changes for 26.2rc1:
- build changes
- manpages
(https://github.com/bitcoin/bitcoin/pull/30260)
Backports:
- #30151
And final changes for 26.2rc1:
- build changes
- manpages
👍 ryanofsky approved a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2107778178)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.
(https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2107778178)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633234440)
These changes were very much necessary for `FuzzedSock` to be fuzzer friendly. Feel free to open a follow up, if you have suggestion how further improve things.
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633234440)
These changes were very much necessary for `FuzzedSock` to be fuzzer friendly. Feel free to open a follow up, if you have suggestion how further improve things.
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633229348)
Feel free to open a follow up.
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633229348)
Feel free to open a follow up.
💬 glozow commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2158358461)
backported to 26.x in #30260
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2158358461)
backported to 26.x in #30260
💬 instagibbs commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#issuecomment-2158379951)
ACK https://github.com/bitcoin/bitcoin/pull/30160/commits/47f705b33fc1381d96c99038e2110e6fe2b2f883
swap coverage added, some of the nits taken, and a couple `Assume()`s added
reviewed via `git range-diff master c99063e 47f705b`
(https://github.com/bitcoin/bitcoin/pull/30160#issuecomment-2158379951)
ACK https://github.com/bitcoin/bitcoin/pull/30160/commits/47f705b33fc1381d96c99038e2110e6fe2b2f883
swap coverage added, some of the nits taken, and a couple `Assume()`s added
reviewed via `git range-diff master c99063e 47f705b`
📝 fanquake opened a pull request: "doc: add release note for 29091 and 29165"
(https://github.com/bitcoin/bitcoin/pull/30261)
GCC 11.x or Clang 15.x are now required to compile Bitcoin Core.
(https://github.com/bitcoin/bitcoin/pull/30261)
GCC 11.x or Clang 15.x are now required to compile Bitcoin Core.
💬 ryanofsky commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158381286)
re: https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2157745629
> No, and I'm not sure it should be given we don't support this. Maybe we can add a comment and assert that just wiping the block index db is not supported for now?
FWIW, my original draft of 804f09dfa116300914e2aeef05ed9710dd504e8c added this code to LoadChainstate:
```c++
// For now, don't allow wiping block tree db without also wiping chainstate
// db. There's no reason this could not work in theory, but in p
...
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158381286)
re: https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2157745629
> No, and I'm not sure it should be given we don't support this. Maybe we can add a comment and assert that just wiping the block index db is not supported for now?
FWIW, my original draft of 804f09dfa116300914e2aeef05ed9710dd504e8c added this code to LoadChainstate:
```c++
// For now, don't allow wiping block tree db without also wiping chainstate
// db. There's no reason this could not work in theory, but in p
...
💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2158381917)
Release note added in #30261.
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2158381917)
Release note added in #30261.
💬 fanquake commented on pull request "build: Bump g++ minimum supported version to 11":
(https://github.com/bitcoin/bitcoin/pull/29091#issuecomment-2158382309)
Release note in #30261.
(https://github.com/bitcoin/bitcoin/pull/29091#issuecomment-2158382309)
Release note in #30261.
👍 maflcko approved a pull request: "doc: add release note for 29091 and 29165"
(https://github.com/bitcoin/bitcoin/pull/30261#pullrequestreview-2107809990)
lgtm
(https://github.com/bitcoin/bitcoin/pull/30261#pullrequestreview-2107809990)
lgtm
💬 instagibbs commented on pull request "Ephemeral Anchors, take 2":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2158409770)
@theStack the primary motivation is to cover cases where non-0 value is attached to handle cases where a smart contract may want to "throw away" a few sats to fees, but otherwise cannot because of the 0-fee requirement of this PR for transactions with ephemeral anchors. If the ephemeral anchor-having transaction had non-0-fee, that would allow endogenous incentives to get it mined on its own, leaving the dust in the utxo set. As an example from the LN spec, [trimmed outputs(https://github.com/li
...
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2158409770)
@theStack the primary motivation is to cover cases where non-0 value is attached to handle cases where a smart contract may want to "throw away" a few sats to fees, but otherwise cannot because of the 0-fee requirement of this PR for transactions with ephemeral anchors. If the ephemeral anchor-having transaction had non-0-fee, that would allow endogenous incentives to get it mined on its own, leaving the dust in the utxo set. As an example from the LN spec, [trimmed outputs(https://github.com/li
...
💬 hebasto commented on pull request "[26.x] backports and final changes for 26.2rc1":
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158414684)
@glozow
Feel free to pick up the https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6 commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Unfortunately, more translations have been vandalized (
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158414684)
@glozow
Feel free to pick up the https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6 commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Unfortunately, more translations have been vandalized (
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633234183)
Does this mean that the time will be frozen for the duration of the test?
From the OP:
> GetTime is called at points in i2p
Where is `GetTime()` called in i2p?
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633234183)
Does this mean that the time will be frozen for the duration of the test?
From the OP:
> GetTime is called at points in i2p
Where is `GetTime()` called in i2p?
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633272627)
Can be simplified:
```suggestion
CThreadInterrupt interrupt;
i2p::sam::Session session{private_key_path, Proxy{CService{}, false}, &interrupt};
```
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633272627)
Can be simplified:
```suggestion
CThreadInterrupt interrupt;
i2p::sam::Session session{private_key_path, Proxy{CService{}, false}, &interrupt};
```
👍 cbergqvist approved a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2107852785)
re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883
Used same range-diff as instagibbs.
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2107852785)
re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883
Used same range-diff as instagibbs.
💬 glozow commented on pull request "[26.x] backports and final changes for 26.2rc1":
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158437058)
> Feel free to pick up the [hebasto@a586d70](https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6) commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Thanks @hebasto! I've cherry-picked that with a minor change to the commit message
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158437058)
> Feel free to pick up the [hebasto@a586d70](https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6) commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Thanks @hebasto! I've cherry-picked that with a minor change to the commit message
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2158443019)
> I am still planning to do the rollback in a cleaner way but[...] It would become a refactoring follow-up that does not change the RPC interface again.
That makes sense.
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2158443019)
> I am still planning to do the rollback in a cleaner way but[...] It would become a refactoring follow-up that does not change the RPC interface again.
That makes sense.