Bitcoin Core Github
42 subscribers
127K links
Download Telegram
💬 maflcko commented on pull request "rpc: add utxo's blockhash and number of confirmations to scantxoutset output":
(https://github.com/bitcoin/bitcoin/pull/30515#discussion_r1690827566)
nit: The two fields should have the same wording, as they refer to the same block
💬 maflcko commented on pull request "rpc: add utxo's blockhash and number of confirmations to scantxoutset output":
(https://github.com/bitcoin/bitcoin/pull/30515#discussion_r1690834408)
```suggestion
{RPCResult::Type::NUM, "confirmations", "Number of confirmations of the unspent transaction output when the scan was done"},
```

nit: (Same wording here, because the `confirmations` refers in the calculation to the same block)
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1690844876)
> ```diff
> + COutPoint out{*txid, static_cast<uint32_t>(nOut)};
> ```

Thanks for looking again. For reference, `getInt` has a built in check, so you could use `getInt<uint32_t>` and avoid this cast. That'd be a behavior change for out-of-range values, but the values are out of range anyway, so should be fine.
📝 fanquake locked a pull request: "Create Wap"
(https://github.com/bitcoin/bitcoin/pull/30521)
http://waptrick.com

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements o
...
paplorinc closed a pull request: "optimization: Precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher"
(https://github.com/bitcoin/bitcoin/pull/30442)
💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1690941661)
Hmm https://corecheck.dev/bitcoin/bitcoin/pulls/30454 is down again
👍 BrandonOdiwuor approved a pull request: "Rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin-core/gui/pull/828#pullrequestreview-2198547338)
ACK 8233ee41ab9648cd0c3bd78bc2a8d692a54d9ea0. Tested on Ubuntu 22.04 using Qt version 5.15.3
💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1690990701)
The rename could be split out and merged ahead?
💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1690979665)
note to myself: Could split this up
💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2249700975)
Ok, I spun up two machines to see if I can reproduce. I left zram and put everything on one SSD. Also, my config has some debug logging enabled. Also, I am using a recent guix build, instead of a source compile of 25.x.

Let me know when this happens again, so that I can check if it happened to me as well. I'll then try to debug this further.


```
sh-5.2$ nproc
2
sh-5.2$ uname --kernel-release --kernel-version
6.1.97-104.177.amzn2023.aarch64 #1 SMP Tue Jul 16 15:18:22 UTC 2024
sh-5.2
...
💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2249707433)
In the meantime it could make sense for you to consider upgrading to a more recent version of Bitcoin Core. According to https://bitcoincore.org/en/lifecycle/ and https://bitcoincore.org/en/security-advisories/ , 25.x will be EOL soon and "Medium and High severity bugs will be disclosed 2 weeks after the [last affected release goes EOL](https://bitcoincore.org/en/lifecycle/). This is a year after a fixed version was first released. A pre-announcement will be made 2 weeks prior to disclosure."

...
🚀 fanquake merged a pull request: "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job"
(https://github.com/bitcoin/bitcoin/pull/30519)
💬 willcl-ark commented on issue "Unexpected behaviour when using `sortedmulti_a` descriptor":
(https://github.com/bitcoin/bitcoin/issues/30518#issuecomment-2249872391)
Thanks for the report.

I would also expect to see these decodes behave the same as the only difference between them should be the sorting. I didn't investigate further yet, but incidentally I see the same behaviour in a toy rust-miniscript parser I have:

```
Loading descriptor: tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),multi_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035
...
💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2249873826)
RISCV build failed here:
```bash
Traceback (most recent call last):
File "/distsrc-base/distsrc-3733b4b088ff-riscv64-linux-gnu/./contrib/devtools/test-security-check.py", line 73, in test_ELF
self.assertEqual(call_security_check(cxx, source, executable, pass_flags + ['-no-pie','-fno-PIE']), (1, executable + ': failed PIE'))
AssertionError: Tuples differ: (1, 'test1: failed PIE FORTIFY') != (1, 'test1: failed PIE')

First differing element 1:
'test1: failed PIE FORTIFY'
'test1: fai
...
💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2249881098)
> Maybe I'm over-complicating it?

And if there were any issues that I am missing, commit fa72dcbfa56177ca878375bae7c7bca6ca6a1f40 would be wrong as well, no?
📝 maflcko opened a pull request: "ci: Add missing qttools5-dev install to Asan task"
(https://github.com/bitcoin/bitcoin/pull/30522)
This is required, according to the docs:

```
$ git grep --line-number 'qtbase5-dev qttools5-dev qttools5-dev-tools' doc
doc/build-unix.md:84: sudo apt-get install qtbase5-dev qttools5-dev qttools5-dev-tools
```

Also, needed for cmake.
💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1691143911)
Done in https://github.com/bitcoin/bitcoin/pull/30522
👍 hebasto approved a pull request: "contrib: fix check-deps.sh to check for weak symbols"
(https://github.com/bitcoin/bitcoin/pull/30415#pullrequestreview-2198811168)
re-ACK 114b2a406e604747bd856f566aa8c7ad84dd8f15.
📝 fanquake converted_to_draft a pull request: "utils: replace boost::date_time usage with c++20 std::chrono"
(https://github.com/bitcoin/bitcoin/pull/30462)
This would be a very straightforward and uncontroversial change if not for the sad state of std lib implementations which implement this functionality.

As of now, `std::chrono::parse` and friends (c++20 additions) are only available as of gcc 14 and (unreleased) clang 19.

Formatting in chrono is quite useful, so I don't think it makes sense to wait years until we require those compilers.

Instead of waiting around, this PR takes the sledgehammer approach of adding a fully-conformant impl
...
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691152128)
Ah I didn't notice this yesterday - I think you may have swapped odd and even? Same in the commented mermaid diagram code just added.