📝 s1Sharp opened a pull request: "refactor: Readable widget naming for debug window"
(https://github.com/bitcoin-core/gui/pull/790)
Fixed widget names in the debug window. Make it more developer friendly to avoid confusion inside Qt Designer.
(https://github.com/bitcoin-core/gui/pull/790)
Fixed widget names in the debug window. Make it more developer friendly to avoid confusion inside Qt Designer.
💬 s1Sharp commented on pull request "refactor: Readable widget naming for debug window":
(https://github.com/bitcoin-core/gui/pull/790#issuecomment-1902752609)
Qt Designer view before changes:
Qt Designer view after changes:
And a few changes for other widget names that were not included in the screenshots
(https://github.com/bitcoin-core/gui/pull/790#issuecomment-1902752609)
Qt Designer view before changes:
Qt Designer view after changes:
And a few changes for other widget names that were not included in the screenshots
💬 ajtowns commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1902869601)
> I compiled with #28217, which sets default value of `permitbaremultisig` to false in `policy.h` and tried to run this test case with `'allowed': False` for expected result but the test fails. I would have expected that one to pass the test.
The test case already explicitly sets `permitbaremultisig=0`, so changing the default should have no effect. These spends should always be allowed, so if you change the test to assert that it's not allowed, the test should fail.
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1902869601)
> I compiled with #28217, which sets default value of `permitbaremultisig` to false in `policy.h` and tried to run this test case with `'allowed': False` for expected result but the test fails. I would have expected that one to pass the test.
The test case already explicitly sets `permitbaremultisig=0`, so changing the default should have no effect. These spends should always be allowed, so if you change the test to assert that it's not allowed, the test should fail.
🤔 ajtowns reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1835540987)
I find myself continually forgetting what the v3 rules are (particularly vs the ephemeral anchor rules, package relay rules, cluster mempool rules, or potential v4+ rules). It would be nice to have a really concise reference, that leaves the rationales/comparisons to an appendix or something. I think when stripped of historical baggage, it only needs to be:
unconfirmed v3 transactions:
* are always subject to rbf rules
* can have at most one unconfirmed v3 ancestor or one unconfirmed v3 d
...
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1835540987)
I find myself continually forgetting what the v3 rules are (particularly vs the ephemeral anchor rules, package relay rules, cluster mempool rules, or potential v4+ rules). It would be nice to have a really concise reference, that leaves the rationales/comparisons to an appendix or something. I think when stripped of historical baggage, it only needs to be:
unconfirmed v3 transactions:
* are always subject to rbf rules
* can have at most one unconfirmed v3 ancestor or one unconfirmed v3 d
...
💬 ajtowns commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461286728)
Perhaps this should be "An **unconfirmed** V3 transaction cannothave more than 1 unconfirmd descendent" ?
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461286728)
Perhaps this should be "An **unconfirmed** V3 transaction cannothave more than 1 unconfirmd descendent" ?
💬 ajtowns commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461284621)
It might be interesting to relax these rules to be:
* A v3 transaction can have a max descendent count of 5 (not 2)
* A v3 transaction with an ancestor count of two or more can have a maximum (sigop-adjusted) descendant size of 1000vb
* A v3 transaction can have at most 1 unconfirmed parent (rather than limiting ancestor count to at most 2)
(where ancestor/descendent size/count includes itself)
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461284621)
It might be interesting to relax these rules to be:
* A v3 transaction can have a max descendent count of 5 (not 2)
* A v3 transaction with an ancestor count of two or more can have a maximum (sigop-adjusted) descendant size of 1000vb
* A v3 transaction can have at most 1 unconfirmed parent (rather than limiting ancestor count to at most 2)
(where ancestor/descendent size/count includes itself)
💬 ajtowns commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461338849)
This seems underspecified: what's the behaviour if you have a v3 transaction in the mempool, with two or more outputs, one of which is spent by another v3 in-mempool transaction, and another (extremely high fee) transaction comes in, spending the other output?
Is it first-come-first-served, making descendent-count-pinning much easier? Or is it "we'll swap the old for the new if it has a better feerate diagram"? Or something else?
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461338849)
This seems underspecified: what's the behaviour if you have a v3 transaction in the mempool, with two or more outputs, one of which is spent by another v3 in-mempool transaction, and another (extremely high fee) transaction comes in, spending the other output?
Is it first-come-first-served, making descendent-count-pinning much easier? Or is it "we'll swap the old for the new if it has a better feerate diagram"? Or something else?
💬 stratospher commented on issue "Intermittent CI failure "fee too high" in wallet_send.py":
(https://github.com/bitcoin/bitcoin/issues/25164#issuecomment-1903232885)
happened here too: https://cirrus-ci.com/task/4577902440742912?logs=ci#L2654
(https://github.com/bitcoin/bitcoin/issues/25164#issuecomment-1903232885)
happened here too: https://cirrus-ci.com/task/4577902440742912?logs=ci#L2654
💬 maflcko commented on issue "Build error on macOS, tag v26.0 builds fine":
(https://github.com/bitcoin/bitcoin/issues/29289#issuecomment-1903399940)
Which version of clang are you using?
You'll have to use at least version 13 or 14, depending on your build settings, but an even later version would be better.
(https://github.com/bitcoin/bitcoin/issues/29289#issuecomment-1903399940)
Which version of clang are you using?
You'll have to use at least version 13 or 14, depending on your build settings, but an even later version would be better.
💬 S3RK commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1903507665)
According to my `range-diff` nothing changed.
reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1903507665)
According to my `range-diff` nothing changed.
reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461526010)
Why 5?
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461526010)
Why 5?
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1903529160)
@maflcko that comment refers to running CI for checking my own work. But what this pull request enables is running CI for pull requests, which may be from myself but also from others.
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1903529160)
@maflcko that comment refers to running CI for checking my own work. But what this pull request enables is running CI for pull requests, which may be from myself but also from others.
💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector":
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903550684)
I like the idea of operating on `CKey` directly. I'll try to add that, though not sure how to implement that securely.
The keys uses in #28983 are not as important as wallet keys, but if we add a generic method to store a `CKey` on disk, then it should do so securely - so that future devs using that function don't themselves in the foot. At the least the current `std::vector<unsigned char>>` does not pretend to be secure.
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903550684)
I like the idea of operating on `CKey` directly. I'll try to add that, though not sure how to implement that securely.
The keys uses in #28983 are not as important as wallet keys, but if we add a generic method to store a `CKey` on disk, then it should do so securely - so that future devs using that function don't themselves in the foot. At the least the current `std::vector<unsigned char>>` does not pretend to be secure.
💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector":
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903561636)
> I guess that by "plain text" here you mean `std::string`, right? `std::string` can store arbitrary non-ASCII characters, including `'\0'`, so it is technically ok to use it for binary data.
I think the way `WriteBinaryFile` was initially used with Tor was that we parse the JSON returned by the Tor daemon and store the `PrivateKey` key field (UTF8 encoced?) in a file.
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903561636)
> I guess that by "plain text" here you mean `std::string`, right? `std::string` can store arbitrary non-ASCII characters, including `'\0'`, so it is technically ok to use it for binary data.
I think the way `WriteBinaryFile` was initially used with Tor was that we parse the JSON returned by the Tor daemon and store the `PrivateKey` key field (UTF8 encoced?) in a file.
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461554508)
> This seems underspecified: what's the behaviour if...
The behavior is what the existing RBF rules say (so no sibling eviction). If there's going to be 2 anchors I feel like v3 is not the right version to use.
> is it "we'll swap the old for the new if it has a better feerate diagram"?
I think sibling eviction can be considered, and feerate diagram comparisons as well. But the idea of splitting up #25038 was to decouple v3 topology restrictions from the (package) RBF rules that would b
...
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1461554508)
> This seems underspecified: what's the behaviour if...
The behavior is what the existing RBF rules say (so no sibling eviction). If there's going to be 2 anchors I feel like v3 is not the right version to use.
> is it "we'll swap the old for the new if it has a better feerate diagram"?
I think sibling eviction can be considered, and feerate diagram comparisons as well. But the idea of splitting up #25038 was to decouple v3 topology restrictions from the (package) RBF rules that would b
...
💬 Sjors commented on pull request "Make (Read/Write)BinaryFile work with char vector":
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903570460)
> I haven't looked in detail, but writing bytes to a file can be achieved with one line of code
That would be a nice simplification. Almost (?) to the point of not needing these helper functions.
(https://github.com/bitcoin/bitcoin/pull/29229#issuecomment-1903570460)
> I haven't looked in detail, but writing bytes to a file can be achieved with one line of code
That would be a nice simplification. Almost (?) to the point of not needing these helper functions.
🚀 glozow merged a pull request: "refactor: remove CTxMemPool::queryHashes()"
(https://github.com/bitcoin/bitcoin/pull/29260)
(https://github.com/bitcoin/bitcoin/pull/29260)
📝 jerzybrzoska opened a pull request: "Update build-unix.md - add boost library"
(https://github.com/bitcoin/bitcoin/pull/29290)
libboost-all-dev is needed to compile on Debian stable (Bookworm).
(https://github.com/bitcoin/bitcoin/pull/29290)
libboost-all-dev is needed to compile on Debian stable (Bookworm).
💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1903697822)
> I find myself continually forgetting what the v3 rules are (particularly vs the ephemeral anchor rules, package relay rules, cluster mempool rules, or potential v4+ rules). It would be nice to have a really concise reference, that leaves the rationales/comparisons to an appendix or something. I think when stripped of historical baggage, it only needs to be:
> ...
I've consolidated the rules in the BIP (https://github.com/bitcoin/bips/pull/1541), agree it's easier to understand this way.
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1903697822)
> I find myself continually forgetting what the v3 rules are (particularly vs the ephemeral anchor rules, package relay rules, cluster mempool rules, or potential v4+ rules). It would be nice to have a really concise reference, that leaves the rationales/comparisons to an appendix or something. I think when stripped of historical baggage, it only needs to be:
> ...
I've consolidated the rules in the BIP (https://github.com/bitcoin/bips/pull/1541), agree it's easier to understand this way.
...
💬 dergoegge commented on pull request "Update build-unix.md - add boost library":
(https://github.com/bitcoin/bitcoin/pull/29290#issuecomment-1903703366)
The docs suggest to install or build boost one line below already: https://github.com/bitcoin/bitcoin/blob/651fb034d85eb5db561bfd24b74f7271417defa5/doc/build-unix.md?plain=1#L49-L51
Does that not work for you?
(https://github.com/bitcoin/bitcoin/pull/29290#issuecomment-1903703366)
The docs suggest to install or build boost one line below already: https://github.com/bitcoin/bitcoin/blob/651fb034d85eb5db561bfd24b74f7271417defa5/doc/build-unix.md?plain=1#L49-L51
Does that not work for you?