📝 glozow opened a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227)
Motivated by #29193. Currently, we only log something (non-debug) when we fail to load the file and at the end of importing all the transactions. That means it's hard to tell what's happening if it's taking a long time to load.
This PR adds a maximum of 10 new unconditional log lines:
- When we start to load transactions.
- Our progress percentage when it advances by at least 10% from the last time we logged. Percentage is based on the number of transactions.
If there are lots of transac
...
(https://github.com/bitcoin/bitcoin/pull/29227)
Motivated by #29193. Currently, we only log something (non-debug) when we fail to load the file and at the end of importing all the transactions. That means it's hard to tell what's happening if it's taking a long time to load.
This PR adds a maximum of 10 new unconditional log lines:
- When we start to load transactions.
- Our progress percentage when it advances by at least 10% from the last time we logged. Percentage is based on the number of transactions.
If there are lots of transac
...
🚀 fanquake merged a pull request: "fuzz: Improve fuzzing stability for ellswift_roundtrip harness"
(https://github.com/bitcoin/bitcoin/pull/29219)
(https://github.com/bitcoin/bitcoin/pull/29219)
💬 maflcko commented on pull request "ci: move CMake into base packages":
(https://github.com/bitcoin/bitcoin/pull/29225#issuecomment-1886999559)
lgtm ACK f3ca6db8d349941f6bad6e8b328033222af8f063
(https://github.com/bitcoin/bitcoin/pull/29225#issuecomment-1886999559)
lgtm ACK f3ca6db8d349941f6bad6e8b328033222af8f063
💬 dergoegge commented on pull request "Revert "build: Fix regression in "ARMv8 CRC32 intrinsics" test"":
(https://github.com/bitcoin/bitcoin/pull/29226#issuecomment-1887003190)
cc @hebasto @maflcko
(https://github.com/bitcoin/bitcoin/pull/29226#issuecomment-1887003190)
cc @hebasto @maflcko
💬 dev7ba commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887073369)
In retrospect, I believe what happened is that upon noticing the mempool wasn't loading, I gracefully shut down the node, causing mempool.dat to be overwritten with the limited contents of the mempool.
It might also be advisable not to overwrite the old mempool.dat if it hasn't finished loading completely.
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887073369)
In retrospect, I believe what happened is that upon noticing the mempool wasn't loading, I gracefully shut down the node, causing mempool.dat to be overwritten with the limited contents of the mempool.
It might also be advisable not to overwrite the old mempool.dat if it hasn't finished loading completely.
💬 deyw-bit commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887125546)
Upon examining the entire debug.log, which includes log entries from the past few months - it occasionally took up to nine hours to fully load the mempool on my node.
It's plausible that the mempool.dat could potentially be overwritten by the limited contents of the mempool if a shutdown is initiated before the loading of the mempool is completed.
`grep -E 'Shutdown|mempool|Bitcoin Core version v' /volume1/docker/bitcoin/debug.log`
2023-10-23T11:24:47Z Bitcoin Core version v25.0.0 (rel
...
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887125546)
Upon examining the entire debug.log, which includes log entries from the past few months - it occasionally took up to nine hours to fully load the mempool on my node.
It's plausible that the mempool.dat could potentially be overwritten by the limited contents of the mempool if a shutdown is initiated before the loading of the mempool is completed.
`grep -E 'Shutdown|mempool|Bitcoin Core version v' /volume1/docker/bitcoin/debug.log`
2023-10-23T11:24:47Z Bitcoin Core version v25.0.0 (rel
...
💬 glozow commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887152637)
@deyw-bit can you tell us the specs of your machine?
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887152637)
@deyw-bit can you tell us the specs of your machine?
💬 maflcko commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887158724)
> I gracefully shut down the node, causing mempool.dat to be overwritten with the limited contents of the mempool.
There is a check to retain the old mempool in that case. So if you never get along to writing the new mempool, due to repeated restarts, the new transactions will be discarded.
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887158724)
> I gracefully shut down the node, causing mempool.dat to be overwritten with the limited contents of the mempool.
There is a check to retain the old mempool in that case. So if you never get along to writing the new mempool, due to repeated restarts, the new transactions will be discarded.
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1887170261)
> One can always use a revoked state with max outgoing HTLC outputs=483 to pin.
> So 483 * 33 + 1000, you have ~18k vbytes state, mempool backlog at 20 sat/vb, you can steal ~100 USD payments.
Ok @instagibbs helped me parse what you're trying to say here (thanks) and IIUC you're talking about a scenario in which:
- The counterparties in a channel have negotiated a very high number of maximum in-flight HTLCS, so they have at some point signed a commitment transaction that was 18KvB in size.
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1887170261)
> One can always use a revoked state with max outgoing HTLC outputs=483 to pin.
> So 483 * 33 + 1000, you have ~18k vbytes state, mempool backlog at 20 sat/vb, you can steal ~100 USD payments.
Ok @instagibbs helped me parse what you're trying to say here (thanks) and IIUC you're talking about a scenario in which:
- The counterparties in a channel have negotiated a very high number of maximum in-flight HTLCS, so they have at some point signed a commitment transaction that was 18KvB in size.
...
📝 maflcko opened a pull request: "test: Remove all-lint.py script"
(https://github.com/bitcoin/bitcoin/pull/29228)
Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests.
Fix that by just calling this subset of lint tests in the test runner directly, and remove the now unused `all-lint.py`.
(https://github.com/bitcoin/bitcoin/pull/29228)
Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests.
Fix that by just calling this subset of lint tests in the test runner directly, and remove the now unused `all-lint.py`.
💬 dergoegge commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1887174559)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1887174559)
Concept ACK
🤔 furszy reviewed a pull request: "wallet: Refactor DumpWallet function to accept -dumpfile path argument"
(https://github.com/bitcoin/bitcoin/pull/29223#pullrequestreview-1815601947)
The rationale behind this was to remove the `<common/args.h>` include from dump.cpp and the `ArgsManager` type from `dump.h` (which are missing here). Also, should pass `fs::path` instead of a string.
This cleans up code but.. it was a tiny nit that I would group with other changes. Making tiny refactoring PRs like this isn't really the best (we could have dozens of it meanwhile we only have a handful amount of reviewers).
(https://github.com/bitcoin/bitcoin/pull/29223#pullrequestreview-1815601947)
The rationale behind this was to remove the `<common/args.h>` include from dump.cpp and the `ArgsManager` type from `dump.h` (which are missing here). Also, should pass `fs::path` instead of a string.
This cleans up code but.. it was a tiny nit that I would group with other changes. Making tiny refactoring PRs like this isn't really the best (we could have dozens of it meanwhile we only have a handful amount of reviewers).
💬 deyw-bit commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887179525)
> @deyw-bit can you tell us the specs of your machine?
I am running the Bitcoin Core v25.1 node in Docker on Synology NAS DS216+II (with 8GB RAM).
The node runs in a docker container with OS Debian GNU/Linux 12 (bookworm) with CPU priority set to low and memory limit set to 7 GB.
The Dockerfile contains following:
***
FROM debian:bookworm-slim
RUN apt-get update -y && apt-get upgrade -y && apt-get install apt -y
RUN apt-get install wget sudo vim gpg curl bash -y
RUN wget https://
...
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887179525)
> @deyw-bit can you tell us the specs of your machine?
I am running the Bitcoin Core v25.1 node in Docker on Synology NAS DS216+II (with 8GB RAM).
The node runs in a docker container with OS Debian GNU/Linux 12 (bookworm) with CPU priority set to low and memory limit set to 7 GB.
The Dockerfile contains following:
***
FROM debian:bookworm-slim
RUN apt-get update -y && apt-get upgrade -y && apt-get install apt -y
RUN apt-get install wget sudo vim gpg curl bash -y
RUN wget https://
...
💬 instagibbs commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1887189668)
> what happened 3+ years ago
I have some bad news, I do not.
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1887189668)
> what happened 3+ years ago
I have some bad news, I do not.
💬 glozow commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887190793)
@deyw-bit How long did IBD take on your machine?
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887190793)
@deyw-bit How long did IBD take on your machine?
💬 deyw-bit commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887205310)
> How long did IBD take on your machine?
To the best of my recollection, the IBD took approx. 10 days. However, this was a few years ago. I guess last time I triggered IBD was with bitcoin-core-0.21.
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1887205310)
> How long did IBD take on your machine?
To the best of my recollection, the IBD took approx. 10 days. However, this was a few years ago. I guess last time I triggered IBD was with bitcoin-core-0.21.
💬 t-bast commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1887226712)
Another thing to note about this attack is that the attacker would be losing funds in the process if the attacked node simply spends the revoked commitments outputs (instead of trying to replace it). Whenever one side publishes a revoked commitment, they will lose any funds that they had in the channel (which is at least their channel reserve, thus 1% of the channel). So the attacked node would pay a large on-chain fee, but would also earn the funds that the attacker loses. In the worst case sce
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1887226712)
Another thing to note about this attack is that the attacker would be losing funds in the process if the attacked node simply spends the revoked commitments outputs (instead of trying to replace it). Whenever one side publishes a revoked commitment, they will lose any funds that they had in the channel (which is at least their channel reserve, thus 1% of the channel). So the attacked node would pay a large on-chain fee, but would also earn the funds that the attacker loses. In the worst case sce
...
💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1448932594)
updated in cfdbcd19b32fd63954d7947dcc639aef291fb6b2
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1448932594)
updated in cfdbcd19b32fd63954d7947dcc639aef291fb6b2
👍 alfonsoromanz approved a pull request: "log mempool loading progress"
(https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1815715513)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/29227/commits/6c3ca0cac38f3b9e307cd0522ba9e3b7a2bd05a5
I have not executed the code but I spent time reviewing the loop and the conditions within it, using your example provided in the PR description (401 txs). Looks good to me
(https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1815715513)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/29227/commits/6c3ca0cac38f3b9e307cd0522ba9e3b7a2bd05a5
I have not executed the code but I spent time reviewing the loop and the conditions within it, using your example provided in the PR description (401 txs). Looks good to me
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1887299001)
Rebased on top of adding CMake into all CIs, the newer upnpc / libnatmp / dropping libtool commits, and added a new commit to deduplicate usage of `-mmaxosx-version-min`.
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1887299001)
Rebased on top of adding CMake into all CIs, the newer upnpc / libnatmp / dropping libtool commits, and added a new commit to deduplicate usage of `-mmaxosx-version-min`.
💬 jamesob commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1448969580)
@stickies-v it's not about system resources; it's about allow the end user to articulate what they see. Not a big deal either way, but I think our current scheme is a little wonky if you compare it to more widely deployed systems e.g. the python logging module or log4j.
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1448969580)
@stickies-v it's not about system resources; it's about allow the end user to articulate what they see. Not a big deal either way, but I think our current scheme is a little wonky if you compare it to more widely deployed systems e.g. the python logging module or log4j.