🤔 mzumsande reviewed a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2298031257)
ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac
> Updated the commit description. Let me know if further updates are needed.
Looks good to me!
> What about us connecting to a synced limited peer, during tip sync, who tries to fetch historical blocks due to the NODE_NETWORK service, like https://github.com/bitcoin/bitcoin/issues/29183.
I think we just wouldn't connect to limited peers during tip sync - only when we are very close to the global tip we accept those as outbound peers (`GetDes
...
(https://github.com/bitcoin/bitcoin/pull/30807#pullrequestreview-2298031257)
ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac
> Updated the commit description. Let me know if further updates are needed.
Looks good to me!
> What about us connecting to a synced limited peer, during tip sync, who tries to fetch historical blocks due to the NODE_NETWORK service, like https://github.com/bitcoin/bitcoin/issues/29183.
I think we just wouldn't connect to limited peers during tip sync - only when we are very close to the global tip we accept those as outbound peers (`GetDes
...
💬 achow101 commented on pull request "docs: updated developer notes for --with-sanitizers to -DSANITIZERS":
(https://github.com/bitcoin/bitcoin/pull/30870#issuecomment-2344256749)
ACK 4b1ce3cac81497dd094e8c34550edf633f4d38ae
(https://github.com/bitcoin/bitcoin/pull/30870#issuecomment-2344256749)
ACK 4b1ce3cac81497dd094e8c34550edf633f4d38ae
🚀 achow101 merged a pull request: "docs: updated developer notes for --with-sanitizers to -DSANITIZERS"
(https://github.com/bitcoin/bitcoin/pull/30870)
(https://github.com/bitcoin/bitcoin/pull/30870)
💬 achow101 commented on pull request "docs: Updated debug build instructions for cmake":
(https://github.com/bitcoin/bitcoin/pull/30840#issuecomment-2344260817)
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
TIL `ccmake`.
(https://github.com/bitcoin/bitcoin/pull/30840#issuecomment-2344260817)
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
TIL `ccmake`.
🚀 achow101 merged a pull request: "docs: Updated debug build instructions for cmake"
(https://github.com/bitcoin/bitcoin/pull/30840)
(https://github.com/bitcoin/bitcoin/pull/30840)
💬 achow101 commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2344266604)
ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2344266604)
ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac
💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1755201921)
[Done](https://github.com/bitcoin/bitcoin/compare/f68e9d8887651828bd99635165fcec43f5cf6d4a..376ba5fcfd72a253b2dfe853781a10eee1af2ccb), thanks
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1755201921)
[Done](https://github.com/bitcoin/bitcoin/compare/f68e9d8887651828bd99635165fcec43f5cf6d4a..376ba5fcfd72a253b2dfe853781a10eee1af2ccb), thanks
💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1755207524)
Fair, [reverted](https://github.com/bitcoin/bitcoin/compare/f68e9d8887651828bd99635165fcec43f5cf6d4a..376ba5fcfd72a253b2dfe853781a10eee1af2ccb) the unrelated lines
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1755207524)
Fair, [reverted](https://github.com/bitcoin/bitcoin/compare/f68e9d8887651828bd99635165fcec43f5cf6d4a..376ba5fcfd72a253b2dfe853781a10eee1af2ccb) the unrelated lines
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1755219785)
Not reuse, but to obviate that the method does two separate things: complex logic for how to serialize a number, followed by inserting bytes - so basically a separation of concerns.
And also https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1741078658 indicated that operators may not be here to stay, so it makes sense to unburden them early.
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1755219785)
Not reuse, but to obviate that the method does two separate things: complex logic for how to serialize a number, followed by inserting bytes - so basically a separation of concerns.
And also https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1741078658 indicated that operators may not be here to stay, so it makes sense to unburden them early.
🚀 achow101 merged a pull request: "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD"
(https://github.com/bitcoin/bitcoin/pull/30807)
(https://github.com/bitcoin/bitcoin/pull/30807)
👋 achow101's pull request is ready for review: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
(https://github.com/bitcoin/bitcoin/pull/30827)
💬 achow101 commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2344308159)
Backported to 28.x in #30827
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2344308159)
Backported to 28.x in #30827
👍 tdb3 approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129278263)
ACK 69bf58dc0e25897e9fde435c9823a921590a90dc
This is definitely an improvement on the existing loadwallet help.
nit: I believe the RPC page listed in the commit message is actually derived from the help output of `loadwallet`. Technically this PR addresses the help output, and downstream things that use it would also benefit. I don't really see a crucial need to change the commit message though, so feel free to ignore. If you happen to change something else, it might be good to also upd
...
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2129278263)
ACK 69bf58dc0e25897e9fde435c9823a921590a90dc
This is definitely an improvement on the existing loadwallet help.
nit: I believe the RPC page listed in the commit message is actually derived from the help output of `loadwallet`. Technically this PR addresses the help output, and downstream things that use it would also benefit. I don't really see a crucial need to change the commit message though, so feel free to ignore. If you happen to change something else, it might be good to also upd
...
💬 tdb3 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646864324)
nit: Same for here:
`Wallet with files under wallets/foo/specialWallet/:`
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646864324)
nit: Same for here:
`Wallet with files under wallets/foo/specialWallet/:`
💬 tdb3 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646863530)
nit: Stating `Load regular` seems extraneous here (and usage of `regular` might bring up some other questions (e.g. non-regular?)). Shortening to `Wallet with files under wallets/foo/:` seems ok.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646863530)
nit: Stating `Load regular` seems extraneous here (and usage of `regular` might bring up some other questions (e.g. non-regular?)). Shortening to `Wallet with files under wallets/foo/:` seems ok.
💬 tdb3 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646865129)
nit: Same for here:
`wallet specified by absolute path`
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646865129)
nit: Same for here:
`wallet specified by absolute path`
💬 tdb3 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1755238118)
This seems to discuss passing in the absolute path of the .dat file. Saw the following behavior:
(regtest=1 in bitcoin.conf)
```
dev@bdev01:~/bitcoin$ src/bitcoind -daemonwait
Bitcoin Core starting
dev@bdev01:~/bitcoin$ src/bitcoin-cli createwallet foo
{
"name": "foo"
}
dev@bdev01:~/bitcoin$ src/bitcoin-cli stop
Bitcoin Core stopping
dev@bdev01:~/bitcoin$ src/bitcoind -daemonwait
Bitcoin Core starting
dev@bdev01:~/bitcoin$ ls -l /home/dev/.bitcoin/regtest/wallets/foo/wallet.dat
...
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1755238118)
This seems to discuss passing in the absolute path of the .dat file. Saw the following behavior:
(regtest=1 in bitcoin.conf)
```
dev@bdev01:~/bitcoin$ src/bitcoind -daemonwait
Bitcoin Core starting
dev@bdev01:~/bitcoin$ src/bitcoin-cli createwallet foo
{
"name": "foo"
}
dev@bdev01:~/bitcoin$ src/bitcoin-cli stop
Bitcoin Core stopping
dev@bdev01:~/bitcoin$ src/bitcoind -daemonwait
Bitcoin Core starting
dev@bdev01:~/bitcoin$ ls -l /home/dev/.bitcoin/regtest/wallets/foo/wallet.dat
...
📝 m3dwards opened a pull request: "test: fix exclude parsing for functional runner"
(https://github.com/bitcoin/bitcoin/pull/30872)
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded.
PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6") but it made the wrong assumption that test names intended to be excluded would include the .py extension.
The foll
...
(https://github.com/bitcoin/bitcoin/pull/30872)
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded.
PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6") but it made the wrong assumption that test names intended to be excluded would include the .py extension.
The foll
...
💬 m3dwards commented on pull request "test: fix exclude parsing for functional runner":
(https://github.com/bitcoin/bitcoin/pull/30872#issuecomment-2344343088)
CC @maflcko
(https://github.com/bitcoin/bitcoin/pull/30872#issuecomment-2344343088)
CC @maflcko
⚠️ instagibbs opened an issue: "Populate a PSBT input with a UTXO not in wallet/mempoo/utxo set"
(https://github.com/bitcoin/bitcoin/issues/30873)
### Please describe the feature you'd like to see added.
If a user wants to make a chain of two transactions and sign the child *without being able to get the parent in their mempool*, I do not believe there is a way for this to be done natively in the PSBT RPCs. As the UTXO is not in the mempool/wallet/utxo set, it cannot be found for signing/finalization, unless injected manually.
Note that this functionality seems to exist in the raw transaction path via the "prevtx" argument for signing,
...
(https://github.com/bitcoin/bitcoin/issues/30873)
### Please describe the feature you'd like to see added.
If a user wants to make a chain of two transactions and sign the child *without being able to get the parent in their mempool*, I do not believe there is a way for this to be done natively in the PSBT RPCs. As the UTXO is not in the mempool/wallet/utxo set, it cannot be found for signing/finalization, unless injected manually.
Note that this functionality seems to exist in the raw transaction path via the "prevtx" argument for signing,
...