💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213903403)
I've added a `bulk_vout` function to `script_util.py` which is used by `MiniWallet` and these tests.

💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084872811)
the cross-compilation toolchain doesn't exist, so it isn't provided by any distribution at all

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213914676)
I've added a version 2 assertion everywhere that `sendall` is used in the tests.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213915214)
I've added a test for alice spending change

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213915397)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213916090)
Yes, this was an outdated comment which I've now removed

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213916340)
Done

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213917014)
Yes, I've updated both of the places where I was doing this.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213923610)
Yes, because when Bob confirms, it is removed from `mempool_conflicts` and Alice's transaction is considered "Inactive". I've added a part to this test case where Alice evicts Bob's transaction.

💬 sfsegreto commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3084894912)
Can you be even more specific please? When you say cross-compilation do you mean cross-compile from Linux to Windows or do you mean cross-compile from Winx64 to WinArm64? And also what toolchain specifically does not exist?

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2213927081)
It would be tricky to consolidate the two because one of them is testing preventing the wallet from creating a transaction and the other is testing marking conflicts correctly.

💬 instagibbs commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-3084911947)
Having issues getting this to work on my local Verizon router. I don't appear to be reachable at the port specified and I don't see any pmp fallbacks happening in logs?

Every 5 minutes I see this set of logs (with port 8339 fwiw):

2025-07-17T17:35:03.446228Z [net] portmap: gateway [IPv4]: <redacted>
2025-07-17T17:35:03.446327Z [net] pcp: Requesting port mapping for addr 0.0.0.0 port 8339 from gateway <redacted>
2025-07-17T17:35:03.446407Z [net] pcp: Internal address after connect: <redacted>
2
...

💬 stickies-v commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#discussion_r2213942752)
> You can try this for any command that captures the output:

Ah cool, thanks for the example. Agreed that improving is out of scope for this PR then.

> I don't think it makes sense to show the traceback when the user pressed CTRL+C?

No strong preference either way, I kinda like seeing where it got interrupted (and I'm already used to it because the `KeyboardInterrupt` exception handling doesn't work for test_runner.py which is my usual interface), but makes sense to not change this beha
...

👍 stickies-v approved a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3030467318)
ACK fa30b34026f76a5b8af997152fced2d281782e0d

🤔 ismaelsadeeq reviewed a pull request: "ipc: add bitcoin-mine test program"
(https://github.com/bitcoin/bitcoin/pull/30437#pullrequestreview-3029237466)
Weak ACK 2910315d1d8a3879f4e0bd50bf90870b642bfede

I’m not sure whether 600a52a0ba08dca06de891e6716ab2730580ddcd is necessary.

Personally, I preferred the previous iteration keeping this PR as a minimal boilerplate to demonstrate IPC usage. This increases the scope quite a bit, and if we're going to demonstrate mining, I think it should be done accurately.

I’ve added some comments below pointing out that we don’t currently handle chain tip changes. Could also be better to use `waitNext`
...

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2213158587)
nit:
```suggestion
# Set paths to bitcoin core binaries allowing overrides with environment
```

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2213893768)
Return when we are not on test chain?

```suggestion
if (!Params().IsTestChain()) return EXIT_SUCCESS;

```

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2213909058)
only send solution when you have a valid block?
```suggestion
if (CheckProofOfWork(block.GetHash(), block.nBits, consensus_params)) block_template->submitSolution(block.nVersion, block.nTime, block.nNonce, block_template->getCoinbaseTx());

```

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2213949664)
The logs are really verbose and clunky I opened a seperate issue https://github.com/bitcoin-core/libmultiprocess/issues/190

💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#discussion_r2213947741)
This should also break when the chain tip changes or generate a new template. Right now, it's possible to pass a large `max_tries` and get stuck.

```suggestion
--tries_remaining;
if (block.hashPrevBlock != mining->getTip()->hash) {
block_template{mining->createNewBlock({})};
block{new_block_template->getBlock()};
}
```