💬 Sun0fABeach commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667370041)
> Concept NACK but I would be very amused to see this get merged. We will engineer around it, of course.
>
> We will go straight to mining pools if we have to (who LOVE to include our high fee transactions).
Since you're saying these changes won't cause any issues for you, we can agree to move forward with the PR.
> We will continue to use the permissionless Bitcoin protocol as we see fit. #FREEDOM
I'm sure you don't have any spam filters on your e-mail inbox since that would be cens
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667370041)
> Concept NACK but I would be very amused to see this get merged. We will engineer around it, of course.
>
> We will go straight to mining pools if we have to (who LOVE to include our high fee transactions).
Since you're saying these changes won't cause any issues for you, we can agree to move forward with the PR.
> We will continue to use the permissionless Bitcoin protocol as we see fit. #FREEDOM
I'm sure you don't have any spam filters on your e-mail inbox since that would be cens
...
📝 TheCharlatan opened a pull request: "kernel: Run sanity checks on context construction"
(https://github.com/bitcoin/bitcoin/pull/28228)
Moving the kernel sanity check code to kernel context instantiation time ensures that it is impossible to create an invalid kernel context. This is based on a suggestion made in a [comment](https://github.com/bitcoin/bitcoin/pull/25065#discussion_r882182549) by @theuni in the [original pull request](https://github.com/bitcoin/bitcoin/pull/25065) first introducing the `kernel::Context`.
Since this touches the daemon initialization logic, also fix a bug where it would be possible for a terminat
...
(https://github.com/bitcoin/bitcoin/pull/28228)
Moving the kernel sanity check code to kernel context instantiation time ensures that it is impossible to create an invalid kernel context. This is based on a suggestion made in a [comment](https://github.com/bitcoin/bitcoin/pull/25065#discussion_r882182549) by @theuni in the [original pull request](https://github.com/bitcoin/bitcoin/pull/25065) first introducing the `kernel::Context`.
Since this touches the daemon initialization logic, also fix a bug where it would be possible for a terminat
...
💬 viresinnumeris-1 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1667376865)
NACK
Clearly, some are going to be adversely affected by this PR if it gets merged.
Bitcoin Core does not belong to any single group. It should not be used to push personal agendas.
Let the node runners decide. Please don't assume they don't know what they're doing and push what _you_ think is best onto others.
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1667376865)
NACK
Clearly, some are going to be adversely affected by this PR if it gets merged.
Bitcoin Core does not belong to any single group. It should not be used to push personal agendas.
Let the node runners decide. Please don't assume they don't know what they're doing and push what _you_ think is best onto others.
💬 russeree commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667380163)
> Making the spam attacks more costly is the rationale behind this change. And the guy who runs the attack is trying to stop this spam mitigation, which is a strong indicator that he believes this change will be effective.
What about P2PK then? It could still be used for the exact same purpose to bloat the UTXO set.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667380163)
> Making the spam attacks more costly is the rationale behind this change. And the guy who runs the attack is trying to stop this spam mitigation, which is a strong indicator that he believes this change will be effective.
What about P2PK then? It could still be used for the exact same purpose to bloat the UTXO set.
💬 RobinLinus commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667394446)
> used for the exact same purpose to bloat the UTXO set
It is more costly to bloat the UTXO set with P2PK, which is exactly the intended purpose of this spam mitigation.
Still, you're right that it makes sense to open a second PR to make p2pk non-standard as well when it gets abused.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667394446)
> used for the exact same purpose to bloat the UTXO set
It is more costly to bloat the UTXO set with P2PK, which is exactly the intended purpose of this spam mitigation.
Still, you're right that it makes sense to open a second PR to make p2pk non-standard as well when it gets abused.
💬 MarcoFalke commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1667401501)
Thanks, I've split each commit to be iwyu targeted to one header/module. The overall force-push diff is zero.
(https://github.com/bitcoin/bitcoin/pull/28200#issuecomment-1667401501)
Thanks, I've split each commit to be iwyu targeted to one header/module. The overall force-push diff is zero.
🤔 mzumsande reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1564808157)
My understanding is that there is a bit of a trade-off with respect to memory (correct me if the byte counting is off!):
While it saves 33kB per peer (so ~4MB with 125 peers; ~250kB for non-listening nodes with 8 peers), it also adds at least 8 bytes per mempool entry for `uint64_t entry_sequence`.
(for me, `sizeof(CTxMemPoolEntry)` even grows from 264 to 280 bytes - packing effects?).
This seems a bit unsatisfactory because the `entry_sequence` data won't be needed after a few seconds (i.e.
...
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1564808157)
My understanding is that there is a bit of a trade-off with respect to memory (correct me if the byte counting is off!):
While it saves 33kB per peer (so ~4MB with 125 peers; ~250kB for non-listening nodes with 8 peers), it also adds at least 8 bytes per mempool entry for `uint64_t entry_sequence`.
(for me, `sizeof(CTxMemPoolEntry)` even grows from 264 to 280 bytes - packing effects?).
This seems a bit unsatisfactory because the `entry_sequence` data won't be needed after a few seconds (i.e.
...
💬 viresinnumeris-1 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667417085)
Concept ACK
Bare multisig has barely any non-spam use cases left.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667417085)
Concept ACK
Bare multisig has barely any non-spam use cases left.
💬 russeree commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667418511)
Idea, what about reducing the standardness to only accept compressed format public keys this reducing the capacity to spam?
Second I just scraped my UTXO set using https://github.com/in3rsha/bitcoin-utxo-dump and filtered my set for only p2ms keeping all row data and the total size is 278MB which is incredibly small even compared to P2PKH which sits at 7.9GB
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1667418511)
Idea, what about reducing the standardness to only accept compressed format public keys this reducing the capacity to spam?
Second I just scraped my UTXO set using https://github.com/in3rsha/bitcoin-utxo-dump and filtered my set for only p2ms keeping all row data and the total size is 278MB which is incredibly small even compared to P2PKH which sits at 7.9GB
🚀 fanquake merged a pull request: "refactor: Remove unused MessageStartChars parameters from BlockManager methods"
(https://github.com/bitcoin/bitcoin/pull/28191)
(https://github.com/bitcoin/bitcoin/pull/28191)
💬 gzuuus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1667509988)
Concept NACK. It is easy to see that bitcoin is neither an efficient nor convenient way to store data in the "cloud", so why support the use of a feature that does not benefit and goes against of bitcoin's value transaction and is inconvenient for node operators and users in general? Strongly agree with the above answers NACK
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1667509988)
Concept NACK. It is easy to see that bitcoin is neither an efficient nor convenient way to store data in the "cloud", so why support the use of a feature that does not benefit and goes against of bitcoin's value transaction and is inconvenient for node operators and users in general? Strongly agree with the above answers NACK
🤔 TheCharlatan reviewed a pull request: "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block"
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1564904754)
ACK 2ccbef905fb6faa2480b49103680529dc7cfc482
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1564904754)
ACK 2ccbef905fb6faa2480b49103680529dc7cfc482
💬 TheCharlatan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1285602074)
In commit 6229158f77a7b1803f2767a089dd94dedf62b533
Nit: You could just forward-declare `struct FlatFilePos` without having to include flatfile (same for `zmqpublishnotifier.h`).
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1285602074)
In commit 6229158f77a7b1803f2767a089dd94dedf62b533
Nit: You could just forward-declare `struct FlatFilePos` without having to include flatfile (same for `zmqpublishnotifier.h`).
💬 fanquake commented on pull request "p2p: diversity network outbounds follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28189#issuecomment-1667517304)
@amitiuttarwar want to undraft this now?
cc @mzumsande @willcl-ark @vasild @naumenkogs
(https://github.com/bitcoin/bitcoin/pull/28189#issuecomment-1667517304)
@amitiuttarwar want to undraft this now?
cc @mzumsande @willcl-ark @vasild @naumenkogs
💬 fanquake commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#issuecomment-1667535093)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28051#issuecomment-1667535093)
Concept ACK
💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285643464)
thx, done
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285643464)
thx, done
💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285644159)
Maybe submit a pull with your suggestion applied to another RPC? Once and if it is merged, I could rebase this one.
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1285644159)
Maybe submit a pull with your suggestion applied to another RPC? Once and if it is merged, I could rebase this one.
💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1667552061)
@real-or-random
> But I don't understand the numbers.
There was a [bug](https://github.com/cirruslabs/cirrus-ci-web/pull/572#issuecomment-1667038748), which has been fixed now.
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1667552061)
@real-or-random
> But I don't understand the numbers.
There was a [bug](https://github.com/cirruslabs/cirrus-ci-web/pull/572#issuecomment-1667038748), which has been fixed now.
💬 dergoegge commented on pull request "assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager":
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1667572204)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1667572204)
Concept ACK
💬 theStack commented on pull request "test: check for specific bip157 disconnect reasons, add test coverage":
(https://github.com/bitcoin/bitcoin/pull/28227#discussion_r1285678494)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/28227#discussion_r1285678494)
Thanks, fixed.