💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1391227222)
This code seems to assume that the 4th scriptSig push is the redeemScript. That won't always be correct; it's the last one.

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1391224005)
I don't think this is correct. P2SH spends are identified by the *spent scriptPubKey* having the right form. This will, I suspect, not have any effect; the scriptSig is generally push-only (and non-standard otherwise), so it'll never match the P2SH template.

💬 sipa commented on pull request "test, refactor: Magic bytes array followup":
(https://github.com/bitcoin/bitcoin/pull/28857#issuecomment-1808326518)
utACK 1e5b86171e81ab4b022b9746bb06e1968ecf4086

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808327245)
> Failing why? Without any information, there is nothing we can do here.
As the exception suggests it's due the addition of the new field `redeemScript`.
Am I right?
```
{a.riahi@bitcoin} > python3.10 ./test/functional/rpc_generate.py
2023-11-13T14:57:56.863000Z TestFramework (INFO): PRNG seed is: 2176820945159495690
2023-11-13T14:57:56.863000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_nt1lv9on
2023-11-13T14:57:57.137000Z TestFramework (INFO): Test rpc gener
...

💬 theuni commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808336780)
> LLVM 16 & 17 are now available. This is ready for review. This bump also contains the recently released [Mes 0.25](https://www.mail-archive.com/info-gnu@gnu.org/msg03232.html), which should improve the bootstrapping experience for RISC-V. cc @laanwj who upstreamed a bunch of the changes there.

This is _major_ and unblocks a bunch of toolchain bumps we've needed to do for years. Great work from everyone involved. Thanks @fanquake for pushing on the guix side.

💬 hebasto commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1808337570)
My Guix builds:
```
x86_64
13a7d1be447ecb614cf43034af4f7a3a7ce7dffbcdb6c1773bc939ba80587ef6 guix-build-92d12f1c8903/output/aarch64-linux-gnu/SHA256SUMS.part
5ab66c69b742a89b0aa52705e48563749cb72ebcc92745a4eb07df285a20c62a guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu-debug.tar.gz
d3224eb0eb66bf4433ed8757667ed438e419db4240b06d76122e8754de241742 guix-build-92d12f1c8903/output/aarch64-linux-gnu/bitcoin-92d12f1c8903-aarch64-linux-gnu.tar.gz
9f5ecd1d
...

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1391241361)
Is there any convenient way to detect redeemScript in the script?

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

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1391244227)
So can I always expect the last `scriptSig` push to be the `redeemScript` if it has one?

💬 theuni commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1808343838)
utACK 3c61c60b90db1b6a77b3804784430fcd57b447b6.

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1391246186)
Yes, BIP16 states that the redeemScript is the last push of the scriptSig.

💬 sipa commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808346467)
@Riahiamirreza To fix that error you'll need to add your new field to the documentation of the affected RPCs.

💬 TheCharlatan commented on pull request "depends: latest config.guess & config.sub":
(https://github.com/bitcoin/bitcoin/pull/28781#issuecomment-1808354390)
ACK 49a92579c705831c7ffbcfb24cdf17a94e9a11a0

Checked the content through https://www.gnu.org/software/gettext/manual/html_node/config_002eguess.html

Guix build (x86_64):
```
83eba00a50fe023bce7066676dc15ae5eed1670a4ae3afe89d6c58d371786551 guix-build-49a92579c705/output/aarch64-linux-gnu/SHA256SUMS.part
6b49be1b06c77ff1265af5856a9e4475ee8dd70eee7556c0481a404e356443e5 guix-build-49a92579c705/output/aarch64-linux-gnu/bitcoin-49a92579c705-aarch64-linux-gnu-debug.tar.gz
b748937e55ffe6314b
...

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