📝 fanquake opened a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282)
This reverts commit ba30a5407e065e9d6dd037351e83f56a43f38f19.
We no-longer support Python 3.8, so remove the monkey patching.
(https://github.com/bitcoin/bitcoin/pull/30282)
This reverts commit ba30a5407e065e9d6dd037351e83f56a43f38f19.
We no-longer support Python 3.8, so remove the monkey patching.
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1638246968)
Point taken for moving the explicit lock after this check, since the lock is taken in `IsInitialBlockDownload()`.
However, this check only runs once every 30 seconds. I don't see how it could possibly affect responsiveness of the software. It is a very fast check I would assume on the order of microseconds every 30 seconds.
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1638246968)
Point taken for moving the explicit lock after this check, since the lock is taken in `IsInitialBlockDownload()`.
However, this check only runs once every 30 seconds. I don't see how it could possibly affect responsiveness of the software. It is a very fast check I would assume on the order of microseconds every 30 seconds.
💬 fanquake commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2165727772)
Looks like the Apple Clang shipping with Xcode 15, is based on Clang 15. Xcode 16 (still in beta) will ship with a Apple Clang based on Clang 16.
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2165727772)
Looks like the Apple Clang shipping with Xcode 15, is based on Clang 15. Xcode 16 (still in beta) will ship with a Apple Clang based on Clang 16.
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262708)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262708)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262828)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262828)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262918)
added at beginning of function
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638262918)
added at beginning of function
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263066)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263066)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263169)
done in this and one more location
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263169)
done in this and one more location
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263303)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1638263303)
done
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2165748009)
@theStack part of splitting out the diagram stuff to a prep PR is that you can just assume it means "economically rational to replace" :+1:
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2165748009)
@theStack part of splitting out the diagram stuff to a prep PR is that you can just assume it means "economically rational to replace" :+1:
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1638266045)
> Isn't this going to always reschedule the task on the first run?
Yes, but do you think this is a problem? It just makes sure the node has not connected any nodes for at least 30 seconds.
> Also, the active height refers to the latest connected block. It doesn't tell us we are up-to-date with the network; To know if we are sync, should use the best known header or call to the ApproximateBestBlockDepth() function.
Doesn't the fact that `IsInitialBlockDownload()` returns false make this
...
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1638266045)
> Isn't this going to always reschedule the task on the first run?
Yes, but do you think this is a problem? It just makes sure the node has not connected any nodes for at least 30 seconds.
> Also, the active height refers to the latest connected block. It doesn't tell us we are up-to-date with the network; To know if we are sync, should use the best known header or call to the ApproximateBestBlockDepth() function.
Doesn't the fact that `IsInitialBlockDownload()` returns false make this
...
📝 theuni opened a pull request: "upnp: fix build with miniupnpc 2.1.8"
(https://github.com/bitcoin/bitcoin/pull/30283)
Fixes https://github.com/bitcoin/bitcoin/issues/30266
Miniupnpc 2.1.8 [changed the function signature of `UPNP_GetValidIGD`](https://github.com/miniupnp/miniupnp/commit/c0a50ce33e3b99ce8a96fd43049bb5b53ffac62f#diff-5a0d7cff00628c2c64a617edb347c0f283e3a75e7df910e7e8438fc6db23f610R122) without taking much care with the abi :(
This is the minimal change to cope with that. Also included in this PR is a temporary bump to 2.1.8 to verify that it builds correctly. I'm happy to revert that and dis
...
(https://github.com/bitcoin/bitcoin/pull/30283)
Fixes https://github.com/bitcoin/bitcoin/issues/30266
Miniupnpc 2.1.8 [changed the function signature of `UPNP_GetValidIGD`](https://github.com/miniupnp/miniupnp/commit/c0a50ce33e3b99ce8a96fd43049bb5b53ffac62f#diff-5a0d7cff00628c2c64a617edb347c0f283e3a75e7df910e7e8438fc6db23f610R122) without taking much care with the abi :(
This is the minimal change to cope with that. Also included in this PR is a temporary bump to 2.1.8 to verify that it builds correctly. I'm happy to revert that and dis
...
💬 ismaelsadeeq commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1638282582)
> Therefore as an "internal error", TX_MISSING_INPUTS (for any reason) it felt more accurate to me to leave as is?
I was reviewing this based on the PR rationale in the OP. You stated that "a transaction's inputs are missing from the UTXO set, which could also be due to all inputs having already been spent (MISSING_INPUTS -> INPUTS_MISSING_OR_SPENT)." That's why I suggested that `TxValidationResult` one should be changed just for uniformity. But Internally, you're right `TX_MISSING_INPUTS` i
...
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1638282582)
> Therefore as an "internal error", TX_MISSING_INPUTS (for any reason) it felt more accurate to me to leave as is?
I was reviewing this based on the PR rationale in the OP. You stated that "a transaction's inputs are missing from the UTXO set, which could also be due to all inputs having already been spent (MISSING_INPUTS -> INPUTS_MISSING_OR_SPENT)." That's why I suggested that `TxValidationResult` one should be changed just for uniformity. But Internally, you're right `TX_MISSING_INPUTS` i
...
💬 theuni commented on pull request "upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2165781468)
Whoops, fixed typo. s/2.1.8/2.2.8/ in several places.
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2165781468)
Whoops, fixed typo. s/2.1.8/2.2.8/ in several places.
💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2165812047)
reACK https://github.com/bitcoin/bitcoin/pull/30111/commits/0e0c422aedd4009ab34eca127e4904d15e81f5be
just a rebase
reviewed via `git range-diff master a9a6de5 0e0c422`
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2165812047)
reACK https://github.com/bitcoin/bitcoin/pull/30111/commits/0e0c422aedd4009ab34eca127e4904d15e81f5be
just a rebase
reviewed via `git range-diff master a9a6de5 0e0c422`
👍 TheCharlatan approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2115950869)
Re-ACK 260f8da71a35232d859d7705861fc1a88bfbbe81
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2115950869)
Re-ACK 260f8da71a35232d859d7705861fc1a88bfbbe81
👍 theuni approved a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/30281#pullrequestreview-2115953719)
utACK 95812d912b6335caa7af2a084d84447fb4aad156
(https://github.com/bitcoin/bitcoin/pull/30281#pullrequestreview-2115953719)
utACK 95812d912b6335caa7af2a084d84447fb4aad156
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2165900439)
Thanks for the review, and for the suggestions. I might add that later to this PR or as a followup.
Trivial rebase after #29015.
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2165900439)
Thanks for the review, and for the suggestions. I might add that later to this PR or as a followup.
Trivial rebase after #29015.
🤔 ismaelsadeeq reviewed a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2116107521)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2116107521)
Concept ACK
👍 BrandonOdiwuor approved a pull request: "test: cover more errors for `signrawtransactionwithkey` RPC"
(https://github.com/bitcoin/bitcoin/pull/30278#pullrequestreview-2116157106)
Code Review ACK e2779ce98b39e14cada08a654928e798436f5a46
(https://github.com/bitcoin/bitcoin/pull/30278#pullrequestreview-2116157106)
Code Review ACK e2779ce98b39e14cada08a654928e798436f5a46