💬 mzumsande commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1847140617)
I think that this will change behavior in the case where we re-submit a previously pruned block with `submitblock`:

On master, we'd skip early here and return "duplicate" because the index is still `BLOCK_VALID_SCRIPTS`-valid.
But with his branch, we'd accept the block and save it to disk once more.

I'm really unsure if that is an undesired change in behavior or not - I guess it could also be viewed as a feature.
In any case, it's a behavior change that should be documented if we do go t
...

👍 ryanofsky approved a pull request: "RPC/Wallet: Convert walletprocesspsbt to use options parameter"
(https://github.com/bitcoin/bitcoin/pull/24963#pullrequestreview-2443280039)
Code review ACK 40143bafb52927eb40ecfde0ea722934213eb902.

This is a pretty simple change to the RPC framework that should make it easy to add new options to RPCs like [createwallet](https://github.com/bitcoin/bitcoin/pull/29129#pullrequestreview-1804913032) that currently accept too many positional parameters, without having to increase the number of positional parameters they already accept.

Without this PR, the only ways to add a new options to an RPC like `createwallet` without breaking
...

💬 ryanofsky commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1847054828)
In commit "RPC/Wallet: Convert walletprocesspsbt to use options parameter" (28023ff415ca1fa9c2b9c12e6bc525f8bc14aac5)

I think it would be good to move this code higher in the function, before the BlockUntilSyncedToCurrentChain and DecodeBase64PSBT calls to give caller more immediate feedback if they are calling this function with invalid parameters or the wrong types.

💬 ryanofsky commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1847064725)
In commit "RPC/Wallet: Convert walletprocesspsbt to use options parameter" (28023ff415ca1fa9c2b9c12e6bc525f8bc14aac5)

Might be good to test a combination of named / positional arguments like

```python
assert_equal(updated, self.nodes[1].walletprocesspsbt(psbt, sign=False, sighashtype='ALL', bip32derivs=True)["psbt"])
```

💬 ryanofsky commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1846994896)
re: https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1470126512

> the github comment this is responding to was mostly about `RPCHelpMan::GetArgMap`, which can still be simplified.

Actually the simplification I suggested to `GetArgMap` won't work as long we want the option to be declared as "options|sign" instead of just "options", which I agree with Luke is less clear. So while my original suggestion was correct and simpler than current PR, I think the current approach is better,
...

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847193982)
Added.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847188474)
Added testcases flipping both descriptor order and blockhash order to ensure results don't change.

🤔 jamesob reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2443598792)
I've pushed changes addressing all outstanding feedback. I've slightly changed the return schema [as discussed on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-11-15), moving all `scriptPubKey` related information into `output_spk`/`prevout_spk`, which matches the schema used in `getrawtransaction 2`.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847219113)
Added a comment to this effect.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847234154)
As discussed [on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-11-15), I've moved all sPK-specific info into a substructure that matches the one used in `getrawtransaction 2`. I've added @instagibbs' test under a commit with him as co-author.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847218317)
Added tests including duplicate and reversal.

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1847250753)
Maybe could come as a follow-up?

🤔 furszy reviewed a pull request: "test: Introduce ensure_for helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2443695664)
utACK 111465d72dd35e42361fc2a089036f652417ed37

💬 TheCharlatan commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1847279135)
Good catch, thank you for taking another look! I'm also not sure what to do in light of this. At the minimum this reason for the duplicate check should be documented. I feel like with this kind of change I would err on the conservative side and not introduce potentially breaking behaviour. Even if this might be a nice way to inject block data into the node, I don't know if this is something anybody needs, even if it now more closely mimics `getblockfrompeer`.

We could keep the duplicate chec
...

💬 TheCharlatan commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1846964232)
Why is this not initialized again with `SCRIPT_ERR_UNKNOWN_ERROR`? I guess it is redundant, because it is set in the verification functions, but might still be good to keep it as a guard?

💬 TheCharlatan commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1847292854)
Isn't this the consensus only result?

⚠️ MORTEZAMIRSALI opened an issue: "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not "
(https://github.com/bitcoin/bitcoin/issues/31319)

💬 MORTEZAMIRSALI commented on issue "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not ":
(https://github.com/bitcoin/bitcoin/issues/31319#issuecomment-2484216000)
balanced and the main owner's financial interests are targeted, Bitcoin is the same, please check all the vandalism of the reservoirs, no one who uses blockchain without the permission of the owner How can the principal destroy interests?

✅ achow101 closed an issue: "Officials of the organization pay attention, just as you guarantee health here, how do you not prevent the invasion of wild animals into the reservoirs related to me? This double action means that no person, whether he was already working or later, enters, or no organization has the right to interfere in the related reservoirs. They don't have the original owner in your repositories, destroy my assets, Twitter changed all my tokens, took my possessions, on the other hand, pay attention to the jackals' support, there have been several crashes today, 400 bitcoins have been mined, destroy them all, why without the permission of the owner, the right to enter address and interests, you should organize and make laws according to my property, everyone is in line to destroy, even tonspn is not "
(https://github.com/bitcoin/bitcoin/issues/31319)

:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31319)