Subject: bug#34170: bitcoin-core bundles leveldb

From: Carl Dong <[email-masked]>
Date: Thu, 6 Jan 2022 19:17:13 -0500

> If desired, it would also be possible to do something in-between
> unbundling and using bitcoin's leveldb: define a 'leveldb/bitcoin'
> variant of the 'leveldb' package (using package/inherit or (package
> (inherit ...) ...)), add it as input to the 'bitcoin' package and tell
> and/or patch bitcoin's buid scripts to use that leveldb.
Yes I think that would be a splendid idea! With regards to patching bitcoin’s builds scripts, Bitcoin Knots follows Bitcoin Core closely, but has a bunch of patches which allow for using system libs, so that might be good to reference: https://github.com/bitcoin/bitcoin/compare/master...bitcoinknots:21.x-syslibs <https://github.com/bitcoin/bitcoin/compare/master...bitcoinknots:21.x-syslibs>

> As source code, use an appropriate commit from
> <https://github.com/bitcoin-core/leveldb-subtree> (and add a comment
> to the definition of bitcoin-core to keep leveldb/bitcoin in-sync).

FYI, according to https://github.com/bitcoin/bitcoin/pull/17398 <https://github.com/bitcoin/bitcoin/pull/17398>, we are currently using the upstream LevelDB commit 0c40829872a9f00f38e11dc370ff8adb3e19f25b

Cheers,
Carl Dong

> On Jan 5, 2022, at 2:13 PM, Maxime Devos <[email-masked]> wrote:
> 
> Hi,
> 
> Carl Dong schreef op wo 05-01-2022 om 12:40 [-0500]:
>> Simon, Maxime, Danny,
>> 
>> Thanks for CCing me on this message! The rationale for bundling
>> leveldb in Bitcoin Core goes a bit beyond convenience, it is several
>> things:
>> 
>> 1. The original reason for sub-treeing is that Bitcoin Core used to
>> maintain its own version of leveldb with its own fixes
>> here: https://github.com/bitcoin-core/leveldb-subtree, since then
>> most of these fixes have been upstreamed as
>> of: https://github.com/bitcoin/bitcoin/pull/17398
> 
> Seems reasonable to me, but the bitcoin project is upstreaming the
> changes it made and most are already upstream, so I would prefer
> to use upstream's leveldb.
> 
>> 2. We also used to support using an external leveldb, however, it
>> seems that it was fragile to rely on external projects to maintain
>> ABI compatibility, see the quoted IRC bug report
>> here: https://github.com/bitcoin/bitcoin/pull/23282. Reasonable minds
>> may disagree on this point, especially coming from Guix where
>> patching is convenient.
> 
> The quoted ABI incompatibility
> 
>    <Talkless> bitcoind fails to start with undefined symbol:
> _ZTIN7leveldb6LoggerE in Debian Sid after leveldb upgraded from 1.22 to
> 1.23: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996486
> 
> doesn't apply in Guix, because guix uses RUNPATH and multiple library
> versions can exist in the same store (in different directories in the
> store).
> 
>> In addition to the above, Bitcoin Core experienced a hard fork in
>> 2013 due to database incompatibilities, which has predisposed
>> maintainers towards a more stringent approach with pinning
>> dependencies and their configure/build-time flags.
>> See: https://blog.bitmex.com/bitcoins-consensus-forks/#was-the-2013-
>> incident-a-hardfork
> 
> I doubt that Guix has sufficient Bitcoin Core users to cause
> a hard fork, but yes, this is an understandable reason to bundle
> things.  But any such problem seems easy to resolve (at the guix side):
> we could simply temporarily switch to an older version of leveldb.
> 
> If desired, it would also be possible to do something in-between
> unbundling and using bitcoin's leveldb: define a 'leveldb/bitcoin'
> variant of the 'leveldb' package (using package/inherit or (package
> (inherit ...) ...)), add it as input to the 'bitcoin' package and tell
> and/or patch bitcoin's buid scripts to use that leveldb.
> 
> As source code, use an appropriate commit from
> <https://github.com/bitcoin-core/leveldb-subtree> (and add a comment
> to the definition of bitcoin-core to keep leveldb/bitcoin in-sync).
> 
> A benefit of this approach (if done properly, with (origin (inherit
> ...) ...) such that patches of 'leveldb' are inherited) above the
> status quo, is that is that if for some reason 'leveldb' is patched in
> Guix, then 'leveldb/bitcoin' receives the patch as well. Another
> benefit is that the dependency 'googletest' and 'benchmark' of leveldb
> would remain unbundled.
> 
> Greetings,
> Maxime.
>