💬 josibake commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063942430)
> Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into /usr/local wouldn't have random odds-and-ends ending up in the root there
We still have the `README.md` in the root, so moving this file doesn't really address that concern. If there is a compelling argument for having _nothing_ in the root, then I'd argue `config/bitcoin.conf.example` would make the most sense (as this is what I've seen for most other tarball distributed software
...
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063942430)
> Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into /usr/local wouldn't have random odds-and-ends ending up in the root there
We still have the `README.md` in the root, so moving this file doesn't really address that concern. If there is a compelling argument for having _nothing_ in the root, then I'd argue `config/bitcoin.conf.example` would make the most sense (as this is what I've seen for most other tarball distributed software
...
💬 josibake commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063950133)
LGTM, although the commit message and PR title still mention moving to `share/examples`
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063950133)
LGTM, although the commit message and PR title still mention moving to `share/examples`
💬 sr-gi commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1570838311)
This is just being moved but agreed is simple enough to include it here.
Covered in [fd81a37](https://github.com/bitcoin/bitcoin/pull/28834/commits/fd81a37239541d0d508402cd4eeb28f38128c1f2)
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1570838311)
This is just being moved but agreed is simple enough to include it here.
Covered in [fd81a37](https://github.com/bitcoin/bitcoin/pull/28834/commits/fd81a37239541d0d508402cd4eeb28f38128c1f2)
📝 BorjaPractica opened a pull request: "modificación"
(https://github.com/bitcoin/bitcoin/pull/29905)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29905)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "modificación"
(https://github.com/bitcoin/bitcoin/pull/29905)
(https://github.com/bitcoin/bitcoin/pull/29905)
📝 fanquake locked a pull request: "modificación"
(https://github.com/bitcoin/bitcoin/pull/29905)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29905)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2063985018)
Either you could generate the mainnet index using the code in this pull request, and then compare it. Or I can take your list and compare it to my result. Whoever gets around to it first :-)
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2063985018)
Either you could generate the mainnet index using the code in this pull request, and then compare it. Or I can take your list and compare it to my result. Whoever gets around to it first :-)
👍 guggero approved a pull request: "ZMQ: Support UNIX domain sockets"
(https://github.com/bitcoin/bitcoin/pull/27679#pullrequestreview-2009075318)
Tested and code review ACK 21d0e6c7b7c7
Thanks a lot for the fix!
(https://github.com/bitcoin/bitcoin/pull/27679#pullrequestreview-2009075318)
Tested and code review ACK 21d0e6c7b7c7
Thanks a lot for the fix!
💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570911819)
> ok, yeah. Then it is not an issue but, isn't error-prone to call `best_hdr_chain` something that might not be the best chain then? E.g. it seems that if we ever add a check only for the active chain blocks here, the `best_hdr_chain.Contains()` usage temptation will be there.
I don't love the name `best_hdr_chain` but don't think it is too bad. I agree it could potentially be confused with the active chain in new code, and that would be a bug. In my original suggestion https://github.com/bit
...
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570911819)
> ok, yeah. Then it is not an issue but, isn't error-prone to call `best_hdr_chain` something that might not be the best chain then? E.g. it seems that if we ever add a check only for the active chain blocks here, the `best_hdr_chain.Contains()` usage temptation will be there.
I don't love the name `best_hdr_chain` but don't think it is too bad. I agree it could potentially be confused with the active chain in new code, and that would be a bug. In my original suggestion https://github.com/bit
...
💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1570920312)
Yes, it needs both `activateWindow()` & `raise()`, let me try it on Windows and I'll incorporate your [suggestion](https://github.com/bitcoin-core/gui/pull/817#discussion_r1569483043), thanks!
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1570920312)
Yes, it needs both `activateWindow()` & `raise()`, let me try it on Windows and I'll incorporate your [suggestion](https://github.com/bitcoin-core/gui/pull/817#discussion_r1569483043), thanks!
💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2064197562)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2064197562)
Concept ACK.
💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2064247329)
> > There are no conflicts with #29865.
>
> Looks like there are now?
Well, there is a conflict in the code, which is being removed in #29865. It won't be a problem for either PR if the other one goes first.
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2064247329)
> > There are no conflicts with #29865.
>
> Looks like there are now?
Well, there is a conflict in the code, which is being removed in #29865. It won't be a problem for either PR if the other one goes first.
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015884)
(deleted `GetCombinedHash`)
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015884)
(deleted `GetCombinedHash`)
🤔 glozow reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2009283703)
Main changes:
- Try same-peer orphanage children (most recent first) before different-peer ones (randomized). Replaced `GetChildren` with `GetChildrenFromSamePeer` and `GetChildrenFromDifferentPeer`.
- Deleted `GetCombinedHash` because it's unused. Also deleted the tests for it
- Changed `GetPackageHash` a bit. Using single-sha256 instead of double-sha256 and using just wtxids instead of serialized vector
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2009283703)
Main changes:
- Try same-peer orphanage children (most recent first) before different-peer ones (randomized). Replaced `GetChildren` with `GetChildrenFromSamePeer` and `GetChildrenFromDifferentPeer`.
- Deleted `GetCombinedHash` because it's unused. Also deleted the tests for it
- Changed `GetPackageHash` a bit. Using single-sha256 instead of double-sha256 and using just wtxids instead of serialized vector
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015190)
deleted
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015190)
deleted
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571014755)
I figure it's easier to read the test this way; you can easily tell that the lexicographical ordering is what I claim it to be
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571014755)
I figure it's easier to read the test this way; you can easily tell that the lexicographical ordering is what I claim it to be
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015988)
(deleted `GetCombinedHash`)
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015988)
(deleted `GetCombinedHash`)
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571006395)
(deleted `GetCombinedHash`)
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571006395)
(deleted `GetCombinedHash`)
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571005910)
removed
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571005910)
removed
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571017712)
Did this now. We first gather all children by the same peer and try by recency order. If we cannot find one that matches, we look for children not by this peer, and try in random order.
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571017712)
Did this now. We first gather all children by the same peer and try by recency order. If we cannot find one that matches, we look for children not by this peer, and try in random order.