💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#issuecomment-1646814647)
tACK
I applied this PRs logic with the `current branches ``.oneline_description``` and was able to get the desired error message.
(https://github.com/bitcoin/bitcoin/pull/28123#issuecomment-1646814647)
tACK
I applied this PRs logic with the `current branches ``.oneline_description``` and was able to get the desired error message.
💬 Ayms commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646816395)
Concept ACK bitcoin surpasses eth in every domain except storage
Current storage methods need two txs and many deviant methods were used in the past to store things in bitcoin, as we saw recently also
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646816395)
Concept ACK bitcoin surpasses eth in every domain except storage
Current storage methods need two txs and many deviant methods were used in the past to store things in bitcoin, as we saw recently also
💬 eval-exec commented on issue "Win64 CI failure in feature_versionbits_warning.py":
(https://github.com/bitcoin/bitcoin/issues/28115#issuecomment-1646817279)
> `AssertionError: [node 0] Node returned unexpected exit code (3) vs (0) when stopping`
Hello, I think a clue to solve this issue is to find out where the `exit code (3)` is returned? But I didn't find where the `exit code (3)` is returned in the source code.
What do you think?
(https://github.com/bitcoin/bitcoin/issues/28115#issuecomment-1646817279)
> `AssertionError: [node 0] Node returned unexpected exit code (3) vs (0) when stopping`
Hello, I think a clue to solve this issue is to find out where the `exit code (3)` is returned? But I didn't find where the `exit code (3)` is returned in the source code.
What do you think?
💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432003)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```
The output of ```ST
...
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432003)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```
The output of ```ST
...
💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432874)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```
The output of ```ST
...
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271432874)
The above suggestion doesn't compile without syntax changes due to ```error: unterminated argument list invoking macro "STR_INTERNAL_BUG"``` This is because of a missing ```)``` to close off the arguments for STR_INTERNAL_BUG and then there is a loose comma after ```m_opts.oneline_description```. Once these are removed code does compile but the output is not as clean as the current commit due to an extra ```"``` on a new line after the name of the ```oneline_description```
The output of ```ST
...
💬 russeree commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646824223)
Pretty sure this is the issue from your supplied ```Debug CLI.log"
```2023-07-22T13:42:39Z Syncing coinstatsindex with block chain from height 0```
```2023-07-22T13:42:39Z ERROR: Commit: Failed to commit latest coinstatsindex state```
adding the startup flag -reindex-chainstate should fix this issue. As per your previous concern you will be removing the levelDB indexes which are quite fast to rebuild. Approx ~7GB at hash 000000000000000000009b48061fe1b12f94ab54e71b289c5db97e0e5fd41874
...
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646824223)
Pretty sure this is the issue from your supplied ```Debug CLI.log"
```2023-07-22T13:42:39Z Syncing coinstatsindex with block chain from height 0```
```2023-07-22T13:42:39Z ERROR: Commit: Failed to commit latest coinstatsindex state```
adding the startup flag -reindex-chainstate should fix this issue. As per your previous concern you will be removing the levelDB indexes which are quite fast to rebuild. Approx ~7GB at hash 000000000000000000009b48061fe1b12f94ab54e71b289c5db97e0e5fd41874
...
💬 new-light-106 commented on issue "build: Windows debug cross-build fails with `-O0`":
(https://github.com/bitcoin/bitcoin/issues/28109#issuecomment-1646834821)
jahad57ghasem@gmail.com
(https://github.com/bitcoin/bitcoin/issues/28109#issuecomment-1646834821)
jahad57ghasem@gmail.com
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1271448113)
Fixed
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1271448113)
Fixed
💬 hebasto commented on issue "libsecp256k1 not instrumented when building with sanitizers":
(https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003)
FWIW, the libsecp256k1 is instrumented properly when using the CMake-based build system (see the full prototype [branch](https://github.com/hebasto/bitcoin/tree/pr25797.69) from https://github.com/bitcoin/bitcoin/pull/25797):
```
$ objdump --disassemble=secp256k1_xonly_pubkey_serialize build/src/test/fuzz/fuzz | grep __sanitizer_cov
d42e90: e8 8b 23 c0 ff call 945220 <__sanitizer_cov_trace_const_cmp8>
d42ff2: e8 39 21 c0 ff call 945130 <__sanitizer_cov_trace_pc_indi
...
(https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003)
FWIW, the libsecp256k1 is instrumented properly when using the CMake-based build system (see the full prototype [branch](https://github.com/hebasto/bitcoin/tree/pr25797.69) from https://github.com/bitcoin/bitcoin/pull/25797):
```
$ objdump --disassemble=secp256k1_xonly_pubkey_serialize build/src/test/fuzz/fuzz | grep __sanitizer_cov
d42e90: e8 8b 23 c0 ff call 945220 <__sanitizer_cov_trace_const_cmp8>
d42ff2: e8 39 21 c0 ff call 945130 <__sanitizer_cov_trace_pc_indi
...
💬 hebasto commented on pull request "build: pass sanitize flags to instrument libsecp256k1 code":
(https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1646846285)
FWIW, CMake [works](https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003) out of the box :)
(https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1646846285)
FWIW, CMake [works](https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003) out of the box :)
📝 hebasto opened a pull request: "Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree"
(https://github.com/bitcoin/bitcoin/pull/28131)
Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.
(https://github.com/bitcoin/bitcoin/pull/28131)
Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.
💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646873055)
@russeree thank you for joining the support effort. I ran Bitcoin Core with the option that you and @MarcoFalke recommended. Here is the command and command line error message produced.
C:\Users\xxxx>bitcoind -reindex-chainstate -debug
Error: A fatal internal error occurred, see debug.log for details
This system won't let me load the entire 365 MB debug.log file. So, I am displaying below the tail end of the file including the fatal error message.
2023-07-23T14:28:27Z [validation] Bloc
...
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646873055)
@russeree thank you for joining the support effort. I ran Bitcoin Core with the option that you and @MarcoFalke recommended. Here is the command and command line error message produced.
C:\Users\xxxx>bitcoind -reindex-chainstate -debug
Error: A fatal internal error occurred, see debug.log for details
This system won't let me load the entire 365 MB debug.log file. So, I am displaying below the tail end of the file including the fatal error message.
2023-07-23T14:28:27Z [validation] Bloc
...
📝 petertodd opened a pull request: "Enable full-rbf by default"
(https://github.com/bitcoin/bitcoin/pull/28132)
In addition to Luxor (3.3% hash power), the following pools are have enabled full-rbf:
- [Antpool, 21%](https://mempool.space/tx/53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643)
- [Binance Pool, 8.2%](https://mempool.space/tx/64a26f750c7c58812bd475d054a9aa05248b6a8a2d53c0db38c0624197a4c68a)
- [KuCoinPool, 1.4%](https://mempool.space/tx/8df183b087d433eac4d8e44eca5c7162a2540ee078a28ca04316fd82e12ec435)
- [Poolin, 1.4%](https://mempool.space/tx/d14956420077974d8d28c2fcd005cd9
...
(https://github.com/bitcoin/bitcoin/pull/28132)
In addition to Luxor (3.3% hash power), the following pools are have enabled full-rbf:
- [Antpool, 21%](https://mempool.space/tx/53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643)
- [Binance Pool, 8.2%](https://mempool.space/tx/64a26f750c7c58812bd475d054a9aa05248b6a8a2d53c0db38c0624197a4c68a)
- [KuCoinPool, 1.4%](https://mempool.space/tx/8df183b087d433eac4d8e44eca5c7162a2540ee078a28ca04316fd82e12ec435)
- [Poolin, 1.4%](https://mempool.space/tx/d14956420077974d8d28c2fcd005cd9
...
💬 petertodd commented on pull request "Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1646874124)
FYI [mempool.space has enabled full-rbf by default](https://github.com/mempool/mempool/pull/3867), among many others.
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1646874124)
FYI [mempool.space has enabled full-rbf by default](https://github.com/mempool/mempool/pull/3867), among many others.
💬 luke-jr commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646878130)
>except storage
Blockchains don't do storage. There's a tool for that called ext4, and BitTorrent for distribution.
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646878130)
>except storage
Blockchains don't do storage. There's a tool for that called ext4, and BitTorrent for distribution.
💬 jonatack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1271484187)
c75d9de0683a91151eb4a508cb64a8937ca92 `s/PAYMENT_H/PAYMENTS_H/` in `ifndef/define/endif` or rename the files to `silentpayment.{h,cpp}`
```bash
$ test/lint/lint-include-guards.py
src/wallet/silentpayments.h seems to be missing the expected include guard:
#ifndef BITCOIN_WALLET_SILENTPAYMENTS_H
#define BITCOIN_WALLET_SILENTPAYMENTS_H
...
#endif // BITCOIN_WALLET_SILENTPAYMENTS_H
```
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1271484187)
c75d9de0683a91151eb4a508cb64a8937ca92 `s/PAYMENT_H/PAYMENTS_H/` in `ifndef/define/endif` or rename the files to `silentpayment.{h,cpp}`
```bash
$ test/lint/lint-include-guards.py
src/wallet/silentpayments.h seems to be missing the expected include guard:
#ifndef BITCOIN_WALLET_SILENTPAYMENTS_H
#define BITCOIN_WALLET_SILENTPAYMENTS_H
...
#endif // BITCOIN_WALLET_SILENTPAYMENTS_H
```
💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646904832)
Concept ACK. Current limits on op_return force people to use hacks to store data hiding in op_codes. Using op_return is more `honest’. In my own case, current op_return limits keep me from posting a signed hash plus some metadata (<256 bytes) and thus I must use Taproot tricks instead.
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646904832)
Concept ACK. Current limits on op_return force people to use hacks to store data hiding in op_codes. Using op_return is more `honest’. In my own case, current op_return limits keep me from posting a signed hash plus some metadata (<256 bytes) and thus I must use Taproot tricks instead.
🤔 jonatack reviewed a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542308295)
Approach ACK b89567f51ade926af8c918e4787046b7ccec8eb0
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542308295)
Approach ACK b89567f51ade926af8c918e4787046b7ccec8eb0
💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271496876)
29e31d8
<details><summary><code>ParseSighash()</code> suggestions</summary><p>
```diff
--- a/src/core_io.h
+++ b/src/core_io.h
@@ -47,7 +47,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string&
-util::Result<int> ParseSighash(const std::optional<std::string>& sighash);
+[[nodiscard]] util::Result<int> ParseSighash(const std::optional<std::string>& sighash);
--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex,
...
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271496876)
29e31d8
<details><summary><code>ParseSighash()</code> suggestions</summary><p>
```diff
--- a/src/core_io.h
+++ b/src/core_io.h
@@ -47,7 +47,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string&
-util::Result<int> ParseSighash(const std::optional<std::string>& sighash);
+[[nodiscard]] util::Result<int> ParseSighash(const std::optional<std::string>& sighash);
--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex,
...
💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271533741)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 this ought to be added to `core_read.cpp` too; see full iwyu suggestions at https://cirrus-ci.com/task/4627440203464704?logs=ci#L1786
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271533741)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 this ought to be added to `core_read.cpp` too; see full iwyu suggestions at https://cirrus-ci.com/task/4627440203464704?logs=ci#L1786
💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271543977)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 When storing a `Result` type, I think it's clearer to name it `result` (seems to be becoming a convention). In particular, `util::ErrorString(parsed_sighash)` is a little confusing.
```cpp
const auto result{ParseSighash(sighash.isNull() ? std::nullopt : std::make_optional(sighash.get_str()))};
if (!result) {
throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original);
}
...
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271543977)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 When storing a `Result` type, I think it's clearer to name it `result` (seems to be becoming a convention). In particular, `util::ErrorString(parsed_sighash)` is a little confusing.
```cpp
const auto result{ParseSighash(sighash.isNull() ? std::nullopt : std::make_optional(sighash.get_str()))};
if (!result) {
throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original);
}
...