💬 l0rinc commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844690315)
These changes caught many people off guard, and the perceived urgency understandably triggered strong reactions.
For proposals this contentious, it helps to start with an educational phase: GitHub issues, blog posts, podcasts, conference talks, and perhaps even a draft BIP, so the rationale is clear and both positions are properly steel-manned before opening a PR (ideally only after the subject has become mundane).
Without that groundwork, it is easy to understand why some view the process as
...
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844690315)
These changes caught many people off guard, and the perceived urgency understandably triggered strong reactions.
For proposals this contentious, it helps to start with an educational phase: GitHub issues, blog posts, podcasts, conference talks, and perhaps even a draft BIP, so the rationale is clear and both positions are properly steel-manned before opening a PR (ideally only after the subject has become mundane).
Without that groundwork, it is easy to understand why some view the process as
...
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2070180650)
Yes, agree, just noted that this introduces a drift - which, as you also pointed out, makes sense.
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2070180650)
Yes, agree, just noted that this introduces a drift - which, as you also pointed out, makes sense.
💬 Sjors commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844699289)
@l0rinc there's a similar proposal on the mailinglist: https://gnusha.org/pi/bitcoindev/ni6KOcP4nfh6qeB7hEprfzZJuOtBTK6g7rv1L3IRa2TIXq3XUX-7MWGPCHbBATaxOdgbzEQsZmJhId5a4BkGmRabCxSsN24JGEiR4r67D9Y=@pm.me/
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844699289)
@l0rinc there's a similar proposal on the mailinglist: https://gnusha.org/pi/bitcoindev/ni6KOcP4nfh6qeB7hEprfzZJuOtBTK6g7rv1L3IRa2TIXq3XUX-7MWGPCHbBATaxOdgbzEQsZmJhId5a4BkGmRabCxSsN24JGEiR4r67D9Y=@pm.me/
💬 Sjors commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844704124)
@laanwj can you add some testing hints to the PR description? E.g. is there a way to deliberately trigger the potential bug that this fixes?
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844704124)
@laanwj can you add some testing hints to the PR description? E.g. is there a way to deliberately trigger the potential bug that this fixes?
🤔 janb84 reviewed a pull request: "mining: rename gbt_force and gbt_force_name"
(https://github.com/bitcoin/bitcoin/pull/32386#pullrequestreview-2809667848)
ACK [0750249](https://github.com/bitcoin/bitcoin/commit/0750249289c092fc8e2e29669fec73a58b873767)
The rename of the variables results in less confusion of the intention of the code. And the extra comment explaining the workings of the ! prefix helps to clarify intentions/code even further.
(https://github.com/bitcoin/bitcoin/pull/32386#pullrequestreview-2809667848)
ACK [0750249](https://github.com/bitcoin/bitcoin/commit/0750249289c092fc8e2e29669fec73a58b873767)
The rename of the variables results in less confusion of the intention of the code. And the extra comment explaining the workings of the ! prefix helps to clarify intentions/code even further.
📝 vasild opened a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394)
The only case of a recursive lock was a nested `ForNode()` call to trim
the size of `lNodesAnnouncingHeaderAndIDs` to `<= 3`. This need not be
nested, so take it out.
Before:
```
// we know we will push one new element at the back
if (size >= 3) pop from front
push at the back
```
After:
```
// maybe push at the back
if (size > 3) pop from the front
```
`lNodesAnnouncingHeaderAndIDs` is protected by `cs_main` which is locked
during the entire operation.
Partially resolves
...
(https://github.com/bitcoin/bitcoin/pull/32394)
The only case of a recursive lock was a nested `ForNode()` call to trim
the size of `lNodesAnnouncingHeaderAndIDs` to `<= 3`. This need not be
nested, so take it out.
Before:
```
// we know we will push one new element at the back
if (size >= 3) pop from front
push at the back
```
After:
```
// maybe push at the back
if (size > 3) pop from the front
```
`lNodesAnnouncingHeaderAndIDs` is protected by `cs_main` which is locked
during the entire operation.
Partially resolves
...
💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844721484)
> [@laanwj can you add some testing hints to the PR description? E.g. is there a way to deliberately trigger the potential bug that this fixes?
Sure: The simplest way to see if everything properly gets closed is to list `/proc/self/fd` at the beginning in the signer, as in https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2830252505 .
Alternatively, if you really want to create a bug, you could actively try to mess around with file descriptors that are already open at the start o
...
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844721484)
> [@laanwj can you add some testing hints to the PR description? E.g. is there a way to deliberately trigger the potential bug that this fixes?
Sure: The simplest way to see if everything properly gets closed is to list `/proc/self/fd` at the beginning in the signer, as in https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2830252505 .
Alternatively, if you really want to create a bug, you could actively try to mess around with file descriptors that are already open at the start o
...
💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844725476)
Made `m_nodes_mutex` non-recursive in https://github.com/bitcoin/bitcoin/pull/32394 which is this PR + one more commit.
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2844725476)
Made `m_nodes_mutex` non-recursive in https://github.com/bitcoin/bitcoin/pull/32394 which is this PR + one more commit.
💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2070201366)
Could you please check updated test cases.
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2070201366)
Could you please check updated test cases.
💬 willcl-ark commented on issue "test: interface_usdt_net.py failure under --valgrind":
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2844745010)
I was not able to get this to reproduce using any of the sanitizers.
Connecting with `gdb`, shows all pointers and variables as valid at the callsite. I agree that `ConnectionTypeAsString` seems the natural culprit as it returns a temporary string, but even patching that to return `const *char`,still saw `SIGTRAP` raised.
Stepping through the tracepoint saw it execute without error, so it does seem to be a race/lifetime issue, but not really too sure where else to go from here.
(https://github.com/bitcoin/bitcoin/issues/32374#issuecomment-2844745010)
I was not able to get this to reproduce using any of the sanitizers.
Connecting with `gdb`, shows all pointers and variables as valid at the callsite. I agree that `ConnectionTypeAsString` seems the natural culprit as it returns a temporary string, but even patching that to return `const *char`,still saw `SIGTRAP` raised.
Stepping through the tracepoint saw it execute without error, so it does seem to be a race/lifetime issue, but not really too sure where else to go from here.
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844755692)
`15cc989538...8b93e0894f`: fix CI clang-tidy
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844755692)
`15cc989538...8b93e0894f`: fix CI clang-tidy
💬 dergoegge commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2844766695)
ACK 47e713ea5a96d3bb2ddd64c8a87607e5ccea8c72
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2844766695)
ACK 47e713ea5a96d3bb2ddd64c8a87607e5ccea8c72
🤔 sipa reviewed a pull request: "crypto: Use secure_allocator for `AES256_ctx`"
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-2809732175)
utACK 591764f6170d6f74d7eebb5fec1cbf5b912098a4, just coding style nits
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-2809732175)
utACK 591764f6170d6f74d7eebb5fec1cbf5b912098a4, just coding style nits
💬 sipa commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070233390)
Coding style nit: `} else {`.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070233390)
Coding style nit: `} else {`.
💬 sipa commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070235644)
Coding style nit: space after `if`.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2070235644)
Coding style nit: space after `if`.
💬 instagibbs commented on pull request "test: test MAX_SCRIPT_SIZE for block validity":
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2844817333)
@Christewart that's a different rule, if you add a test PR and it's not already covered I'll ack it!
(https://github.com/bitcoin/bitcoin/pull/32304#issuecomment-2844817333)
@Christewart that's a different rule, if you add a test PR and it's not already covered I'll ack it!
🤔 Bonobirho99 reviewed a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-2809778055)
- [ ] @smola 
# the solution SIDRA cap market on Mainn list auditor internal server it is maljefayri ceo
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-2809778055)
- [ ] @smola 
# the solution SIDRA cap market on Mainn list auditor internal server it is maljefayri ceo
💬 Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2844850180)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d
🌲 🪚
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2844850180)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d
🌲 🪚
👍 Sjors approved a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2809792896)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d 🌲 🪚
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2809792896)
ACK 1c0d89358e12fc871e99c8304d5cb50965cf7b9d 🌲 🪚
💬 sonnyxsm commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844905503)
this is not your wastebucket.
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844905503)
this is not your wastebucket.