💬 maflcko commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413530272)
Those checks are part of the bash script itself. If there is a `BEGIN`, without a matching `scripted-diff` prefix, or vice-versa, the script should already fail.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413530272)
Those checks are part of the bash script itself. If there is a `BEGIN`, without a matching `scripted-diff` prefix, or vice-versa, the script should already fail.
💬 maflcko commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413532736)
If there are (other) bugs, it would be good to enumerate them, with exact steps to reproduce. Then, it would be good to fix them. Once all bugs are fixed, I presume adding a label won't be needed.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413532736)
If there are (other) bugs, it would be good to enumerate them, with exact steps to reproduce. Then, it would be good to fix them. Once all bugs are fixed, I presume adding a label won't be needed.
💬 maflcko commented on pull request "rpc: Disallow non-matching transactions in combinerawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31091#discussion_r1800909183)
```suggestion
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Transactions to be combined do not match."));
```
nit?
(https://github.com/bitcoin/bitcoin/pull/31091#discussion_r1800909183)
```suggestion
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Transactions to be combined do not match."));
```
nit?
✅ instagibbs closed a pull request: "rpc: Disallow non-matching transactions in combinerawtransaction"
(https://github.com/bitcoin/bitcoin/pull/31091)
(https://github.com/bitcoin/bitcoin/pull/31091)
💬 instagibbs commented on pull request "rpc: Disallow non-matching transactions in combinerawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31091#issuecomment-2413539804)
nevermind, pre-segwit is lame, im outta here
(https://github.com/bitcoin/bitcoin/pull/31091#issuecomment-2413539804)
nevermind, pre-segwit is lame, im outta here
💬 achow101 commented on pull request "rpc: Disallow non-matching transactions in combinerawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31091#discussion_r1800915326)
For txs with pre-segwit and nested segwit inputs, txids will not match because the scriptSigs will be different. This check is actually more involved as the transactions need to have scriptSigs stripped.
(https://github.com/bitcoin/bitcoin/pull/31091#discussion_r1800915326)
For txs with pre-segwit and nested segwit inputs, txids will not match because the scriptSigs will be different. This check is actually more involved as the transactions need to have scriptSigs stripped.
💬 maflcko commented on pull request "lint: commit-script-check.sh: echo to stderr":
(https://github.com/bitcoin/bitcoin/pull/31063#issuecomment-2413547989)
This makes it easier to rebase a scripted-diff. Now, it is trivial to run it and redirect the failing diff and apply it later. Previously it was required to be manually edited before applying.
(https://github.com/bitcoin/bitcoin/pull/31063#issuecomment-2413547989)
This makes it easier to rebase a scripted-diff. Now, it is trivial to run it and redirect the failing diff and apply it later. Previously it was required to be manually edited before applying.
💬 l0rinc commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413563398)
> If there is a BEGIN, without a matching scripted-diff
Which can be replaced with `BEGlN` (i.e. a lowercase L instead of an I or any other unicode trickery) and reviewers would be fooled into thinking that the scripts were validated by the CI.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413563398)
> If there is a BEGIN, without a matching scripted-diff
Which can be replaced with `BEGlN` (i.e. a lowercase L instead of an I or any other unicode trickery) and reviewers would be fooled into thinking that the scripts were validated by the CI.
📝 brunoerg opened a pull request: "doc: fuzz: remove Honggfuzz NetDriver instructions"
(https://github.com/bitcoin/bitcoin/pull/31092)
Remove Honggfuzz NetDriver instructions from the documentation since it has not been useful for us. See https://github.com/bitcoin/bitcoin/issues/30957 and https://github.com/bitcoin/bitcoin/pull/31012.
(https://github.com/bitcoin/bitcoin/pull/31092)
Remove Honggfuzz NetDriver instructions from the documentation since it has not been useful for us. See https://github.com/bitcoin/bitcoin/issues/30957 and https://github.com/bitcoin/bitcoin/pull/31012.
✅ brunoerg closed a pull request: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012)
(https://github.com/bitcoin/bitcoin/pull/31012)
💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2413564192)
Closing this in favor of #31092
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2413564192)
Closing this in favor of #31092
💬 maflcko commented on pull request "doc: fuzz: remove Honggfuzz NetDriver instructions":
(https://github.com/bitcoin/bitcoin/pull/31092#issuecomment-2413577707)
lgtm ACK d823ba6e20bd5fd312b65cf8f71c407c1861793e
Should be trivial to add back, if there is need.
(https://github.com/bitcoin/bitcoin/pull/31092#issuecomment-2413577707)
lgtm ACK d823ba6e20bd5fd312b65cf8f71c407c1861793e
Should be trivial to add back, if there is need.
🤔 marcofleon reviewed a pull request: "doc: fuzz: remove Honggfuzz NetDriver instructions"
(https://github.com/bitcoin/bitcoin/pull/31092#pullrequestreview-2369004044)
ACK d823ba6e20bd5fd312b65cf8f71c407c1861793e
(https://github.com/bitcoin/bitcoin/pull/31092#pullrequestreview-2369004044)
ACK d823ba6e20bd5fd312b65cf8f71c407c1861793e
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2413603230)
re-ACK 30f30bbcaae67d725360c3e7b83207904e1a9b19
The recent push was rebase on master after https://github.com/bitcoin/bitcoin/pull/30857
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2413603230)
re-ACK 30f30bbcaae67d725360c3e7b83207904e1a9b19
The recent push was rebase on master after https://github.com/bitcoin/bitcoin/pull/30857
✅ fanquake closed a pull request: "guix: Use LTO to build releases"
(https://github.com/bitcoin/bitcoin/pull/25391)
(https://github.com/bitcoin/bitcoin/pull/25391)
💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2413605487)
Going to come back to this after static builds.
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2413605487)
Going to come back to this after static builds.
💬 darosior commented on issue "Unexpected behaviour when using `sortedmulti_a` descriptor":
(https://github.com/bitcoin/bitcoin/issues/30518#issuecomment-2413607192)
@sipa i agree it's a point in favour of ordering (or randomizing) the order of keys. But i don't think it's a reason to have it in the descriptor language itself. The application can just randomize the order before creating the descriptor.
(https://github.com/bitcoin/bitcoin/issues/30518#issuecomment-2413607192)
@sipa i agree it's a point in favour of ordering (or randomizing) the order of keys. But i don't think it's a reason to have it in the descriptor language itself. The application can just randomize the order before creating the descriptor.
🚀 fanquake merged a pull request: "doc: fuzz: remove Honggfuzz NetDriver instructions"
(https://github.com/bitcoin/bitcoin/pull/31092)
(https://github.com/bitcoin/bitcoin/pull/31092)
👍 TheCharlatan approved a pull request: "depends: sqlite 3.46.1"
(https://github.com/bitcoin/bitcoin/pull/29991#pullrequestreview-2369019884)
ACK def6dd0c597f2c7b0c55910792e646b8ff93e36c
(https://github.com/bitcoin/bitcoin/pull/29991#pullrequestreview-2369019884)
ACK def6dd0c597f2c7b0c55910792e646b8ff93e36c
💬 darosior commented on issue "Unexpected behaviour when using `sortedmulti_a` descriptor":
(https://github.com/bitcoin/bitcoin/issues/30518#issuecomment-2413624990)
This is also very specific to one usecase. If the descriptor used by the application is `c:or_i(pk_k(A),pk_k(B))` does this mean the language needs a `sorted_or_i` (same goes for all combination fragments)? In my opinion no, it just means the application shouldn't leak the order by always placing the key for the same role in the same spot.
(https://github.com/bitcoin/bitcoin/issues/30518#issuecomment-2413624990)
This is also very specific to one usecase. If the descriptor used by the application is `c:or_i(pk_k(A),pk_k(B))` does this mean the language needs a `sorted_or_i` (same goes for all combination fragments)? In my opinion no, it just means the application shouldn't leak the order by always placing the key for the same role in the same spot.