🤔 theuni requested changes to a pull request: "Bugfix: Skip tests for tools not being built"
(https://github.com/bitcoin/bitcoin/pull/23027#pullrequestreview-1413433373)
@luke-jr If you're still interested in this, I agree with the previous reviewers:
- PR needs a description, if only for posterity
- As @fanquake mentioned, `ENABLE_BITCOIN_UTIL` could be used here.
I'm not at all familiar with the test framework, but this looks a lot like a re-implementation of something that's already cleanly abstracted. What exactly isn't getting correctly skipped?
(https://github.com/bitcoin/bitcoin/pull/23027#pullrequestreview-1413433373)
@luke-jr If you're still interested in this, I agree with the previous reviewers:
- PR needs a description, if only for posterity
- As @fanquake mentioned, `ENABLE_BITCOIN_UTIL` could be used here.
I'm not at all familiar with the test framework, but this looks a lot like a re-implementation of something that's already cleanly abstracted. What exactly isn't getting correctly skipped?
📝 MarcoFalke opened a pull request: " ci: Remove CI_EXEC bloat in test/06_script_b.sh "
(https://github.com/bitcoin/bitcoin/pull/27573)
`CI_EXEC` has many issues:
* It is roughly equivalent to `bash -c "$*"`, meaning that the full command will be treated as a single string, ignoring tokens.
* It must be put in front of (almost) every command, making it easy to forget, hard to debug the resulting failure, and the code verbose.
Fix all issues in one script by removing it.
(https://github.com/bitcoin/bitcoin/pull/27573)
`CI_EXEC` has many issues:
* It is roughly equivalent to `bash -c "$*"`, meaning that the full command will be treated as a single string, ignoring tokens.
* It must be put in front of (almost) every command, making it easy to forget, hard to debug the resulting failure, and the code verbose.
Fix all issues in one script by removing it.
🚀 fanquake merged a pull request: "test: add coverage to rpc_scantxoutset.py"
(https://github.com/bitcoin/bitcoin/pull/27422)
(https://github.com/bitcoin/bitcoin/pull/27422)
📝 MarcoFalke converted_to_draft a pull request: "ci: Run iwyu on all src files"
(https://github.com/bitcoin/bitcoin/pull/27571)
This makes it easier to look at the CI output of a file without having to manually add it first to the list.
(https://github.com/bitcoin/bitcoin/pull/27571)
This makes it easier to look at the CI output of a file without having to manually add it first to the list.
💬 mzumsande commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535074436)
> Hi facing similar issue. I'm running an rpc node.
Also 22.0? Which value of `rpcthreads` are you using?
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535074436)
> Hi facing similar issue. I'm running an rpc node.
Also 22.0? Which value of `rpcthreads` are you using?
🤔 Sosyetekadir reviewed a pull request: "refactor: Remove need to pass chainparams to BlockManager methods"
(https://github.com/bitcoin/bitcoin/pull/27570#pullrequestreview-1413526067)
Sosyete
(https://github.com/bitcoin/bitcoin/pull/27570#pullrequestreview-1413526067)
Sosyete
💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1185280732)
or to be lazier, just set the initial utxos' prevout.n index to some impossibly high numbers to ever be generated
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1185280732)
or to be lazier, just set the initial utxos' prevout.n index to some impossibly high numbers to ever be generated
💬 Sosyetekadir commented on pull request "refactor: Remove need to pass chainparams to BlockManager methods":
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535096000)
Ben ve sen
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535096000)
Ben ve sen
💬 sangaman commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535121219)
FYI, I am now using v24.0.1 with `rpcthreads=64` as mentioned above, and I still see this issue although it still appears to be slow and unpredictable. I just checked and bitcoind is currently using close to 4 GB RAM, which is a lot higher than it would be if I'd restarted the node within the past few hours or even days.
I increased the `rpcthreads` setting to make sure bitcoind plays nice with lnd, but I don't expect that I actually need it that high. I can switch to the default `rpcthreads`
...
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535121219)
FYI, I am now using v24.0.1 with `rpcthreads=64` as mentioned above, and I still see this issue although it still appears to be slow and unpredictable. I just checked and bitcoind is currently using close to 4 GB RAM, which is a lot higher than it would be if I'd restarted the node within the past few hours or even days.
I increased the `rpcthreads` setting to make sure bitcoind plays nice with lnd, but I don't expect that I actually need it that high. I can switch to the default `rpcthreads`
...
💬 willcl-ark commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535130495)
Thanks @sangaman. Additionally, do you have any estimates on how many rpcs you are making per time to the node?
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535130495)
Thanks @sangaman. Additionally, do you have any estimates on how many rpcs you are making per time to the node?
💬 sangaman commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535138024)
Hmm I'm not exactly sure. This bitcoind node is primarily used to serve an lnd node as well as an electrum server and mempool.space block explorer, although the latter two get very light usage aside from staying in sync with the chain. If you think it'd be helpful I can try to turn on debug logs and see how many rpc calls are made over a certain period of time.
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1535138024)
Hmm I'm not exactly sure. This bitcoind node is primarily used to serve an lnd node as well as an electrum server and mempool.space block explorer, although the latter two get very light usage aside from staying in sync with the chain. If you think it'd be helpful I can try to turn on debug logs and see how many rpc calls are made over a certain period of time.
💬 MarcoFalke commented on pull request "refactor: Remove need to pass chainparams to BlockManager methods":
(https://github.com/bitcoin/bitcoin/pull/27570#discussion_r1185310001)
I guess I disagree, but made private for now to close the discussion for now
(https://github.com/bitcoin/bitcoin/pull/27570#discussion_r1185310001)
I guess I disagree, but made private for now to close the discussion for now
💬 dergoegge commented on pull request "refactor: Remove need to pass chainparams to BlockManager methods":
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535159628)
What about these three:
https://github.com/bitcoin/bitcoin/blob/fa5d7c39eb992467e43b12db213b27913374fb83/src/node/blockstorage.h#L237-L239
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535159628)
What about these three:
https://github.com/bitcoin/bitcoin/blob/fa5d7c39eb992467e43b12db213b27913374fb83/src/node/blockstorage.h#L237-L239
📝 dergoegge opened a pull request: "doc: Add post branch-off note about fuzz input pruning"
(https://github.com/bitcoin/bitcoin/pull/27574)
(https://github.com/bitcoin/bitcoin/pull/27574)
💬 dergoegge commented on pull request "doc: Add post branch-off note about fuzz input pruning":
(https://github.com/bitcoin/bitcoin/pull/27574#issuecomment-1535164393)
cc @MarcoFalke
(https://github.com/bitcoin/bitcoin/pull/27574#issuecomment-1535164393)
cc @MarcoFalke
📝 hebasto opened a pull request: "Introduce platform-agnostic `ALWAYS_INLINE` macro"
(https://github.com/bitcoin/bitcoin/pull/27575)
Split from https://github.com/bitcoin/bitcoin/pull/24773 as requested in https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1534954977.
(https://github.com/bitcoin/bitcoin/pull/27575)
Split from https://github.com/bitcoin/bitcoin/pull/24773 as requested in https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1534954977.
💬 hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1535175516)
> Sorry, yes, I meant to factor out those changes into a separate PR.
Thanks! Done in https://github.com/bitcoin/bitcoin/pull/27575.
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1535175516)
> Sorry, yes, I meant to factor out those changes into a separate PR.
Thanks! Done in https://github.com/bitcoin/bitcoin/pull/27575.
💬 MarcoFalke commented on pull request "refactor: Remove need to pass chainparams to BlockManager methods":
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535178750)
They are not members of the `class BlockManager`, so fixing them is non-trivial and should happen in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535178750)
They are not members of the `class BlockManager`, so fixing them is non-trivial and should happen in a follow-up.
💬 dergoegge commented on pull request "refactor: Remove need to pass chainparams to BlockManager methods":
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535182923)
> They are not members of the class BlockManager, so fixing them is non-trivial and should happen in a follow-up.
Ugh for some reason I thought #27125 was already merged🤦
(https://github.com/bitcoin/bitcoin/pull/27570#issuecomment-1535182923)
> They are not members of the class BlockManager, so fixing them is non-trivial and should happen in a follow-up.
Ugh for some reason I thought #27125 was already merged🤦
💬 hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1535184586)
> > Sorry, yes, I meant to factor out those changes into a separate PR.
>
> Thanks! Done in #27575.
Rebased on top of the #27575 and drafted. Please review #27575 first.
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1535184586)
> > Sorry, yes, I meant to factor out those changes into a separate PR.
>
> Thanks! Done in #27575.
Rebased on top of the #27575 and drafted. Please review #27575 first.