💬 ryanofsky commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924946068)
> Personally, I don't like how `TxStateInactive` is so broad. [...]
Sure, but then I'm pretty sure we need to add a `bool abandoned;` field to the `TxStateBlockConflicted` struct so we don't lose track of whether the user has abandoned the transaction.
Alternately, we could go more in the other direction, getting rid of the inactive catch-all and dividing TxStateInactive into 4 states:
- TxStateAbandonedAndConflictsWithMempool
- TxStateAbandonedAndProbablyDoesntConflictWithMempool
- T
...
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924946068)
> Personally, I don't like how `TxStateInactive` is so broad. [...]
Sure, but then I'm pretty sure we need to add a `bool abandoned;` field to the `TxStateBlockConflicted` struct so we don't lose track of whether the user has abandoned the transaction.
Alternately, we could go more in the other direction, getting rid of the inactive catch-all and dividing TxStateInactive into 4 states:
- TxStateAbandonedAndConflictsWithMempool
- TxStateAbandonedAndProbablyDoesntConflictWithMempool
- T
...
💬 ryanofsky commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924962910)
> Personally, I don't like how `TxStateInactive` is so broad. [...]
I will also add that I think dividing the TxStateInactive state instead of just adding a new field to it makes this PR considerably harder to review because you have to manually search for every line of code that is accessing the `m_state` field directly or indirectly and verify that it is either:
1. Treating the TxStateInactive and TxStateMempoolConflicted states equivalently and not changing behavior
2. Treating the TxS
...
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924962910)
> Personally, I don't like how `TxStateInactive` is so broad. [...]
I will also add that I think dividing the TxStateInactive state instead of just adding a new field to it makes this PR considerably harder to review because you have to manually search for every line of code that is accessing the `m_state` field directly or indirectly and verify that it is either:
1. Treating the TxStateInactive and TxStateMempoolConflicted states equivalently and not changing behavior
2. Treating the TxS
...
💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924990135)
> For example, Mara pool when they mined OFAC-compliant blocks. This was clearly a hostile move, and was clearly done for short-term profit. They were almost immediately forced to stop because they knew hashers would move to other pools.
> Also F2Pool, same deal.
- These are the examples for **excluding** some transactions. `permitbaremultisig=1` does not exclude but **include** bare multisig transactions.
- Ocean is a good example of following a different mempool policy for some template
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924990135)
> For example, Mara pool when they mined OFAC-compliant blocks. This was clearly a hostile move, and was clearly done for short-term profit. They were almost immediately forced to stop because they knew hashers would move to other pools.
> Also F2Pool, same deal.
- These are the examples for **excluding** some transactions. `permitbaremultisig=1` does not exclude but **include** bare multisig transactions.
- Ocean is a good example of following a different mempool policy for some template
...
💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924998475)
> Sure, but then I'm pretty sure we need to add a `bool abandoned;` field to the `TxStateMempoolConflicted` struct so we don't lose track of whether the user has abandoned the transaction.
We already lose track of that if a conflict is created and that conflict confirms. Abandoned is not a permanent state - as soon as it shows back up in the mempool, it'll no longer be abandoned. As soon as a conflict appears in a block, it'll no longer be abandoned. Likewise, if a transaction shows up in the
...
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924998475)
> Sure, but then I'm pretty sure we need to add a `bool abandoned;` field to the `TxStateMempoolConflicted` struct so we don't lose track of whether the user has abandoned the transaction.
We already lose track of that if a conflict is created and that conflict confirms. Abandoned is not a permanent state - as soon as it shows back up in the mempool, it'll no longer be abandoned. As soon as a conflict appears in a block, it'll no longer be abandoned. Likewise, if a transaction shows up in the
...
💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925020805)
>These are the examples for **excluding** some transactions. `permitbaremultisig=1` does not exclude but **include** bare multisig transactions.
I don't see how this is relevant. Default mempool policy can either exclude or include certain classes of transactions. Altering these defaults is defying the user community's explicitly stated wishes.
>Ocean is a good example of following a different mempool policy for some templates which **excludes** some transactions. Although it seems irrati
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925020805)
>These are the examples for **excluding** some transactions. `permitbaremultisig=1` does not exclude but **include** bare multisig transactions.
I don't see how this is relevant. Default mempool policy can either exclude or include certain classes of transactions. Altering these defaults is defying the user community's explicitly stated wishes.
>Ocean is a good example of following a different mempool policy for some templates which **excludes** some transactions. Although it seems irrati
...
💬 ryanofsky commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1925021602)
> Abandoned is not a permanent state - as soon as it shows back up in the mempool, it'll no longer be abandoned.
It makes sense that a transaction in the mempool can't be treated as abandoned. But I don't see why the abandoned state be should be cleared just because a mempool heuristic happened to detect a conflict. That just seems like a bad UX and a confusion of two orthogonal concepts. The abandoned state is transient in the sense that it can be overriden by mempool events and block confir
...
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1925021602)
> Abandoned is not a permanent state - as soon as it shows back up in the mempool, it'll no longer be abandoned.
It makes sense that a transaction in the mempool can't be treated as abandoned. But I don't see why the abandoned state be should be cleared just because a mempool heuristic happened to detect a conflict. That just seems like a bad UX and a confusion of two orthogonal concepts. The abandoned state is transient in the sense that it can be overriden by mempool events and block confir
...
🚀 ryanofsky merged a pull request: "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs"
(https://github.com/bitcoin/bitcoin/pull/28868)
(https://github.com/bitcoin/bitcoin/pull/28868)
💬 petertodd commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925168788)
@chrisguida
> For example, Mara pool [when they mined OFAC-compliant blocks](https://www.theblock.co/linked/106865/marathon-ofac-bitcoin-mining-pool-taproot). This was clearly a hostile move, and was clearly done for **short-term profit**. They were almost immediately forced to stop because they knew hashers would move to other pools.
(emphasis mine)
Why do you think Mara was mining OFAC compliant blocks for *short-term* profit? They were leaving fees on the table, and I'm unaware of a
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925168788)
@chrisguida
> For example, Mara pool [when they mined OFAC-compliant blocks](https://www.theblock.co/linked/106865/marathon-ofac-bitcoin-mining-pool-taproot). This was clearly a hostile move, and was clearly done for **short-term profit**. They were almost immediately forced to stop because they knew hashers would move to other pools.
(emphasis mine)
Why do you think Mara was mining OFAC compliant blocks for *short-term* profit? They were leaving fees on the table, and I'm unaware of a
...
💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925193807)
> I don't see how this is relevant. Default mempool policy can either exclude or include certain classes of transactions.
It is relevant and I have edited the comment to make the difference more clear. There is another technical difference in excluding and including. You just need a small percentage of nodes and miners for some transactions to be included. While excluding requires almost everyone to follow the same policy.
> Altering these defaults is defying the user community's explicitl
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925193807)
> I don't see how this is relevant. Default mempool policy can either exclude or include certain classes of transactions.
It is relevant and I have edited the comment to make the difference more clear. There is another technical difference in excluding and including. You just need a small percentage of nodes and miners for some transactions to be included. While excluding requires almost everyone to follow the same policy.
> Altering these defaults is defying the user community's explicitl
...
💬 vostrnad commented on pull request "debugwindow: update session ID tooltip":
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1925201534)
ACK 3bf00e13609eefa6ddb11353519bb1aec2342513
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1925201534)
ACK 3bf00e13609eefa6ddb11353519bb1aec2342513
💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1925228385)
I updated the numbers and rebased the pull-req.
We should get this merged for the next release. It's pretty silly to have a default mempool policy that only 30% of hash power is following.
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1925228385)
I updated the numbers and rebased the pull-req.
We should get this merged for the next release. It's pretty silly to have a default mempool policy that only 30% of hash power is following.
💬 hebasto commented on issue "ci: Android NDK has too old libc++":
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925295864)
Qt docs [claim](https://doc.qt.io/qt-6/android.html) that it supports Clang 14.0.6 (NDK r25b).
It is possible to build depends with Clang 14.0.7 ([NDK r25c](https://github.com/android/ndk/wiki/Changelog-r25)) with API level >= 33.
However, the compiler errors remain the same, unfortunately.
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925295864)
Qt docs [claim](https://doc.qt.io/qt-6/android.html) that it supports Clang 14.0.6 (NDK r25b).
It is possible to build depends with Clang 14.0.7 ([NDK r25c](https://github.com/android/ndk/wiki/Changelog-r25)) with API level >= 33.
However, the compiler errors remain the same, unfortunately.
💬 maflcko commented on issue "ci: Android NDK has too old libc++":
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925301630)
> Clang 14.0.6 (NDK r25b).
Sure, but the issue is the libc++, not clang. A older libc++ is included in r25, see https://github.com/android/ndk/issues/1530#issuecomment-1659055600
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925301630)
> Clang 14.0.6 (NDK r25b).
Sure, but the issue is the libc++, not clang. A older libc++ is included in r25, see https://github.com/android/ndk/issues/1530#issuecomment-1659055600
💬 hebasto commented on issue "ci: Android NDK has too old libc++":
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925302087)
> > Clang 14.0.6 (NDK r25b).
>
> Sure, but the issue is the libc++, not clang. A older libc++ is included in r25, see [android/ndk#1530 (comment)](https://github.com/android/ndk/issues/1530#issuecomment-1659055600)
Right. I mean, even the latest Qt codebase cannot be compiled with NDK > 25.
(https://github.com/bitcoin/bitcoin/issues/29360#issuecomment-1925302087)
> > Clang 14.0.6 (NDK r25b).
>
> Sure, but the issue is the libc++, not clang. A older libc++ is included in r25, see [android/ndk#1530 (comment)](https://github.com/android/ndk/issues/1530#issuecomment-1659055600)
Right. I mean, even the latest Qt codebase cannot be compiled with NDK > 25.
💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925319950)
@petertodd
> Why do you think Mara was mining OFAC compliant blocks for _short-term_ profit? They were leaving fees on the table, and I'm unaware of any short term financial incentive they had to to block those transactions.
I would think this is obvious. Filtering OFAC-sanctioned transactions costs a mining pool at most [a few hundred dollars per day](https://b10c.me/observations/08-missing-sanctioned-transactions/), while [maintaining a good ESG score](https://www.mara.com/about-us#esg) a
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1925319950)
@petertodd
> Why do you think Mara was mining OFAC compliant blocks for _short-term_ profit? They were leaving fees on the table, and I'm unaware of any short term financial incentive they had to to block those transactions.
I would think this is obvious. Filtering OFAC-sanctioned transactions costs a mining pool at most [a few hundred dollars per day](https://b10c.me/observations/08-missing-sanctioned-transactions/), while [maintaining a good ESG score](https://www.mara.com/about-us#esg) a
...
📝 furszy opened a pull request: "wallet: remove unused 'accept_no_keys' arg from decryption process"
(https://github.com/bitcoin/bitcoin/pull/29375)
Found it while reviewing other PR. Couldn't contain myself from cleaning it up.
The wallet decryption process (`CheckDecryptionKey()` and `Unlock()`)
contains an arg 'accept_no_keys,' introduced in #13926, that has
never been used.
Additionally, this also removes the unimplemented `SplitWalletPath`
function.
(https://github.com/bitcoin/bitcoin/pull/29375)
Found it while reviewing other PR. Couldn't contain myself from cleaning it up.
The wallet decryption process (`CheckDecryptionKey()` and `Unlock()`)
contains an arg 'accept_no_keys,' introduced in #13926, that has
never been used.
Additionally, this also removes the unimplemented `SplitWalletPath`
function.
📝 Chicook opened a pull request: "Create .edicomfigSG"
(https://github.com/bitcoin/bitcoin/pull/29376)
I hope this suggestion helps.
<!--
*** 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 impr
...
(https://github.com/bitcoin/bitcoin/pull/29376)
I hope this suggestion helps.
<!--
*** 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 impr
...
✅ fanquake closed a pull request: "Create .edicomfigSG"
(https://github.com/bitcoin/bitcoin/pull/29376)
(https://github.com/bitcoin/bitcoin/pull/29376)
📝 TheCharlatan opened a pull request: "test: Add makefile target for running unit tests"
(https://github.com/bitcoin/bitcoin/pull/29377)
`make check` runs a bunch of other subtree tests that exercise code that is hardly ever changed and have a comparatively long runtime. There seems to be no target for running just the unit tests, so add one.
Alternatively the secp256k1 tests could be removed from the `check-local` target, reducing its runtime.
(https://github.com/bitcoin/bitcoin/pull/29377)
`make check` runs a bunch of other subtree tests that exercise code that is hardly ever changed and have a comparatively long runtime. There seems to be no target for running just the unit tests, so add one.
Alternatively the secp256k1 tests could be removed from the `check-local` target, reducing its runtime.
⚠️ cornwarecjp opened an issue: "Incorrect amount in transaction page"
(https://github.com/bitcoin/bitcoin/issues/29378)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
In the Bitcoin Core GUI, I exported the transaction list to csv. Using a spreadsheet program, I added up all transaction amounts in the CSV file. The sum didn't equal the available amount on the overview page of Bitcoin Core.
The available amount on the overview page *does* correspond to the sum of my UTXO amounts (as seen using coin control in the Bitcoin Core GUI). It also corresponds
...
(https://github.com/bitcoin/bitcoin/issues/29378)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
In the Bitcoin Core GUI, I exported the transaction list to csv. Using a spreadsheet program, I added up all transaction amounts in the CSV file. The sum didn't equal the available amount on the overview page of Bitcoin Core.
The available amount on the overview page *does* correspond to the sum of my UTXO amounts (as seen using coin control in the Bitcoin Core GUI). It also corresponds
...
💬 achow101 commented on issue "Incorrect amount in transaction page":
(https://github.com/bitcoin/bitcoin/issues/29378#issuecomment-1925469320)
Can you try using 26.0? Display of send to self txs in the gui (and therefore in what the csv contains) was changed in 26.0 and I think that will more accurately reflect the state of these transactions.
(https://github.com/bitcoin/bitcoin/issues/29378#issuecomment-1925469320)
Can you try using 26.0? Display of send to self txs in the gui (and therefore in what the csv contains) was changed in 26.0 and I think that will more accurately reflect the state of these transactions.