Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ kosuodhmwa opened an issue: "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk"
(https://github.com/bitcoin/bitcoin/issues/30191)
I ask me why? Now i added the txindex=1 option to bitcoin.conf file and it's the OS disk that gets smaller and smaller - NOT the disk where .bicoin data directory is located... that's very, very confusing i.m.o

B: = bitcoin data disk where .bitcoin data directory is
I: = disk where the virtualbox bitcoind VM (Debian 12.x) is.

![image](https://github.com/bitcoin/bitcoin/assets/24782840/baf29916-9c8b-45ba-8759-91026a2839f8)
πŸ’¬ kosuodhmwa commented on issue "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk":
(https://github.com/bitcoin/bitcoin/issues/30191#issuecomment-2137526218)
maybe it saves tons of logs in /var/ log ??
πŸ’¬ maflcko commented on issue "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk":
(https://github.com/bitcoin/bitcoin/issues/30191#issuecomment-2137538541)
We can't help you by guessing what you did and what went wrong. You'll have to explain everything in detail.

Make sure to fully read https://github.com/bitcoin/bitcoin/blob/master/doc/files.md and understand it, first.
πŸ’¬ fanquake commented on pull request "build: remove usage of BOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137542167)
> Does --disable-suppress-external-warnings only work with depends?

It should not be depends specific.

> Should --disable-suppress-external-warnings be enabled in some CI tasks?

That depends on what we are trying to achieve, and if we are planning on patching external libraries / code and upstreaming relevant changes etc. It's also likely to lead to more CI "breakage" on distro version, or other changes, that may not be relevant for us.
πŸ’¬ maflcko commented on pull request "build: remove usage of BOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137551455)
> That depends on what we are trying to achieve

Wouldn't it be good to know about issues, such as the one here (`BOOST_NO_CXX98_FUNCTION_BASE`) as early as possible? Not sure how to achieve that, other than setting `--disable-suppress-external-warnings` in at least some CI tasks.
πŸ“ fanquake opened a pull request: "build: remove `--enable-lcov-branch-coverage`"
(https://github.com/bitcoin/bitcoin/pull/30192)
This supports lcov `2.x` in the sense that we are no-longer hardcoding version specific options, and instead will use the `LCOV_OPTS` variable (which is the more correct/flexible thing to do in any case). It's also quite likely that devs are already having to pass extra options to lcov `2.x`, given it's more stringent in terms of coverage generation and error checking. See this thread for an example: https://github.com/linux-test-project/lcov/issues/238.

Tested on one machine (LCOV 2.0, gcc 1
...
πŸ’¬ fjahr commented on pull request "Prevent file descriptor exhaustion from too many RPC calls":
(https://github.com/bitcoin/bitcoin/pull/27731#issuecomment-2137569667)
> > removing libevent
>
> Nice. Is there a tracking issue?

Not yet, but should be open soon (TM).
πŸ’¬ fanquake commented on pull request "build: remove usage of BOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137582493)
> Wouldn't it be good to know about issues, <snip> as early as possible?

I think if we are going to do that, then enabling it on a rolling / nightly distro type CI, i.e fedora rawhide, would be the most useful, otherwise, I think it's unlikely to actually turn much up, given packages are pinned in depends, and in the distro. Note that `*--suppress-external-warnings` was another feature not ported to CMake, as apparently it isn't needed; so someone would have to look at porting it again, so it
...
πŸ€” maflcko reviewed a pull request: "build: remove `--enable-lcov-branch-coverage`"
(https://github.com/bitcoin/bitcoin/pull/30192#pullrequestreview-2085631803)
utACK b08e07070085edf8cb5fda55d8849188fd83cfc6
πŸ’¬ maflcko commented on pull request "build: remove `--enable-lcov-branch-coverage`":
(https://github.com/bitcoin/bitcoin/pull/30192#discussion_r1619018413)
nit: filename
πŸ’¬ fanquake commented on pull request "build: remove `--enable-lcov-branch-coverage`":
(https://github.com/bitcoin/bitcoin/pull/30192#discussion_r1619022478)
Updated
πŸ’¬ maflcko commented on pull request "build: remove usage of BOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137599157)
> I think it's unlikely to actually turn much up, given packages are pinned in depends

at least for the sanitizer builds, the compiler is bumped regularly, so it seems good to check those bumps when they happen.
πŸ’¬ hebasto commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1619032750)
Using this approach instead of two distinct signatures -- `void push_back(const T&)` and `void push_back(T&&)` -- can lead to some confusion in error messages.

Consider this:
```c++
struct A { int m_a{0}; } a;
struct B { int m_b{0}; } b;

std::vector<A> v;
v.push_back(a);
// ERROR:
// no known conversion for argument 1 from β€˜B’ to β€˜const std::vector<A>::value_type&’ {aka β€˜const A&’}
// no known conversion for argument 1 from β€˜B’ to β€˜std::vector<A>::value_type&&’ {aka β€˜A&&’}
// v.pu
...
πŸ“ m3dwards opened a pull request: "ci: move ASAN job to GitHub Actions from Cirrus CI"
(https://github.com/bitcoin/bitcoin/pull/30193)
Draft PR for moving the ASAN + LSAN + USDT + friends job to github actions from Cirrus.
πŸ’¬ maflcko commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2137638765)
Concept ACK, may review tomorrow if GHA is back online by then.
πŸ“ theuni opened a pull request: "refactor: use recommended type hiding on multi_index types"
(https://github.com/bitcoin/bitcoin/pull/30194)
Recommended by boost docs:
https://www.boost.org/doc/libs/1_85_0/libs/multi_index/doc/compiler_specifics.html#type_hiding

This significantly reduces the size of the symbol name lengths that end up in the binaries as well as in compiler warnings/errors. Otherwise there should be no functional change.

Example before:

> 0000000000000000 W unsigned long boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_laye
...
πŸ’¬ hebasto commented on pull request "ci: move ASAN job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#issuecomment-2137644075)
Concept ACK.
πŸ’¬ theuni commented on pull request "refactor: use recommended type hiding on multi_index types":
(https://github.com/bitcoin/bitcoin/pull/30194#issuecomment-2137652409)
Note that I started by limiting `BOOST_MULTI_INDEX_LIMIT_INDEXED_BY_SIZE` and `BOOST_MULTI_INDEX_LIMIT_TAG_SIZE` as also recommended by the docs (which helped), but with the hiding applied with this commit those defines appear to be moot.
πŸ‘ fanquake approved a pull request: "build, test, doc: Temporarily remove Android-related stuff"
(https://github.com/bitcoin/bitcoin/pull/30049#pullrequestreview-2085710294)
ACK 5deb0b024e14c7c63d405c651d1ca323560a1c21 - given none of this is currently tested/wont compile. Can be revisted in future.
πŸ‘ hebasto approved a pull request: "build: remove `--enable-lcov-branch-coverage`"
(https://github.com/bitcoin/bitcoin/pull/30192#pullrequestreview-2085711219)
ACK 462e761672d387c98e729e920a4102feaedc53cb. A similar approach was used in https://github.com/hebasto/bitcoin/pull/191.