💬 maflcko commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160756)
The log you posted says "Hardware Error", which lead me to believe that this is a hardware error and not a software error?
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160756)
The log you posted says "Hardware Error", which lead me to believe that this is a hardware error and not a software error?
💬 maflcko commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160823)
Bitcoin Core makes heavy use of CPU, RAM and disk IO. Hardware defects might only become visible when running Bitcoin Core. You might want to check your hardware for defects.
* memtest86 to check your RAM
* to check the CPU behaviour under load, use linpack or Prime95
* to test your storage device use smartctl or CrystalDiskInfo
Source: https://bitcoin.stackexchange.com/a/12206
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160823)
Bitcoin Core makes heavy use of CPU, RAM and disk IO. Hardware defects might only become visible when running Bitcoin Core. You might want to check your hardware for defects.
* memtest86 to check your RAM
* to check the CPU behaviour under load, use linpack or Prime95
* to test your storage device use smartctl or CrystalDiskInfo
Source: https://bitcoin.stackexchange.com/a/12206
💬 maflcko commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160918)
Otherwise, if you believe this is a software error, can you try in valgrind or another debugger, to get more information about the cause?
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1837160918)
Otherwise, if you believe this is a software error, can you try in valgrind or another debugger, to get more information about the cause?
💬 hebasto commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921)
I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
@theStack
Since you initially chose to use a vector, do you have any objections to this approach?
---
Here are the Windows functional tests -- https://github.com/hebasto/bitcoin/actions/runs/7072075125/job/19250502576
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921)
I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
@theStack
Since you initially chose to use a vector, do you have any objections to this approach?
---
Here are the Windows functional tests -- https://github.com/hebasto/bitcoin/actions/runs/7072075125/job/19250502576
💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1412875672)
You are right, I pushed a fix.
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1412875672)
You are right, I pushed a fix.
👋 hebasto's pull request is ready for review: "POC: Replace Boost.Process with cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/28981)
(https://github.com/bitcoin/bitcoin/pull/28981)
💬 hebasto commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837257182)
CI turned green. Undrafted.
cc @achow101 @Sjors
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837257182)
CI turned green. Undrafted.
cc @achow101 @Sjors
⚠️ Antwan52 opened an issue: "?"
(https://github.com/bitcoin/bitcoin/issues/28988)
0xb0e8EF343c70170FBd50f42917FE9Bd21300a5ff
(https://github.com/bitcoin/bitcoin/issues/28988)
0xb0e8EF343c70170FBd50f42917FE9Bd21300a5ff
✅ fanquake closed an issue: "?"
(https://github.com/bitcoin/bitcoin/issues/28988)
(https://github.com/bitcoin/bitcoin/issues/28988)
:lock: fanquake locked an issue: "?"
(https://github.com/bitcoin/bitcoin/issues/28988)
(https://github.com/bitcoin/bitcoin/issues/28988)
💬 brunoerg commented on pull request "test: add coverage for bech32m in `wallet_keypool_topup`":
(https://github.com/bitcoin/bitcoin/pull/28928#issuecomment-1837259374)
Good idea, @furszy.
master: 1.78s user 0.57s system 28% cpu 8.157 total
this PR: 0.76s user 0.20s system 35% cpu 2.668 total
(https://github.com/bitcoin/bitcoin/pull/28928#issuecomment-1837259374)
Good idea, @furszy.
master: 1.78s user 0.57s system 28% cpu 8.157 total
this PR: 0.76s user 0.20s system 35% cpu 2.668 total
📝 hebasto opened a pull request: "test: Fix test by checking the actual exception instance"
(https://github.com/bitcoin/bitcoin/pull/28989)
The `system_tests/run_command` test is broken because it passes even with the diff as follows:
```diff
--- a/src/test/system_tests.cpp
+++ b/src/test/system_tests.cpp
@@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(run_command)
});
}
{
- BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON
+ BOOST_REQUIRE_THROW(RunCommandParseJSON("invalid_command \"{\""), std::runtime_error); // Unable to parse JSON
}
//
...
(https://github.com/bitcoin/bitcoin/pull/28989)
The `system_tests/run_command` test is broken because it passes even with the diff as follows:
```diff
--- a/src/test/system_tests.cpp
+++ b/src/test/system_tests.cpp
@@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(run_command)
});
}
{
- BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON
+ BOOST_REQUIRE_THROW(RunCommandParseJSON("invalid_command \"{\""), std::runtime_error); // Unable to parse JSON
}
//
...
👋 TheCharlatan's pull request is ready for review: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960)
(https://github.com/bitcoin/bitcoin/pull/28960)
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1837267077)
Undrafting this now. I tested this approach extensively against my proof of concept [C FFI](https://github.com/TheCharlatan/bitcoin/pull/7) and proof of concept [Rust library](https://github.com/TheCharlatan/rust-bitcoinkernel), and think it's not that bad an interface to have. Not sure why the msvc build is failing (only seems to be the Qt build?).
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1837267077)
Undrafting this now. I tested this approach extensively against my proof of concept [C FFI](https://github.com/TheCharlatan/bitcoin/pull/7) and proof of concept [Rust library](https://github.com/TheCharlatan/rust-bitcoinkernel), and think it's not that bad an interface to have. Not sure why the msvc build is failing (only seems to be the Qt build?).
💬 hebasto commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1837269861)
> Not sure why the msvc build is failing (only seems to be the Qt build?).
cc @sipsorcery
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1837269861)
> Not sure why the msvc build is failing (only seems to be the Qt build?).
cc @sipsorcery
💬 aureleoules commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1837290207)
The benchmark `WalletCreateTxUsePresetInputsAndCoinSelection` seems to segfault occasionally on this pull (increasing min-time makes it crash consistently)
```bash
$ valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection -min-time=10000
==131665== Memcheck, a memory error detector
==131665== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==131665== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==131665== Command: ./src
...
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1837290207)
The benchmark `WalletCreateTxUsePresetInputsAndCoinSelection` seems to segfault occasionally on this pull (increasing min-time makes it crash consistently)
```bash
$ valgrind ./src/bench/bench_bitcoin -filter=WalletCreateTxUsePresetInputsAndCoinSelection -min-time=10000
==131665== Memcheck, a memory error detector
==131665== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==131665== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==131665== Command: ./src
...
👍 pablomartin4btc approved a pull request: "rpc: encryptwallet help, mention HD seed rotation and backup requirement"
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1760994448)
re ACK 30983e4e30a136ce7d2efc3639f0c17ed89ea38b
(https://github.com/bitcoin/bitcoin/pull/28980#pullrequestreview-1760994448)
re ACK 30983e4e30a136ce7d2efc3639f0c17ed89ea38b
💬 theStack commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837299151)
> I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
>
> @theStack Since you initially chose to use a vector, do you have any objections to this approach?
No objections at all. I would have expected that passing the arguments as a single string is _more_ prone to quoting issues than if it's done in a list, where quoting shouldn't be needed at all, but if it works, all the better :) (Maybe that
...
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837299151)
> I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
>
> @theStack Since you initially chose to use a vector, do you have any objections to this approach?
No objections at all. I would have expected that passing the arguments as a single string is _more_ prone to quoting issues than if it's done in a list, where quoting shouldn't be needed at all, but if it works, all the better :) (Maybe that
...
💬 Sapibeltv commented on issue "Libbitcoinkernel Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1837312263)
We are sapibel families and Seexy Nadege Entertainment 2vtv and SAPIBELTVradio is the most important tools to use the internet marketing industry provider https://github.com/bitcoin/bitcoin/assets/107956561/38841f2f-3194-4f66-aa76-9eeb96988216
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1837312263)
We are sapibel families and Seexy Nadege Entertainment 2vtv and SAPIBELTVradio is the most important tools to use the internet marketing industry provider https://github.com/bitcoin/bitcoin/assets/107956561/38841f2f-3194-4f66-aa76-9eeb96988216
💬 hebasto commented on pull request "POC: Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837430222)
> > I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
> > @theStack Since you initially chose to use a vector, do you have any objections to this approach?
>
> No objections at all. I would have expected that passing the arguments as a single string is _more_ prone to quoting issues than if it's done in a list, where quoting shouldn't be needed at all, but if it works, all the better :) (Maybe t
...
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837430222)
> > I switched to the `Popen` constructor that accepts `const std::string&` instead of `std::vector<std::string>` to address quoting issues on Windows.
> > @theStack Since you initially chose to use a vector, do you have any objections to this approach?
>
> No objections at all. I would have expected that passing the arguments as a single string is _more_ prone to quoting issues than if it's done in a list, where quoting shouldn't be needed at all, but if it works, all the better :) (Maybe t
...
💬 S3RK commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1837467046)
Concept ACK.
The PR description should say that we don't use hardcoded fee rate value, but rather multiple of long-term-fee-rate. LTFR is an existing user configurable parameter. It's also a good fit for the purpose of determining when to be more "aggressive" with coin selection, because it captures forward looking fee market expectations.
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1837467046)
Concept ACK.
The PR description should say that we don't use hardcoded fee rate value, but rather multiple of long-term-fee-rate. LTFR is an existing user configurable parameter. It's also a good fit for the purpose of determining when to be more "aggressive" with coin selection, because it captures forward looking fee market expectations.