💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391301747)
Makes sense to me. I'm in favor of returning all the information that we can to the user
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391301747)
Makes sense to me. I'm in favor of returning all the information that we can to the user
💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808457492)
@sipa As far as I understand having the information of UTXOs has nothing to do with pruned mode. Don't pruned nodes store the whole UTXO set? (I can guess the problem can occur for pruned mode when a reorg happen and the old data to restore is pruned, but I'm not sure this is the case)
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808457492)
@sipa As far as I understand having the information of UTXOs has nothing to do with pruned mode. Don't pruned nodes store the whole UTXO set? (I can guess the problem can occur for pruned mode when a reorg happen and the old data to restore is pruned, but I'm not sure this is the case)
💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808460629)
@Riahiamirreza Correct, but `TxToUniv` does not (currently) get information about the UTXO set. There is nothing impossible about this problem, it's just more than a trivial change to that function.
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808460629)
@Riahiamirreza Correct, but `TxToUniv` does not (currently) get information about the UTXO set. There is nothing impossible about this problem, it's just more than a trivial change to that function.
💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808464414)
@Riahiamirreza Actually, ignore what I said about the UTXO set; that's irrelevant. `getrawtransaction` works for confirmed transactions, and the UTXOs such transactions have spent are no longer unspent, and thus no longer in the UTXO set.
You need access to the spentness information, and that is only available on non-pruned nodes. Thankfully, `TxToUniv` already gets that information (through the undo structure passed to it).
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808464414)
@Riahiamirreza Actually, ignore what I said about the UTXO set; that's irrelevant. `getrawtransaction` works for confirmed transactions, and the UTXOs such transactions have spent are no longer unspent, and thus no longer in the UTXO set.
You need access to the spentness information, and that is only available on non-pruned nodes. Thankfully, `TxToUniv` already gets that information (through the undo structure passed to it).
👍 hebasto approved a pull request: "build: remove `-bind_at_load` usage"
(https://github.com/bitcoin/bitcoin/pull/28783#pullrequestreview-1727666140)
ACK 3c61c60b90db1b6a77b3804784430fcd57b447b6, tested on macOS Sonoma 14.1.1 (23B81, Apple M1) and Ubuntu 23.10 (cross-compiling for macOS). Also I've verified the actual diff in the `libtool` script.
(https://github.com/bitcoin/bitcoin/pull/28783#pullrequestreview-1727666140)
ACK 3c61c60b90db1b6a77b3804784430fcd57b447b6, tested on macOS Sonoma 14.1.1 (23B81, Apple M1) and Ubuntu 23.10 (cross-compiling for macOS). Also I've verified the actual diff in the `libtool` script.
💬 achow101 commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1808484775)
re-ACK fa6b053b5c964fb35935fa994cb782c0731a56f8
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1808484775)
re-ACK fa6b053b5c964fb35935fa994cb782c0731a56f8
📝 maflcko opened a pull request: "lint: Report all lint errors instead of early exit"
(https://github.com/bitcoin/bitcoin/pull/28862)
`all-lint.py` currently collects all failures. However, the `06_script.sh` does not, since July this year (https://github.com/bitcoin/bitcoin/pull/28103#discussion_r1268115806).
Fix this by printing all failures before exiting.
Can be tested by modifying (for example) two subtrees in the same commit and then running the linters.
(https://github.com/bitcoin/bitcoin/pull/28862)
`all-lint.py` currently collects all failures. However, the `06_script.sh` does not, since July this year (https://github.com/bitcoin/bitcoin/pull/28103#discussion_r1268115806).
Fix this by printing all failures before exiting.
Can be tested by modifying (for example) two subtrees in the same commit and then running the linters.
🚀 achow101 merged a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207)
(https://github.com/bitcoin/bitcoin/pull/28207)
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391354199)
Is using txid intentional here?
If you use txid, `BroadcastTransaction` will rebroadcast the mempool tx if the result was `DIFFERENT_WITNESS`.
If you use wtxid, we skip it.
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391354199)
Is using txid intentional here?
If you use txid, `BroadcastTransaction` will rebroadcast the mempool tx if the result was `DIFFERENT_WITNESS`.
If you use wtxid, we skip it.
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391348626)
Comment says "Everything in the package should be submitted" but you're only checking that the results exist in `m_tx_results`. Do you want to check that `mempool.exists(wtxid)` as well?
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391348626)
Comment says "Everything in the package should be submitted" but you're only checking that the results exist in `m_tx_results`. Do you want to check that `mempool.exists(wtxid)` as well?
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391359467)
"all transactions were submitted" is not necessarily true anymore, no?
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391359467)
"all transactions were submitted" is not necessarily true anymore, no?
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391367243)
wtxid
```suggestion
std::map<Wtxid, std::optional<std::string>> wtxid_to_error_strings;
```
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391367243)
wtxid
```suggestion
std::map<Wtxid, std::optional<std::string>> wtxid_to_error_strings;
```
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391368212)
also, why have this intermediate map? can't you do this in the loop at the end that populates `rpc_result`?
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391368212)
also, why have this intermediate map? can't you do this in the loop at the end that populates `rpc_result`?
🚀 fanquake merged a pull request: "guix: update time-machine"
(https://github.com/bitcoin/bitcoin/pull/28580)
(https://github.com/bitcoin/bitcoin/pull/28580)
👋 fanquake's pull request is ready for review: "guix: update signapple (drop macho & altgraph)"
(https://github.com/bitcoin/bitcoin/pull/28859)
(https://github.com/bitcoin/bitcoin/pull/28859)
💬 glozow commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391386220)
Prefer using underscores https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184510850
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391386220)
Prefer using underscores https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184510850
📝 ismaelsadeeq opened a pull request: "wallet: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863)
- Requested in [#28391 comment](https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382997719)
* The error message is static when a new transaction is created and package limit is reached.
`Transaction has too long of a mempool chain`
While the [`CTxMempool::CheckPackageLimits`](https://github.com/bitcoin/bitcoin/blob/5800c558eb5efb4839ed00d6967e43306d68e1c3/src/txmempool.cpp#L199) provide explicit information about the error message.
* This PR updates [`checkChainLimits`](https:/
...
(https://github.com/bitcoin/bitcoin/pull/28863)
- Requested in [#28391 comment](https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382997719)
* The error message is static when a new transaction is created and package limit is reached.
`Transaction has too long of a mempool chain`
While the [`CTxMempool::CheckPackageLimits`](https://github.com/bitcoin/bitcoin/blob/5800c558eb5efb4839ed00d6967e43306d68e1c3/src/txmempool.cpp#L199) provide explicit information about the error message.
* This PR updates [`checkChainLimits`](https:/
...
⚠️ dergoegge opened an issue: "build: Compiling with `-mno-sse4.1` does not fail the sse4.1 instrinsics check"
(https://github.com/bitcoin/bitcoin/issues/28864)
I would have expected that `CXXFLAGS="-mno-sse4.1" ./configure` would make the sse4.1 intrinsics check fail but it doesn't:
```
configure:23377: checking for SSE4.1 intrinsics
configure:23396: clang++ -std=c++17 -c -msse4.1 -mno-sse4.1 conftest.cpp >&5
configure:23396: $? = 0
configure:23398: result: yes
```
Not sure what is going on here, seems like a bug.
(https://github.com/bitcoin/bitcoin/issues/28864)
I would have expected that `CXXFLAGS="-mno-sse4.1" ./configure` would make the sse4.1 intrinsics check fail but it doesn't:
```
configure:23377: checking for SSE4.1 intrinsics
configure:23396: clang++ -std=c++17 -c -msse4.1 -mno-sse4.1 conftest.cpp >&5
configure:23396: $? = 0
configure:23398: result: yes
```
Not sure what is going on here, seems like a bug.
💬 fanquake commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#discussion_r1391405285)
Pretty sure I did this so we could consolidate the gcc hardening options, doesn't need to be a blocker for this, so droppe for now.
(https://github.com/bitcoin/bitcoin/pull/28461#discussion_r1391405285)
Pretty sure I did this so we could consolidate the gcc hardening options, doesn't need to be a blocker for this, so droppe for now.
💬 fanquake commented on pull request "guix: update signapple (drop macho & altgraph)":
(https://github.com/bitcoin/bitcoin/pull/28859#issuecomment-1808598108)
Guix Build (aarch64):
```bash
7ea71c9d553c8afaa6c1f8ba72b643783910a972faabd6712f85020dbe8dbdae guix-build-f718a74b124c/output/arm64-apple-darwin/SHA256SUMS.part
3263327cbd4ee22c21256410da377b38399d07f163a6d1c6c266aa1f9024de64 guix-build-f718a74b124c/output/arm64-apple-darwin/bitcoin-f718a74b124c-arm64-apple-darwin-unsigned.tar.gz
dc639d8aff10827c0e1a24e4edf16425cf08d443948659ecdb76bb9d2bcae90e guix-build-f718a74b124c/output/arm64-apple-darwin/bitcoin-f718a74b124c-arm64-apple-darwin-unsign
...
(https://github.com/bitcoin/bitcoin/pull/28859#issuecomment-1808598108)
Guix Build (aarch64):
```bash
7ea71c9d553c8afaa6c1f8ba72b643783910a972faabd6712f85020dbe8dbdae guix-build-f718a74b124c/output/arm64-apple-darwin/SHA256SUMS.part
3263327cbd4ee22c21256410da377b38399d07f163a6d1c6c266aa1f9024de64 guix-build-f718a74b124c/output/arm64-apple-darwin/bitcoin-f718a74b124c-arm64-apple-darwin-unsigned.tar.gz
dc639d8aff10827c0e1a24e4edf16425cf08d443948659ecdb76bb9d2bcae90e guix-build-f718a74b124c/output/arm64-apple-darwin/bitcoin-f718a74b124c-arm64-apple-darwin-unsign
...