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

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, we are currently using the upstream LevelDB commit 0c40829872a9f00f38e11dc370ff8adb3e19f25b

Cheers,
Carl Dong


On Jan 5, 2022, at 2:13 PM, Maxime Devos <maximedevos@telenet.be> 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.