💬 RicYashiroLee commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1666625863)
> > The size of OP_RETURN should ideally be larger to accommodate both a hash, and a reference to something outside of Bitcoin as the smallest way to make that reference / consume the least space.
>
> You can store 80 bytes in an op return. A SHA256 hash is 32 bytes.
>
> Why do you need more than 48 bytes for a reference?
Mostly, Bitcoin's TINY SCOPE has no space for unrelated non-primary garbage data, desincentivizes bitcoiners of running full nodes and implicitly hurt Bitcoin's true d
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1666625863)
> > The size of OP_RETURN should ideally be larger to accommodate both a hash, and a reference to something outside of Bitcoin as the smallest way to make that reference / consume the least space.
>
> You can store 80 bytes in an op return. A SHA256 hash is 32 bytes.
>
> Why do you need more than 48 bytes for a reference?
Mostly, Bitcoin's TINY SCOPE has no space for unrelated non-primary garbage data, desincentivizes bitcoiners of running full nodes and implicitly hurt Bitcoin's true d
...
💬 iBeizsley commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1666629910)
> > You can store 80 bytes in an op return. A SHA256 hash is 32 bytes.
> > Why do you need more than 48 bytes for a reference?
>
> Varies depending on the actual URI
>
> Assuming a SHA-1 hash of the following, but stored in binary form of 20 bytes 0e7364296be76d9ce5bc53dee6a15024d139760c
>
> How should something like this be stored in 60 bytes https://github.com/PoodleLabs/PoodleLabs.BitcoinBinaryVerificationScripts
The hash itself can act as a resource identifier, and the following
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1666629910)
> > You can store 80 bytes in an op return. A SHA256 hash is 32 bytes.
> > Why do you need more than 48 bytes for a reference?
>
> Varies depending on the actual URI
>
> Assuming a SHA-1 hash of the following, but stored in binary form of 20 bytes 0e7364296be76d9ce5bc53dee6a15024d139760c
>
> How should something like this be stored in 60 bytes https://github.com/PoodleLabs/PoodleLabs.BitcoinBinaryVerificationScripts
The hash itself can act as a resource identifier, and the following
...
💬 coinables commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666668653)
> > NACK, zero data to back-up PR claim that disallowing non-multisig p2sh would reduce spam, pure conjecture. A quick review of block data shows insignificant amount of non-multisig p2sh usage (not spam). If they pay the fee, it's not spam.
>
> with respect, I think there is [historical data AND precedent](https://blog.bitmex.com/dapps-or-only-bitcoin-transactions-the-2014-debate/) to suggest that default policies can reduce the amount of onchain spam. (ie the reduction in counterparty spam
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666668653)
> > NACK, zero data to back-up PR claim that disallowing non-multisig p2sh would reduce spam, pure conjecture. A quick review of block data shows insignificant amount of non-multisig p2sh usage (not spam). If they pay the fee, it's not spam.
>
> with respect, I think there is [historical data AND precedent](https://blog.bitmex.com/dapps-or-only-bitcoin-transactions-the-2014-debate/) to suggest that default policies can reduce the amount of onchain spam. (ie the reduction in counterparty spam
...
💬 Symphonic3 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666722223)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666722223)
Concept ACK
💬 YellowRoseCx commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1666725276)
> > why not focus efforts on reducing the need for Replace by Fee to begin with?
>
> As I and others have explained above in the linked resources, eg https://petertodd.org/2023/fullrbf-multiparty-protocols and https://petertodd.org/2023/why-you-should-run-mempoolfullrbf, it is impossible to eliminate the need for transaction replacement.
After reading your blog posts and thinking about it for the day, I've got some questions about it if you don't mind.
One of the issues you write about i
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1666725276)
> > why not focus efforts on reducing the need for Replace by Fee to begin with?
>
> As I and others have explained above in the linked resources, eg https://petertodd.org/2023/fullrbf-multiparty-protocols and https://petertodd.org/2023/why-you-should-run-mempoolfullrbf, it is impossible to eliminate the need for transaction replacement.
After reading your blog posts and thinking about it for the day, I've got some questions about it if you don't mind.
One of the issues you write about i
...
📝 martinus opened a pull request: "util/blockstorage: Add `TRACE_RAII`, slightly faster -reindex-chainstate with CBufferedFile"
(https://github.com/bitcoin/bitcoin/pull/28226)
TLDR:
* Adds a `TRACE_RAII` macro to easily trace runtime of a code block.
* Switch to `CBufferedFile` in `BlockManager::ReadBlockFromDisk` is slightly faster:
* 9% faster unserialization => * 1.2% faster `-reindex-chainstate`
---
While profiling `-reindex-changestate` I saw lots of `fread()` calls in in `BlockManager::ReadBlockFromDisk`. This replaces the use of `CAutoFile` with `CBufferedFile` with a small buffer, leading to much fewer calls to
`fread()`, which gives a little speedup
...
(https://github.com/bitcoin/bitcoin/pull/28226)
TLDR:
* Adds a `TRACE_RAII` macro to easily trace runtime of a code block.
* Switch to `CBufferedFile` in `BlockManager::ReadBlockFromDisk` is slightly faster:
* 9% faster unserialization => * 1.2% faster `-reindex-chainstate`
---
While profiling `-reindex-changestate` I saw lots of `fread()` calls in in `BlockManager::ReadBlockFromDisk`. This replaces the use of `CAutoFile` with `CBufferedFile` with a small buffer, leading to much fewer calls to
`fread()`, which gives a little speedup
...
💬 MarcoFalke commented on pull request "util/blockstorage: Add `TRACE_RAII`, slightly faster -reindex-chainstate with CBufferedFile":
(https://github.com/bitcoin/bitcoin/pull/28226#issuecomment-1666759670)
Shouldn't `std::fread` be already cached? I could imagine the result to be different on different hardware? Also, `-reindex-chainstate` seems rare enough to not spend time to optimize its runtime? Going further, if the only goal of `CBufferedFile` is to speed up `-reindex` by 1%, we could consider removing it. Since the implementation is put in the header file of `streams.h`, removing it could reduce peak compile memory usage by .5% and compilation speed by .3% on every fresh compilation IIRC. O
...
(https://github.com/bitcoin/bitcoin/pull/28226#issuecomment-1666759670)
Shouldn't `std::fread` be already cached? I could imagine the result to be different on different hardware? Also, `-reindex-chainstate` seems rare enough to not spend time to optimize its runtime? Going further, if the only goal of `CBufferedFile` is to speed up `-reindex` by 1%, we could consider removing it. Since the implementation is put in the header file of `streams.h`, removing it could reduce peak compile memory usage by .5% and compilation speed by .3% on every fresh compilation IIRC. O
...
💬 MarcoFalke commented on issue "fuzz:crc32c::ExtendArm64 Undefined symbols for architecture arm64:":
(https://github.com/bitcoin/bitcoin/issues/28223#issuecomment-1666762164)
Closing for now. Let us know if this is still an issue or if you have any other questions.
(https://github.com/bitcoin/bitcoin/issues/28223#issuecomment-1666762164)
Closing for now. Let us know if this is still an issue or if you have any other questions.
✅ MarcoFalke closed an issue: "fuzz:crc32c::ExtendArm64 Undefined symbols for architecture arm64:"
(https://github.com/bitcoin/bitcoin/issues/28223)
(https://github.com/bitcoin/bitcoin/issues/28223)
💬 MarcoFalke commented on pull request "Bump python minimum version to 3.9":
(https://github.com/bitcoin/bitcoin/pull/28211#issuecomment-1666771306)
> Isn't Stream a rolling release? What about RHEL?
I don't think Stream it is a rolling release. It should ship all the same package *versions* as the corresponding RHEL release (however, the binaries may not be bit-by-bit-identical).
I've adjusted the pull request description to say `CentOS-like 8/9`.
You can verify it locally by running a fresh install of CentOS 9 Stream / Alma Linux 9 / Rocky Linux 9 and typing `python3 --version`. For me, it gives: `Python 3.9.17`. Trying to install
...
(https://github.com/bitcoin/bitcoin/pull/28211#issuecomment-1666771306)
> Isn't Stream a rolling release? What about RHEL?
I don't think Stream it is a rolling release. It should ship all the same package *versions* as the corresponding RHEL release (however, the binaries may not be bit-by-bit-identical).
I've adjusted the pull request description to say `CentOS-like 8/9`.
You can verify it locally by running a fresh install of CentOS 9 Stream / Alma Linux 9 / Rocky Linux 9 and typing `python3 --version`. For me, it gives: `Python 3.9.17`. Trying to install
...
💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1666779214)
> (Although downgrading might be something we want to support?)
It is. You can simply set `-persistmempoolv1=1`.
(https://github.com/bitcoin/bitcoin/pull/28207#issuecomment-1666779214)
> (Although downgrading might be something we want to support?)
It is. You can simply set `-persistmempoolv1=1`.
💬 TheCharlatan commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1666788309)
ACK 1c976c691cc4b20f43071aabf36c7afed1571057
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1666788309)
ACK 1c976c691cc4b20f43071aabf36c7afed1571057
💬 MarcoFalke commented on issue "Flaky `wallet_transactiontime_rescan.py --legacy-wallet` functional test":
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1666789562)
Looks like it did got dispatched to thread 2, but thread 0 was blocking for 24 minutes:
```
test 2023-08-04T17:19:25.474000Z TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-rpcwallet=encrypted_wallet', 'walletpassphrase', 'passphrase', '1']
node0 2023-08-04T17:19:25.478233Z (mocktime: 2023-09-03T17:11:37Z) [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for /wallet/encrypted_wallet from 127.0.0.1:56776
node0 2023-08-04T17:19:25.478352Z (mocktime:
...
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1666789562)
Looks like it did got dispatched to thread 2, but thread 0 was blocking for 24 minutes:
```
test 2023-08-04T17:19:25.474000Z TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-rpcwallet=encrypted_wallet', 'walletpassphrase', 'passphrase', '1']
node0 2023-08-04T17:19:25.478233Z (mocktime: 2023-09-03T17:11:37Z) [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for /wallet/encrypted_wallet from 127.0.0.1:56776
node0 2023-08-04T17:19:25.478352Z (mocktime:
...
💬 MarcoFalke commented on issue "Flaky `wallet_transactiontime_rescan.py --legacy-wallet` functional test":
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1666789809)
Probably an upstream GCE outage?
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1666789809)
Probably an upstream GCE outage?
💬 MarcoFalke commented on pull request "test: create wallet specific for test_locked_wallet case":
(https://github.com/bitcoin/bitcoin/pull/28139#discussion_r1285185051)
I wonder why implicit fee estimation passed in the first place. Previously there were two inputs and a fee of 550sat. Now there is a fee of 2200 sat with one input.
Mind creating a pull, as I can't test intermittent issues? I think this needs clarification/fixup even absent an intermittent issue.
(https://github.com/bitcoin/bitcoin/pull/28139#discussion_r1285185051)
I wonder why implicit fee estimation passed in the first place. Previously there were two inputs and a fee of 550sat. Now there is a fee of 2200 sat with one input.
Mind creating a pull, as I can't test intermittent issues? I think this needs clarification/fixup even absent an intermittent issue.
💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1666799645)
> This will remove Cirrus from the loop entirely
On a second thought, I don't think this is possible. We use Cirrus CI compute credits to run the linters *immediately*, which take a few seconds. I don't think there is another CI provider that offers this feature, and I think this feature is well worth it to pay for (usually USD$ ~0.01 per run)
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1666799645)
> This will remove Cirrus from the loop entirely
On a second thought, I don't think this is possible. We use Cirrus CI compute credits to run the linters *immediately*, which take a few seconds. I don't think there is another CI provider that offers this feature, and I think this feature is well worth it to pay for (usually USD$ ~0.01 per run)
💬 1ma commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666801219)
> This propaganda needs to stop. OP_RETURN is not bad. In case you forgot it's inspcriptions that have led to the largest blocks ever on bitcoin, not bare p2sh.
>
> If we followed the logic proposed in your response we should also roll back taproot and replace the limit on the script sigs. Spam from taproot lifting the limit on scriptsigs far exceeds the legitimate uses of all p2sh usage. Lifting the script sig without validating the input is a bug no one wants to admit exists. I can decode a
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666801219)
> This propaganda needs to stop. OP_RETURN is not bad. In case you forgot it's inspcriptions that have led to the largest blocks ever on bitcoin, not bare p2sh.
>
> If we followed the logic proposed in your response we should also roll back taproot and replace the limit on the script sigs. Spam from taproot lifting the limit on scriptsigs far exceeds the legitimate uses of all p2sh usage. Lifting the script sig without validating the input is a bug no one wants to admit exists. I can decode a
...
💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1285189804)
Thanks, good catch. Fixed typo.
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1285189804)
Thanks, good catch. Fixed typo.
💬 MarcoFalke commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1285189851)
v1=false means v2=true, which is what you are asking for and what this pull request is doing, no?
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1285189851)
v1=false means v2=true, which is what you are asking for and what this pull request is doing, no?
🤔 zkfrio requested changes to a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1564119825)
NACK
After reading recent [discussion](https://github.com/bitcoin/bitcoin/pull/27578) about standardness, I think we should relax some standardness rules in Bitcoin Core if P2P has to remain relevant for transaction relay of a censorship resistant network.
Even if `false` is made default in next version, there will be some nodes and miners allowing bare multisig and that's all a normal user needs to use bare multisig.
(https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1564119825)
NACK
After reading recent [discussion](https://github.com/bitcoin/bitcoin/pull/27578) about standardness, I think we should relax some standardness rules in Bitcoin Core if P2P has to remain relevant for transaction relay of a censorship resistant network.
Even if `false` is made default in next version, there will be some nodes and miners allowing bare multisig and that's all a normal user needs to use bare multisig.