👍 hebasto approved a pull request: "[26.0] Finalize or rc4"
(https://github.com/bitcoin/bitcoin/pull/28959#pullrequestreview-1760853383)
ACK b1d350c78b0a26e3c514a79b928578727df70538, I have reviewed the code and it looks OK.
(https://github.com/bitcoin/bitcoin/pull/28959#pullrequestreview-1760853383)
ACK b1d350c78b0a26e3c514a79b928578727df70538, I have reviewed the code and it looks OK.
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1412789203)
"resolving" this thread because I removed the fuzz tests from this PR
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1412789203)
"resolving" this thread because I removed the fuzz tests from this PR
💬 darosior commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1412791517)
Why?
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1412791517)
Why?
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1412793508)
Just not done testing yet and SigNet is easier.
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1412793508)
Just not done testing yet and SigNet is easier.
🤔 furszy reviewed a pull request: "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs"
(https://github.com/bitcoin/bitcoin/pull/28868#pullrequestreview-1760869833)
Code review ACK 0cf1ae5a
(https://github.com/bitcoin/bitcoin/pull/28868#pullrequestreview-1760869833)
Code review ACK 0cf1ae5a
💬 furszy commented on pull request "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs":
(https://github.com/bitcoin/bitcoin/pull/28868#discussion_r1412796530)
tiny nit:
Could save the "is mine" result at the top and use it in both if/else paths.
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp (revision ee14243fa526cfaa0ba538286588bb27e13d8c3c)
+++ b/src/wallet/wallet.cpp (revision e13fcf39a74a45696d1536017b13e5dd393a8ae3)
@@ -3922,31 +3922,32 @@
}
}
for (const auto& [_pos, wtx] : wtxOrdered) {
- if (!IsMine(*wtx->tx) && !IsFromMe(*wtx->tx)) {
- // Check it i
...
(https://github.com/bitcoin/bitcoin/pull/28868#discussion_r1412796530)
tiny nit:
Could save the "is mine" result at the top and use it in both if/else paths.
```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp (revision ee14243fa526cfaa0ba538286588bb27e13d8c3c)
+++ b/src/wallet/wallet.cpp (revision e13fcf39a74a45696d1536017b13e5dd393a8ae3)
@@ -3922,31 +3922,32 @@
}
}
for (const auto& [_pos, wtx] : wtxOrdered) {
- if (!IsMine(*wtx->tx) && !IsFromMe(*wtx->tx)) {
- // Check it i
...
💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1412797598)
done
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1412797598)
done
📝 furszy opened a pull request: "wallet: simplify and batch zap wallet txes process"
(https://github.com/bitcoin/bitcoin/pull/28987)
Work decoupled from #28574. Brother of #28894.
Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process.
1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`.
2) Instead of performing single write operations per removed tx recor
...
(https://github.com/bitcoin/bitcoin/pull/28987)
Work decoupled from #28574. Brother of #28894.
Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process.
1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`.
2) Instead of performing single write operations per removed tx recor
...
💬 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
}
//
...