💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1283710640)
Does it worth? BnB internally calls `ComputeSetAndWaste` which checks change. Any unexpected action regarding it would affect the assertion - `assert(best_waste == result.GetWaste());`.
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1283710640)
Does it worth? BnB internally calls `ComputeSetAndWaste` which checks change. Any unexpected action regarding it would affect the assertion - `assert(best_waste == result.GetWaste());`.
💬 brunoerg commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283716489)
I don't think it's worth to mention this in a release note, seems like a specific code nuance to be exposed. Isn't it?
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283716489)
I don't think it's worth to mention this in a release note, seems like a specific code nuance to be exposed. Isn't it?
💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#issuecomment-1664651486)
> https://github.com/bitcoin/bitcoin/commit/68e48a0185751d24eecb194b8efd7028c8b590f3: can you elaborate a bit in the commit description why nSendOffset can be dropped?
In the current codebase, the send buffering (= remembering the to-be-sent bytes which we haven't managed to send yet) is done `vSendMsg` (a queue of *byte arrays*) + `nSendOffset` (the position within `vSendMsg[0]` up to where we've sent things). After this PR, `vSendMsg` is turned into a queue of *messages*, which have not yet
...
(https://github.com/bitcoin/bitcoin/pull/28165#issuecomment-1664651486)
> https://github.com/bitcoin/bitcoin/commit/68e48a0185751d24eecb194b8efd7028c8b590f3: can you elaborate a bit in the commit description why nSendOffset can be dropped?
In the current codebase, the send buffering (= remembering the to-be-sent bytes which we haven't managed to send yet) is done `vSendMsg` (a queue of *byte arrays*) + `nSendOffset` (the position within `vSendMsg[0]` up to where we've sent things). After this PR, `vSendMsg` is turned into a queue of *messages*, which have not yet
...
💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664652771)
> > Why do you believe that expanding a publication mechanism that is 4x more expensive than the heavily used taproot witness mechanism will "increase the spam problems significantly"?
>
> @ChristopherA just said that he's paying you to push this because it makes it cheaper and easier for him to write his non-bitcoin stuff into the chain.
Some clarification: I said I would contribute $1K toward a bounty for a real PR. Others said they might contribute to that bounty as well. My real goal w
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664652771)
> > Why do you believe that expanding a publication mechanism that is 4x more expensive than the heavily used taproot witness mechanism will "increase the spam problems significantly"?
>
> @ChristopherA just said that he's paying you to push this because it makes it cheaper and easier for him to write his non-bitcoin stuff into the chain.
Some clarification: I said I would contribute $1K toward a bounty for a real PR. Others said they might contribute to that bounty as well. My real goal w
...
💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664659414)
A question to those who object to this PR, would any of you change your mind if a new `is_standard` was simply a little larger? Maybe a modest 256 or 512 bytes?
Though I believe there are other reasons OP_RETURN shouldn't be limited, if advancing a modest limit is an option, it is at least a way for devs who "want to do the right thing" rather than enormous inscriptions and other hacks, I would support it.
-- Christopher Allen
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664659414)
A question to those who object to this PR, would any of you change your mind if a new `is_standard` was simply a little larger? Maybe a modest 256 or 512 bytes?
Though I believe there are other reasons OP_RETURN shouldn't be limited, if advancing a modest limit is an option, it is at least a way for devs who "want to do the right thing" rather than enormous inscriptions and other hacks, I would support it.
-- Christopher Allen
💬 paplorinc commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283740534)
```Python
# Test that 1-of-1000 and 999-of-1000 raise exception
for k in [1, 999]:
assert_raises_rpc_error(-5, "Cannot have 1000 keys in multi_a; must have between 1 and 999 keys, inclusive", self.do_test_k_of_n, k, 1000)
```
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283740534)
```Python
# Test that 1-of-1000 and 999-of-1000 raise exception
for k in [1, 999]:
assert_raises_rpc_error(-5, "Cannot have 1000 keys in multi_a; must have between 1 and 999 keys, inclusive", self.do_test_k_of_n, k, 1000)
```
💬 Empact commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1664679951)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28199#issuecomment-1664679951)
Concept ACK
💬 ariard commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1664750667)
> I'm not aware of any second layer protocols that are improved by having full-rbf more broadly available.
@TheBlueMatt See https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html and https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-May/003920.html
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1664750667)
> I'm not aware of any second layer protocols that are improved by having full-rbf more broadly available.
@TheBlueMatt See https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html and https://lists.linuxfoundation.org/pipermail/lightning-dev/2023-May/003920.html
💬 brunoerg commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283810102)
I don't think it's really a bug. It seems like this was the expected behavior (due to examples and docs - e.g. having to specify port). However, I also think it should be able to disconnect all nodes from an IP. I'm gonna address it.
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1283810102)
I don't think it's really a bug. It seems like this was the expected behavior (due to examples and docs - e.g. having to specify port). However, I also think it should be able to disconnect all nodes from an IP. I'm gonna address it.
💬 1ma commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664757776)
> My goal is not "non-bitcoin stuff". My specific challenge is that Lightning and Nostr use public keys in a persistent way that are not rotatable, which is a bad cryptographic practice. By committing to a rotated key and some other cryptographic in advance in L1, they could be. I believe this could be as small as a signed hash and a little metadata.
@ChristopherA can you clarify why it is imperative for this use case to store that data on the Bitcoin blockchain?
To give you some insight i
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664757776)
> My goal is not "non-bitcoin stuff". My specific challenge is that Lightning and Nostr use public keys in a persistent way that are not rotatable, which is a bad cryptographic practice. By committing to a rotated key and some other cryptographic in advance in L1, they could be. I believe this could be as small as a signed hash and a little metadata.
@ChristopherA can you clarify why it is imperative for this use case to store that data on the Bitcoin blockchain?
To give you some insight i
...
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283872989)
oh interesting. do I understand this correctly: the `LOCK_RETURNED` annotation tells thread safety analysis that this function will return `m_nodes_mutex`, even though it's just an empty function, which lets the lambda use `EXCLUSIVE_LOCKS_REQUIRED`. this works because it doesn't need to actually return the mutex there, it just needs to promise the compiler it will be there, which is handled elsewhere.
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283872989)
oh interesting. do I understand this correctly: the `LOCK_RETURNED` annotation tells thread safety analysis that this function will return `m_nodes_mutex`, even though it's just an empty function, which lets the lambda use `EXCLUSIVE_LOCKS_REQUIRED`. this works because it doesn't need to actually return the mutex there, it just needs to promise the compiler it will be there, which is handled elsewhere.
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1664868727)
I haven't been able to cause a crash with the connection-based code. Given that the connection-based code is different than the crashing code here, do you want to open a new PR with the connection-based code @stickies-v ?
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1664868727)
I haven't been able to cause a crash with the connection-based code. Given that the connection-based code is different than the crashing code here, do you want to open a new PR with the connection-based code @stickies-v ?
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283935267)
hm, thinking about this more... I believe this workaround diminishes some of the value of the compile-time notation. for example, if `ForEachNode` no longer acquired `m_nodes_mutex`, then the compiler wouldn't be able to flag the issue, but the `AssertLockHeld` would catch it at runtime.
the advantage is that this one weak check allows to maintain the compiler checks at the other levels (of functions and members that are protected), and we remove the recursive locking of the mutex.
this
...
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1283935267)
hm, thinking about this more... I believe this workaround diminishes some of the value of the compile-time notation. for example, if `ForEachNode` no longer acquired `m_nodes_mutex`, then the compiler wouldn't be able to flag the issue, but the `AssertLockHeld` would catch it at runtime.
the advantage is that this one weak check allows to maintain the compiler checks at the other levels (of functions and members that are protected), and we remove the recursive locking of the mutex.
this
...
📝 fanquake reopened a pull request: "p2p: diversity network outbounds follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28189)
release notes for #27213, will rebase after it gets merge
(https://github.com/bitcoin/bitcoin/pull/28189)
release notes for #27213, will rebase after it gets merge
💬 BenWestgate commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1664944819)
I have a project that syncs bitcoin core on an external drive and this would really help a lot of people testing it with smaller flash drives and higher RAM. So much so that I've made scripts to do manual pruning (back to 550mib) only when disk space is getting low to emulate this.
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1664944819)
I have a project that syncs bitcoin core on an external drive and this would really help a lot of people testing it with smaller flash drives and higher RAM. So much so that I've made scripts to do manual pruning (back to 550mib) only when disk space is getting low to emulate this.
💬 RobinLinus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664950834)
> I can now put it into an inscription-style transaction and even get a 4x discount
This is not true. Having to use two transactions requires at least 150 bytes of overhead. Removing the spam protection for op_return outputs effectively results in a discount for small inscriptions.
Furthermore, you're saying yourself that you want this change because it makes it easier for you to use the chain for contentious non-bitcoin use cases such as inscribing Nostr keys. However, the reason why you
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1664950834)
> I can now put it into an inscription-style transaction and even get a 4x discount
This is not true. Having to use two transactions requires at least 150 bytes of overhead. Removing the spam protection for op_return outputs effectively results in a discount for small inscriptions.
Furthermore, you're saying yourself that you want this change because it makes it easier for you to use the chain for contentious non-bitcoin use cases such as inscribing Nostr keys. However, the reason why you
...
💬 MarcoFalke commented on pull request "scripted-diff: Specify Python major version explicitly on Windows":
(https://github.com/bitcoin/bitcoin/pull/28213#issuecomment-1665034791)
lgtm ACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
(https://github.com/bitcoin/bitcoin/pull/28213#issuecomment-1665034791)
lgtm ACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283998840)
I much appreciate the input @paplorinc, this was the syntax I was looking for 🙏 I saw the first version of your message and immediately thought about doing the other part ([0, 1, 999, 1000]). You read my mind, this makes very much sense! Also, doing [1, 999] would be quite time consuming because each run lasts about a second so in total it would need 1000 seconds to complete. I'll update the PR to support this version.
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283998840)
I much appreciate the input @paplorinc, this was the syntax I was looking for 🙏 I saw the first version of your message and immediately thought about doing the other part ([0, 1, 999, 1000]). You read my mind, this makes very much sense! Also, doing [1, 999] would be quite time consuming because each run lasts about a second so in total it would need 1000 seconds to complete. I'll update the PR to support this version.
💬 MarcoFalke commented on pull request "refactor: use string_view for passing string literals to Parse{Hash,Hex}":
(https://github.com/bitcoin/bitcoin/pull/28172#issuecomment-1665040232)
lgtm ACK b94581a8acecafe5ffff15692485a5572d1db57c
(https://github.com/bitcoin/bitcoin/pull/28172#issuecomment-1665040232)
lgtm ACK b94581a8acecafe5ffff15692485a5572d1db57c
💬 ajtowns commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1284001844)
Because `ForEachNode` takes a `std::function`, the thread safety annotations on the lambda are dropped, so the compiler can't verify that `ForEachNode` has actually taken all the locks that the lambda expects to have held. Problem is that doing it any other way (eg templates) requires `ForEachNode` to be annotated to require any extra locks that the lambda requires that were already held by the caller.
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1284001844)
Because `ForEachNode` takes a `std::function`, the thread safety annotations on the lambda are dropped, so the compiler can't verify that `ForEachNode` has actually taken all the locks that the lambda expects to have held. Problem is that doing it any other way (eg templates) requires `ForEachNode` to be annotated to require any extra locks that the lambda requires that were already held by the caller.