💬 jonatack commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1476798503)
Using an unordered set instead of a set, `find`/`contains` complexity can be reduced from O(log n) to O(1). Remember to add the `#include <unordered_set>` header in this case.
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1476798503)
Using an unordered set instead of a set, `find`/`contains` complexity can be reduced from O(log n) to O(1). Remember to add the `#include <unordered_set>` header in this case.
⚠️ nsvrn opened an issue: "IBD stalling issue in v26.0 and assumevalid=0"
(https://github.com/bitcoin/bitcoin/issues/29374)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm using `v26` with `assumevalid=0` and the IBD keeps stalling after the log message says `Potential stale tip detected, will try using extra outbound peer`, it retries but same message keeps repeating unless I restart the software.
I saw someone on bitcoin stack exchange suggested to not use VPN, I'm not using any VPN or tor. Just simple clearnet on a fresh Linux OS.
### Expected behav
...
(https://github.com/bitcoin/bitcoin/issues/29374)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I'm using `v26` with `assumevalid=0` and the IBD keeps stalling after the log message says `Potential stale tip detected, will try using extra outbound peer`, it retries but same message keeps repeating unless I restart the software.
I saw someone on bitcoin stack exchange suggested to not use VPN, I'm not using any VPN or tor. Just simple clearnet on a fresh Linux OS.
### Expected behav
...
💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1476798246)
I think this hasn't been reverted yet?
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1476798246)
I think this hasn't been reverted yet?
💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1476797365)
In 5fbf18b89d6768bed1be95dcbc98797df5788dfc "wallet: track mempool conflicts"
I'm a little bit confused by the "erase this wtx entry" part of the comment. AFAICT, no entries are being erased, Perhaps this should say:
```suggestion
// If this wtx has no other mempool conflicts, mark it as inactive
```
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1476797365)
In 5fbf18b89d6768bed1be95dcbc98797df5788dfc "wallet: track mempool conflicts"
I'm a little bit confused by the "erase this wtx entry" part of the comment. AFAICT, no entries are being erased, Perhaps this should say:
```suggestion
// If this wtx has no other mempool conflicts, mark it as inactive
```
✅ nsvrn closed an issue: "IBD stalling issue in v26.0 and assumevalid=0"
(https://github.com/bitcoin/bitcoin/issues/29374)
(https://github.com/bitcoin/bitcoin/issues/29374)
💬 nsvrn commented on issue "IBD stalling issue in v26.0 and assumevalid=0":
(https://github.com/bitcoin/bitcoin/issues/29374#issuecomment-1924808938)
Closed this issue for now, just realized that it seems to be an issue with internet connection going down on the machine. It could be unrelated to Bitcoin software so doesn't make sense to investigate further on this one.
(https://github.com/bitcoin/bitcoin/issues/29374#issuecomment-1924808938)
Closed this issue for now, just realized that it seems to be an issue with internet connection going down on the machine. It could be unrelated to Bitcoin software so doesn't make sense to investigate further on this one.
💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924814451)
> It seems to me like a more straightforward way to implement this feature would be to add a `bool mempool_conflicts` field to `TxStateInactive` rather than adding a new `TxStateMempoolConflicted` state. The description of `TxStateInactive` says "May conflict with the mempool, or with an unknown block, or be abandoned, never broadcast, or rejected from the mempool for another reason." So the inactive state was meant to handle transactions that could have conflicts or be inactive for a variety of
...
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1924814451)
> It seems to me like a more straightforward way to implement this feature would be to add a `bool mempool_conflicts` field to `TxStateInactive` rather than adding a new `TxStateMempoolConflicted` state. The description of `TxStateInactive` says "May conflict with the mempool, or with an unknown block, or be abandoned, never broadcast, or rejected from the mempool for another reason." So the inactive state was meant to handle transactions that could have conflicts or be inactive for a variety of
...
💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924833293)
@petertodd You appear not to have read my message at all. I will repeat myself inline where necessary.
> You're basically making an argument to altruism.
No, I'm not. I'm appealing to long-term self-interest for all involved. Altruism involves accepting harm to oneself so that others may benefit. Following Core's default mempool policy involves miners sacrificing _short-term profits_ in order to boost _long-term profits_. Please note that, on the contrary, what _you_ are suggesting involve
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924833293)
@petertodd You appear not to have read my message at all. I will repeat myself inline where necessary.
> You're basically making an argument to altruism.
No, I'm not. I'm appealing to long-term self-interest for all involved. Altruism involves accepting harm to oneself so that others may benefit. Following Core's default mempool policy involves miners sacrificing _short-term profits_ in order to boost _long-term profits_. Please note that, on the contrary, what _you_ are suggesting involve
...
💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924893374)
> The default mempool policy serves a similar purpose. Can miners circumvent these limits? Absolutely, but there are harsh consequences, and ultimately miners' interests are much better served by simply playing by the rules.
What are the harsh consequences of changing mempool policies in a permissionless network?
> "Convincing more hash power to set -permitbaremultisig=0" is precisely what I'm doing right now, by lobbying for this PR to be merged into bitcoin core. Sitting on our hands and
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924893374)
> The default mempool policy serves a similar purpose. Can miners circumvent these limits? Absolutely, but there are harsh consequences, and ultimately miners' interests are much better served by simply playing by the rules.
What are the harsh consequences of changing mempool policies in a permissionless network?
> "Convincing more hash power to set -permitbaremultisig=0" is precisely what I'm doing right now, by lobbying for this PR to be merged into bitcoin core. Sitting on our hands and
...
💬 furszy commented on pull request "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs":
(https://github.com/bitcoin/bitcoin/pull/28868#discussion_r1476878728)
in 78ba0e6748d:
I know that this shouldn't fail, but it would be good to log the error if it happens and inform the user in the error message that they need to reload the wallet manually.
(same for the others)
(https://github.com/bitcoin/bitcoin/pull/28868#discussion_r1476878728)
in 78ba0e6748d:
I know that this shouldn't fail, but it would be good to log the error if it happens and inform the user in the error message that they need to reload the wallet manually.
(same for the others)
🤔 furszy reviewed a pull request: "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs"
(https://github.com/bitcoin/bitcoin/pull/28868#pullrequestreview-1860637525)
Code review ACK 4da76ca2
Also verified that the tests fail when bug fix commits are reverted.
(https://github.com/bitcoin/bitcoin/pull/28868#pullrequestreview-1860637525)
Code review ACK 4da76ca2
Also verified that the tests fail when bug fix commits are reverted.
💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924930353)
> > The default mempool policy serves a similar purpose. Can miners circumvent these limits? Absolutely, but there are harsh consequences, and ultimately miners' interests are much better served by simply playing by the rules.
>
> What are the harsh consequences of changing mempool policies in a permissionless network?
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 m
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1924930353)
> > The default mempool policy serves a similar purpose. Can miners circumvent these limits? Absolutely, but there are harsh consequences, and ultimately miners' interests are much better served by simply playing by the rules.
>
> What are the harsh consequences of changing mempool policies in a permissionless network?
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 m
...
💬 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
...