💬 maflcko commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1808316213)
> These are the currently failing tests.

Failing why? Without any information, there is nothing we can do here.

Recall that the macOS CI passes on this pull request currently.

This looks like a local problem on your side.

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