Bitcoin Core Github
44 subscribers
120K links
Download Telegram
šŸ¤” glozow reviewed a pull request: "policy: make pathological transactions packed with legacy sigops non-standard"
(https://github.com/bitcoin/bitcoin/pull/32521#pullrequestreview-2975393556)
Strong concept ACK. However, it seems that "this might be a soft fork in the future" is only a secondary motivation (and a bit of a presumptuous one). The primary motivation should be the same as what's in BIP 54: to avoid long validation times.

Generally, a PR that restricts default policy should have a corresponding post on the mailing list. I'm not that concerned that there is someone who needs to relay pathological transactions, but it would feel weird for Core to merge this without any a
...
āš ļø ekrembal opened an issue: "Internal bug detected: FinalizeAndExtractPSBT(psbtx_copy, mtx)"
(https://github.com/bitcoin/bitcoin/issues/32849)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

```
bitcoin-cli -testnet4 descriptorprocesspsbt cHNidP8BAF4CAAAAAfrAU8xRZDY7er/kAUGzVtup++LN4otxeOtANj9Tw8xYAAAAAAD9////AeAPlwAAAAAAIlEgmQIRpk7kW78uK6z/DBX37H4Pi4OLy3Fau0pXquQSXLUAAAAAAAEBK4CWmAAAAAAAIlEg5cugS+vKVwm3OSTEr0sIVU/ahEJzIS6iXjsvWNwJ0VUBAwSDAAAAAQhDAUFATOMkSjzzD+7k1AnzZgJ+G+sDZXX5ob0tlQ9mZIseDe73u4fU6SPKdtxve2Wm/mVNrRIP5cPEnkcMv0BJ2iIWgyEWtJa/uuFJh4F8U9WSvgqmbEXHuURDwfdFUTc/nONN
...
šŸ’¬ romanz commented on pull request "doc: add `/spenttxouts` to REST-interface.md":
(https://github.com/bitcoin/bitcoin/pull/32842#issuecomment-3024141446)
Thanks!
šŸ’¬ fanquake commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3024157584)
@theuni want to rebase this now that #32465 is in?
šŸ’¬ fanquake commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024213036)
Is this `v22.0` or `v26.0`: https://github.com/bitcoin/bitcoin/issues/32762#issuecomment-2978990456? Please only open issues when using a maintained version of Bitcoin Core, which is currently `27.x`, `28.x` or `29.x`.
šŸ’¬ ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-3024221935)
> Thanks, I've used parts of this suggestion but with some changes

Thanks for following up, and it's a good observation that calling `filename()` on an `fs::path` like I [suggested](https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-2821309098) doesn't always always return a usable file prefix, since it can return odd fragments like ".." and doesn't handle trailing slashes.

But I feel like it would still be straightforward to address the problem of turning a wallet name into a safe
...
šŸ’¬ saravadeanil commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024230141)
@fanquake Thanks for checking I am using `v22.0` for signet node.

I was able to fix the initial sync issue by `addnode` referring https://github.com/bitcoin/bitcoin/issues/32762#issuecomment-2977713689
šŸ’¬ sipa commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024239736)
Bitcoin Core has no websocket support whatsoever.
šŸ‘‹ l0rinc's pull request is ready for review: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827)
šŸ’¬ l0rinc commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#discussion_r2177745211)
That change seems riskier than the rest, we can do it in a follow-up.
The PR is ready for review.
šŸ’¬ maflcko commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024315066)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?
šŸ’¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177796421)
It seems to me the first historical tx that violates this new rule is https://mempool.space/tx/659135664894e50040830edb516a76f704fd2be409ecd8d1ea9916c002ab28a2 with 2585 sigops.
šŸ’¬ l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177807221)
> Apologies if that came across as patronizing

> if you insistently derail a PR with irrelevant concerns

no, they're not irrelevant or silly, I expect better arguments than these
šŸ’¬ pinheadmz commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177826383)
Can we please ask a different reviewer for an opinion on this one thing to break the tie before anyone's feelings get hurt?
šŸ’¬ pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2177843490)
> This should be && instead of ||.

Yeah, my bad.

> .isNull() check is not really reliable either, because it will happily accept an empty array.

At the moment you could do:
```
./build/bin/bitcoin-cli -signet -datadir=/tmp/btc getdescriptoractivity '[]' '[]'
{
"activity": [
]
}
```
And that doesn't make sense either.
> I think just making sure we only execute the request.params[n].get_array().getValues() statements if we've first checked that !request.params[n].isNull() sho
...
šŸ’¬ sipa commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177844858)
@l0rinc We should certainly not gratuitously make changes the worsen performance, but I think there have been reasonable arguments given here:
* It results in simpler code changes, specifically ones that impact consensus logic less.
* The slowdown in negligible compared the overall cost of transaction validation. It's good to be aware that there is a slowdown, but also reasonable to dismiss it on the basis that it's entirely justifiable in the presence of a much more important concern (reducin
...
šŸ’¬ darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2177851038)
> Do we have data on how many main-chain transactions historically exceed the new threshold of 2500 legacy sigops?
I’d like to confirm that the new policy would block no real-world traffic and is purely a preventive DoS measure ahead of any future BIP-54 soft-fork activation.

I don't think past usage is necessarily the best predictor, i think it's more convincing to reason through what real-world traffic could look like. The BIP [goes through this reasoning](https://github.com/bitcoin/bips/b
...
šŸ’¬ saravadeanil commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024436337)
I am trying to perform rawtx using websocket.

It think it works on bitcoin testnet & mainnet network with version v22.0.0.

I am wondering why it's not working with signet.
šŸ’¬ sipa commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024444002)
Can you elaborate on what you're using? Because as I mentioned, Bitcoin Core has no websocket support. You may be using JSON-RPC, or ZMQ, or a P2P connection...
šŸ’¬ saravadeanil commented on issue "[BTC signet v22.0] websocket not working as expect":
(https://github.com/bitcoin/bitcoin/issues/32848#issuecomment-3024449821)
@sipa Sorry for the confusion, I am using zmqpubrawtx port.