💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1665802905)
Thanks for the review and patch @vasild, I will try to review it over the weekend and keep this moving forward.
I have in the mean time been considering a totally different approach to thread-safe deletion of nodes from `CConnman`, but it's somewhat more invasive (in terms of LOC)... I was experimenting with whether `m_nodes` would work better as a vector of `share_ptr`, meaning they could be added and removed from `m_nodes` (and `m_nodes_disconnected`) much more safely from the perspective o
...
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1665802905)
Thanks for the review and patch @vasild, I will try to review it over the weekend and keep this moving forward.
I have in the mean time been considering a totally different approach to thread-safe deletion of nodes from `CConnman`, but it's somewhat more invasive (in terms of LOC)... I was experimenting with whether `m_nodes` would work better as a vector of `share_ptr`, meaning they could be added and removed from `m_nodes` (and `m_nodes_disconnected`) much more safely from the perspective o
...
💬 ryanofsky commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1284581060)
> Yeah ok, that is also a possibility. Great.
>
> Could the fix be decoupled into a standalone PR? Sounds like something that we could rapidly merge independently of this PR. It fixes a bug and also makes this work smaller.
Yes, I am definitely looking for suggestions on how many of the first commits it would make sense to split off into a separate PR.
But just to be clear, there isn't a bug here, just suboptimal behavior. If -reindex-chainstate is used and there are still block-connect
...
(https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1284581060)
> Yeah ok, that is also a possibility. Great.
>
> Could the fix be decoupled into a standalone PR? Sounds like something that we could rapidly merge independently of this PR. It fixes a bug and also makes this work smaller.
Yes, I am definitely looking for suggestions on how many of the first commits it would make sense to split off into a separate PR.
But just to be clear, there isn't a bug here, just suboptimal behavior. If -reindex-chainstate is used and there are still block-connect
...
💬 theuni commented on pull request "[WIP] guix: use `-muse-unaligned-vector-move` for Windows builds":
(https://github.com/bitcoin/bitcoin/pull/28151#issuecomment-1665814980)
> > Any reason not to add it to our configure directly rather than guix?
>
> I think that's actually all we can do here, adding this (to configure) as a best-effort for anyone using an unpatched compiler (where it'd be most beneficial when building `bitcoind.exe`), while also retaining our GCC patching in Guix.
Are you sure those two aren't mutually exclusive? If so, yeah, that sounds reasonable. Though I think we should make an effort to [produce working binaries](https://github.com/bitc
...
(https://github.com/bitcoin/bitcoin/pull/28151#issuecomment-1665814980)
> > Any reason not to add it to our configure directly rather than guix?
>
> I think that's actually all we can do here, adding this (to configure) as a best-effort for anyone using an unpatched compiler (where it'd be most beneficial when building `bitcoind.exe`), while also retaining our GCC patching in Guix.
Are you sure those two aren't mutually exclusive? If so, yeah, that sounds reasonable. Though I think we should make an effort to [produce working binaries](https://github.com/bitc
...
💬 jesterhodl commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665828005)
As a Bitcoin Node runner, Lightning Node operator, Nostr Relay operator and Nostr user, I recognize the problem @ChristopherA is looking to alleviate.
I recognize the problem is in the realm of authentication where cryptographic primitives and an immutable storage could be one option to pursue for a potential solution.
As a Bitcoiner I Concept NACK because:
* Timechain is for monetary data, other uses should forever stay as tolerable, low volume noise, or it will *decrease our capacity to
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665828005)
As a Bitcoin Node runner, Lightning Node operator, Nostr Relay operator and Nostr user, I recognize the problem @ChristopherA is looking to alleviate.
I recognize the problem is in the realm of authentication where cryptographic primitives and an immutable storage could be one option to pursue for a potential solution.
As a Bitcoiner I Concept NACK because:
* Timechain is for monetary data, other uses should forever stay as tolerable, low volume noise, or it will *decrease our capacity to
...
💬 vasild commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1665852012)
Yes! I was thinking about the same! We are doing some manual refcounting here which is creating all kinds of headaches and is prone to bugs. There is a neat solution already for this `std::shared_ptr`. Long term, this is a must have IMO.
To minimize the size of the initial PR, I guess you can just change `CConnman::m_nodes` and `CConnman::m_nodes_disconnected` from `std::vector<CNode*>` to `std::vector<CNodeRef>` and the needed changes to get that to compile and work.
Later I think we don'
...
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1665852012)
Yes! I was thinking about the same! We are doing some manual refcounting here which is creating all kinds of headaches and is prone to bugs. There is a neat solution already for this `std::shared_ptr`. Long term, this is a must have IMO.
To minimize the size of the initial PR, I guess you can just change `CConnman::m_nodes` and `CConnman::m_nodes_disconnected` from `std::vector<CNode*>` to `std::vector<CNodeRef>` and the needed changes to get that to compile and work.
Later I think we don'
...
💬 rot13maxi commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665852027)
For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a
This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665852027)
For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a
This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1284610972)
_moving discussion from https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138_
> 4. Finally, on signet I never got an INV for one of my own txs back. so getmempoolinfo : unbroadcastcount only ever increased. I notice that the tests only use python P2PInterface()'s for peers and the observer_outbound peer and is manually forced to inv our txid... makes me wonder if real bitcoind peers have different enough relay behavior that it doesnt work as well in production. For example, w
...
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1284610972)
_moving discussion from https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138_
> 4. Finally, on signet I never got an INV for one of my own txs back. so getmempoolinfo : unbroadcastcount only ever increased. I notice that the tests only use python P2PInterface()'s for peers and the observer_outbound peer and is manually forced to inv our txid... makes me wonder if real bitcoind peers have different enough relay behavior that it doesnt work as well in production. For example, w
...
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1665867224)
`6e6f83b0f7...36074c093b`: rebase, fix a bug where [we sent the transaction without the witness](https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1284610972), and extend the test to cover that
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1665867224)
`6e6f83b0f7...36074c093b`: rebase, fix a bug where [we sent the transaction without the witness](https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1284610972), and extend the test to cover that
💬 RobinLinus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665884417)
> This creates many unprunable outputs
This is their pitch though _"Unprunable UTXO Art"_ . They are intentionally spamming the chain. Relaxing bitcoin's spam mitigations doesn't help against that.
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665884417)
> This creates many unprunable outputs
This is their pitch though _"Unprunable UTXO Art"_ . They are intentionally spamming the chain. Relaxing bitcoin's spam mitigations doesn't help against that.
💬 MajesticBank commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665898833)
With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
Maybe that's the thing we don't like in this proposal?
Anyone claiming they are accepting 0-conf bitcoin transaction over internet is spreading FUD and that having full-rbf by default does anything bad to their non-existent payment system.
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665898833)
With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
Maybe that's the thing we don't like in this proposal?
Anyone claiming they are accepting 0-conf bitcoin transaction over internet is spreading FUD and that having full-rbf by default does anything bad to their non-existent payment system.
💬 1ma commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665903498)
> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a
This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
The stamps "community" can only route around OP_RETURN because back in 2014 Mike Hearn derailed the discussion to m
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665903498)
> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a
This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
The stamps "community" can only route around OP_RETURN because back in 2014 Mike Hearn derailed the discussion to m
...
💬 RicYashiroLee commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665924680)
IMV from too many commentary I have seen already from Peter, this is just another massive example of altcoining mindset, of many wishful thinking ideas coming from Peter Todd, detrimental to Bitcoin Full Node Operators, and only giving, as a direct consequence, more power to a central hub of Core Devs who are in high risk of conflict of interests, as he is included in such risk group of service providers and centralizers, 'scaling' business oriented. Full Node operators, on the contrary, are the
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665924680)
IMV from too many commentary I have seen already from Peter, this is just another massive example of altcoining mindset, of many wishful thinking ideas coming from Peter Todd, detrimental to Bitcoin Full Node Operators, and only giving, as a direct consequence, more power to a central hub of Core Devs who are in high risk of conflict of interests, as he is included in such risk group of service providers and centralizers, 'scaling' business oriented. Full Node operators, on the contrary, are the
...
💬 YellowRoseCx commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665933394)
> With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
>
> Maybe that's the thing we don't like in this proposal?
>
> Anyone claiming they are accepting 0-conf bitcoin transaction over internet is spreading FUD and that having full-rbf by default does anything bad to their non-existent payment system.
How long do you want Bitcoin users to stand waiting in a store before they can leave with their groceri
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665933394)
> With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
>
> Maybe that's the thing we don't like in this proposal?
>
> Anyone claiming they are accepting 0-conf bitcoin transaction over internet is spreading FUD and that having full-rbf by default does anything bad to their non-existent payment system.
How long do you want Bitcoin users to stand waiting in a store before they can leave with their groceri
...
💬 jonatack commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284653088)
835f094 It looks like this empty vector might be declarable `constexpr`, at least with clang 16.0.6 arm64.
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284653088)
835f094 It looks like this empty vector might be declarable `constexpr`, at least with clang 16.0.6 arm64.
🤔 jonatack reviewed a pull request: "refactor: Add util::Result failure values, multiple error and warning messages"
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1563250744)
ACK 9e80d0b754a28733c79a52c8e0431616c31d071c
Nice documentation, code, and commit message improvements. Some non-blocking comments.
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1563250744)
ACK 9e80d0b754a28733c79a52c8e0431616c31d071c
Nice documentation, code, and commit message improvements. Some non-blocking comments.
💬 jonatack commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284654294)
9ec1bda Can `Base` here be private instead of protected?
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284654294)
9ec1bda Can `Base` here be private instead of protected?
💬 jonatack commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284664601)
9ec1bda
```
src/util/result.h:300: OT ==> TO, OF, OR, NOT
src/util/result.h:301: OT ==> TO, OF, OR, NOT
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1284664601)
9ec1bda
```
src/util/result.h:300: OT ==> TO, OF, OR, NOT
src/util/result.h:301: OT ==> TO, OF, OR, NOT
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665939229)
> With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
That is correct. The BIP125 flag is a way to distinguish transactions from different wallets, and Chainanalysis certainly makes use of this. Deployment of Full-RBF would eliminate the need for any wallet to set BIP125 flags, allowing all wallets to eventually transition to having identical characteristics.
> Anyone claiming they are accepting 0-conf bi
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1665939229)
> With having full-rbf by default privacy would be increased for transactions created by bitcoin core, this is very important pull request.
That is correct. The BIP125 flag is a way to distinguish transactions from different wallets, and Chainanalysis certainly makes use of this. Deployment of Full-RBF would eliminate the need for any wallet to set BIP125 flags, allowing all wallets to eventually transition to having identical characteristics.
> Anyone claiming they are accepting 0-conf bi
...
💬 RicYashiroLee commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665951546)
> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
The key word IMV is making things cumbersom for any out of scope use of Bitcoin, not more relaxed. And enable Node o
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665951546)
> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETURN limit and doing things like embedding data in bare multisig: https://mempool.space/tx/1c61f2d4000e1bc2ce27ec24e4fcbbb376c2bd19641e071403e68d84d48eea8a This creates many unprunable outputs and takes up more blockspace, so its bad for block capacity and utxoset bloat.
The key word IMV is making things cumbersom for any out of scope use of Bitcoin, not more relaxed. And enable Node o
...
💬 rot13maxi commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665971141)
If you believe that core shouldnt be making policy decisions on behalf of users, then shouldnt the default be that there is no default policy and that users can set it to whatever they want? Thats what this PR would do (for this policy item).
On Fri, Aug 4, 2023 at 1:25 PM, Richard Yashiro Lee ***@***.***(mailto:On Fri, Aug 4, 2023 at 1:25 PM, Richard Yashiro Lee <<a href=)> wrote:
>> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETUR
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1665971141)
If you believe that core shouldnt be making policy decisions on behalf of users, then shouldnt the default be that there is no default policy and that users can set it to whatever they want? Thats what this PR would do (for this policy item).
On Fri, Aug 4, 2023 at 1:25 PM, Richard Yashiro Lee ***@***.***(mailto:On Fri, Aug 4, 2023 at 1:25 PM, Richard Yashiro Lee <<a href=)> wrote:
>> For readers of the thread who are not familiar with the STAMPS community, people are going around the OP_RETUR
...