💬 instagibbs commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391258045)
added a top-level "package-msg" response which is "success" when everything goes through.

I also changed the logic to allow `PCKG_POLICY` to continue if there are transaction results, because this can happen if single transactions make it in, then a subsequent subpackage hits chain limits and is rejected.

💬 instagibbs commented on pull request "bugfix, Change up submitpackage results to return results for all transactions":
(https://github.com/bitcoin/bitcoin/pull/28848#discussion_r1391258432)
I will squash if/when we decide this is a good direction

👍 laanwj approved a pull request: "guix: update time-machine"
(https://github.com/bitcoin/bitcoin/pull/28580#pullrequestreview-1727575704)
LGTM ACK 92d12f1c890350f40d8e5d5c6a59d5c172ea7550

👍 maflcko approved a pull request: "test, refactor: Magic bytes array followup"
(https://github.com/bitcoin/bitcoin/pull/28857#pullrequestreview-1727579221)
lgtm

💬 maflcko commented on pull request "test, refactor: Magic bytes array followup":
(https://github.com/bitcoin/bitcoin/pull/28857#discussion_r1391276525)
```suggestion
BOOST_CHECK_EQUAL(GetSerializeSize(std::array<uint8_t, 2>{}, 0), 2U);
```

nit: I think array will fill itself with zeros if they are omitted

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808393908)
To be clear: this cannot just be implemented inside `TxToUniv`. It needs access to the UTXOs being spent by a transaction. I'm not sure what the right approach is, but it's not a trivial change.

💬 hebasto commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1808406276)
My Guix builds:
```
x86_64
99a81851f831f102e8f52e3e75b72b9228b3317d79670c7093b40550a28681e9 guix-build-3c61c60b90db/output/arm64-apple-darwin/SHA256SUMS.part
3a6d8c63489481d3c58ed6a54b2b1264a2489932a0f1c528c453e620ec3f1c69 guix-build-3c61c60b90db/output/arm64-apple-darwin/bitcoin-3c61c60b90db-arm64-apple-darwin-unsigned.tar.gz
816808b6c7df70c171bace2b95d66d34fb042b15f0d690c6f519479e5f3a57fd guix-build-3c61c60b90db/output/arm64-apple-darwin/bitcoin-3c61c60b90db-arm64-apple-darwin-unsigned
...

💬 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

💬 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)

💬 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.

💬 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).

👍 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.

💬 achow101 commented on pull request "mempool: Persist with XOR":
(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.

🚀 achow101 merged a pull request: "mempool: Persist with XOR"
(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.

💬 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?

💬 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?

💬 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;
```

💬 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`?