> 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 > (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 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 > > 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 > (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. >