< bitcoin-git> [bitcoin] fanquake pushed 3 commits to 0.21: https://github.com/bitcoin/bitcoin/compare/0b9a046c1ae0...e358b43f7d89
< bitcoin-git> bitcoin/0.21 7de019b Andrew Chow: Introduce DeferringSignatureChecker and inherit with SignatureExtractor
< bitcoin-git> bitcoin/0.21 f79189c Andrew Chow: Test that signrawtx works when a signed CSV and CLTV inputs are present
< bitcoin-git> bitcoin/0.21 e358b43 fanquake: Merge #21640: [0.21] Introduce DeferredSignatureChecker and have Signature...
< bitcoin-git> [bitcoin] fanquake merged pull request #21640: [0.21] Introduce DeferredSignatureChecker and have SignatureExtractorClass subclass it (0.21...0.21-sig-ext) https://github.com/bitcoin/bitcoin/pull/21640
< aj> "Canceled because of an ongoing incident!" well, that's a novel test result
< luke-jr> ?
< jarthur> Am I a dummy? I configure with --enable-shared and still end up bitcoind being made dynamically linked
< sipa> shared means dynamically linked
< sipa> no?
< jarthur> Yes, sorry, sipa. I meant --enable-static :)
< jarthur> same problem
< sipa> is everything dynamically linked, or just system libraries?
< sipa> because iirc those can't be statically linked
< jarthur> According to ldd, I see libpthread, libboost*, libevent, libc all dynamically linked
< jarthur> Is there a bug open for the issue?
< sipa> tbh, i have no idea what --enable-shared and --enable-static are supposed to do
< sipa> i just vaguely recall it was a bit more complicated than that to get the static binaries we build for releases
< jarthur> Maybe I don't either. jgarzik hinted it ought to do what I'm trying to do in this comment https://github.com/bitcoin/bitcoin/issues/3781#issuecomment-36507746
< luke-jr> afaik --enable-static/shared is all about what libraries we *produce*
< sipa> jarthur: i think you can safely ignore anything said about the build system over 7 years ago :)
< luke-jr> XD
< luke-jr> jarthur: do you actually have static libraries to use installed?
< jarthur> I believe so. I see a libca and using the Debian -dev packages for the deps.
< jarthur> *libc.a
< jarthur> I see other projects have added a custom --enable-static-exec or --fully-static ./configure args, so maybe --enable-static means you only get static libbitcoin and libtest. Also tried adding --disable-shared without luck. I'll see if I can whip something up from older gitian when it built a static bitcoin core distributable.
< jarthur> My use case is tinier and more secure bitcoind Linux container images.
< sipa> can't you use the release binaries?
< sipa> they're statically linked
< jarthur> Including glibc?
< luke-jr> glibc doesn't support static linking…?
< sipa> i don't think so; i'm not sure that you can
< luke-jr> jarthur: static linking isn't more secure (nor tinier if you have more than one binary)
< luke-jr> the only real use case for it, is portability, and apparently that's defeated by us using Boost anyway :/
< sipa> we're pretty close to only having compile-tike boost dependencies
< jarthur> The idea is to remove bash and other distribution toolchain from the container's execution environment so that say a buffer overflow exploit has less toolchain available for further exploitation.
< luke-jr> sipa: that doesn't help
< luke-jr> sipa: boost compiles to code that only promises to work on the system that compiled it
< luke-jr> jarthur: sure, but that's not related to static linking
< luke-jr> sipa: and not even then sometimes ☺ (when I encountered this, the bitcoind was built on the same machine it was broken on!)
< sipa> luke-jr: the statx call thing?
< jarthur> luke-jr it is if it's my path to that execution environment, e.g. docker's scratch layer giving you pretty much just kernel->bitcoind in the execution environment. I'm definitely open to other ideas you have for getting there.
< luke-jr> yeah
< sipa> jarthur: you should just use release binaries
< luke-jr> (^ and build those yourself)
< sipa> luke-jr: that seems to be a pretty isolated case, imo
< sipa> (and specific to the filesystem library, which we'll droo)
< luke-jr> sipa: apparently other people with the issue are encountering it trying to run things in Docker
< luke-jr> because older Docker doesn't allow statx calls
< sipa> yeah, it's pretty annoying
< luke-jr> or you meant the issue itself was isolate
< sipa> but also, at leasst this specific case (statx) we'll get rid of by just dropping boost filesystem
< luke-jr> the scary thing is, I'm not sure we'd necessarily notice issues like this
< luke-jr> this time it happened to result in an exception that aborted early..
< sipa> well we'd presumably hear about stuff that doesn't work
< luke-jr> but who knows what other system calls missing might result in
< luke-jr> I'm glad we ban boost in consensus code :P
< sipa> are you sure stdc++ doesn't make similar assumptions? :)
< luke-jr> pretty sure
< sipa> yeah, i assume so.too
< sipa> but i don't actually know
< luke-jr> glibc at least actually insists you have a newer linux-headers than the kernel, in some cases
< luke-jr> as it doesn't have compile-time fallbacks, only runtime)(
< sipa> good
< luke-jr> the combination means that 2.6 system would have to switch to linux-headers-latest for glibc builds, and linux-headers-2.6.32 for boost builds >_<
< luke-jr> of course, 2.6 being as old as it is, my end conclusion was he's just gonna have to upgrade it :P
< luke-jr> one way or another
< sipa> statx was introduced in linux 4.11, which is from 2017
< sipa> that's not very old
< luke-jr> hmm
< luke-jr> I think glibc allows linux-headers back to 3.2
< sipa> that's from 2012
< luke-jr> kinda sad there isn't really a comparable solution in 2021 (for Linux containers)
< jarthur> Just tried the release binary, is dynamically linked to libc unfortunately. It looks like it stopped being fully static after this: https://github.com/bitcoin/bitcoin/pull/4042
< sipa> jarthur: glibc doesn't support linking statically
< sipa> (you can make it work apparently, but it's very discouraged)
< sipa> why do you insist on statically linked libc?
< sipa> the release binaries are intended to work on fairly ancient libc versions, fwiw; way older than the ones of the system it's built on
< luke-jr> (not that you'd want to use an obsolete glibc)
< jarthur> Minimal docker image without distro toolchain, or any part of the FHS file system really. It's not the end of the world if I have to include a copy of libc in the image, just would rather not. It's a common practice for small programs. I'm trying to apply it to bitcoind. The fact that fully static bitcoind binaries used to be distributed gave me hope it might be simple
< sipa> oh i see
< luke-jr> afaik we've always had to dynamic link to glibc?
< sipa> there was some disgussion of playing around with uclibc or some other more minimalistic libc, which would allow that
< sipa> right now, i wouldn't recommend straying from dynamic glibc
< sipa> luke-jr: the commit jarthur linked to earlier apparently reverted an earlier commit that made release binaries fully static
< sipa> but i don't know if that was ever used in a release
< jarthur> I believe 0.9.1 and possibly 0.9.2 distributables had binaries with statically linked glibc. Can't check now though as bitcoin.org took them down for security reasons.
< jarthur> ty :)
< jarthur> Looks like it was only Bitcoin Core 0.9.1 $file bitcoin-0.9.1-linux/bin/64/bitcoind.static
< jarthur> bitcoin-0.9.1-linux/bin/64/bitcoind.static: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, for GNU/Linux 2.6.24, BuildID[sha1]=8b02de16c470e9de817d6c551325517d2ad16a05, stripped
< jarthur> Is the biggest risk to issues stemming from kernel compatibility (and licensing if you distribute the binary outside of LGPL)? IMO you're already taking the kernel compatibility risk with shared glibc in your container image.
< aj> if you statically link glibc then use it against a different version of the nss libraries (i think?) it fails; kernel licensing isn't an issue afaik
< luke-jr> yeah, Linux's syscall interface is widely recognised as not considered a derivative work thus immune to GPL
< jarthur> aj I think the issues are with glibc licensing and not kernel licensing since you will technically be distributing glibc if you distribute your binary.
< luke-jr> true
< luke-jr> afaik it doesn't add any restrictions over Qt tho
< instagibbs> someone ban chew917 here https://github.com/bitcoin/bitcoin/pull/21464
< instagibbs> thanks
< bitcoin-git> [bitcoin] hebasto closed pull request #21591: build, qt: Fix wrong cross-compiling detection on macOS (master...210403-cross) https://github.com/bitcoin/bitcoin/pull/21591
< bitcoin-git> [bitcoin] hebasto reopened pull request #21591: build, qt: Fix wrong cross-compiling detection on macOS (master...210403-cross) https://github.com/bitcoin/bitcoin/pull/21591
< bitcoin-git> [bitcoin] fanquake closed pull request #19242: Add -uaappend option to append a literal string to user agent (master...uaappend) https://github.com/bitcoin/bitcoin/pull/19242
< wumpus> instagibbs: done
< bitcoin-git> [bitcoin] AlbanAndrieu closed pull request #21646: -daemonwait change to -daemon (master...master) https://github.com/bitcoin/bitcoin/pull/21646
< bitcoin-git> [bitcoin] vasild opened pull request #21659: net: flag relevant Sock methods with [[nodiscard]] (master...Sock_nodiscard) https://github.com/bitcoin/bitcoin/pull/21659
< bitcoin-git> [bitcoin] Emzy opened pull request #21661: doc: Fix name of script guix-build (master...patch-2) https://github.com/bitcoin/bitcoin/pull/21661
< bitcoin-git> [bitcoin] fdoving opened pull request #21662: build,boost: update download url. (master...fdov-boost-url) https://github.com/bitcoin/bitcoin/pull/21662
< bitcoin-git> [bitcoin] hebasto opened pull request #21663: ci: Fix macOS brew install command (master...210412-brew) https://github.com/bitcoin/bitcoin/pull/21663
< wumpus> we really need to fix the CI it's failling on almost everything
< wumpus> oh something with macos, looks like there is a PR for it #21663
< gribble> https://github.com/bitcoin/bitcoin/issues/21663 | ci: Fix macOS brew install command by hebasto · Pull Request #21663 · bitcoin/bitcoin · GitHub
< jakesyl> Anyone have any idea why a transaction version would be negative? https://github.com/btcsuite/btcd/pull/1701#issuecomment-817995493
< sipa> jakesyl: it's a number, it can be anything
< sipa> probably just someone playing around
< luke-jr> jakesyl: see BIP 68
< hebasto> wumpus: yeah, 21663 fixed brew, but ci log shows ridiculous configure failure
< luke-jr> ie, it's not really signed
< * luke-jr> wonders if there's anything that *does* treat it as signed
< sipa> luke-jr: BIP68 treats it as signed, afaik
< luke-jr> sipa: it explicitly doesn't…?
< hebasto> wumpus: probably, it is related to new Autoconf 2.71
< sipa> luke-jr: you are right
< sipa> luke-jr: standardness rules may treat it as signed
< luke-jr> src/policy/policy.cpp: if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION || tx.nVersion < 1) {
< luke-jr> I'm not sure the sign matters for that
< sipa> it does
< luke-jr> how?
< luke-jr> O.o even RPC treats it as unsigned: src/core_write.cpp: entry.pushKV("version", static_cast<int64_t>(static_cast<uint32_t>(tx.nVersion)));
< jakesyl> it looks like bip 68 casts nVersion to a uint32 (fEnforceBIP68)
< sipa> luke-jr: nVersion is an int32_t
< sipa> so if the serialization is 0xffffffff, it'd be interpreted as -1, and which be "< 1"
< luke-jr> sipa: yes, but if it were interpreted as 0xffffffff it would be >CTransaction::MAX_STANDARD_VERSION
< sipa> luke-jr: ah, good point!
< sipa> indeed, it doesn't matter
< sipa> at least with the current CTransaction::MAX_STANDARD_VERSION value
< luke-jr> quickly glancing over the code, I think we treat it as unsigned everywhere XD
< sipa> because the only two qualifying values are 1 and 2, and their serialization in signed and unsigned is identical
< jakesyl> only exception I can think of *might* be the rpc interface
< luke-jr> jakesyl: see above
< sipa> i vaguely recall that the RPC interface was changed a few years ago, precisely because it's semantically unsigned everywhere already
< sipa> but somehow didn't want to touch the actual data type for fear of consensus implications
< jakesyl> oh read that wrong. Got it - thanks!
< luke-jr> yeah, I'd want to do a lot more in-depth review before messing with it for that reason
< jakesyl> yeah - on btcd think we'll just match the bitcoin-core rpc interface there
< luke-jr> and frankly probably not worth it
< sipa> but at least at first glance i agree that it most likely doesn't affect anything
< jakesyl> (it's currently signed in our return)
< luke-jr> but as far as documentation, IMO we should just say it's unsigned
< sipa> yes, makes sense
< jakesyl> oh interesting - btw likely reason this is the first time we're seeing it (and it's on testnet) is b/c `IsStandardTx`isn't called w/ `-testnet`
< sipa> yeah
< sipa> you're unlikely to see that on mainnet
< jamesob> Anyone seen these 403 forbidden errors during CI runs before? https://github.com/bitcoin/bitcoin/pull/21526/checks?check_run_id=2327358485
< luke-jr> only recently; but I didn't find a solution
< wumpus> jamesob: yes, see #21663
< gribble> https://github.com/bitcoin/bitcoin/issues/21663 | ci: Fix macOS brew install command by hebasto · Pull Request #21663 · bitcoin/bitcoin · GitHub
< jamesob> wumpus: thanks
< wumpus> we should merge that soon but first waiting for CI to be green
< luke-jr> oh, that's the same one?
< wumpus> i think so
< jamesob> wumpus: I ACK'd only on the basis of the macOS CI run being green
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/f6c44e999b7d...708517b2dd64
< bitcoin-git> bitcoin/master b738155 Hennadii Stepanov: ci: Fix macOS brew install command
< bitcoin-git> bitcoin/master 708517b W. J. van der Laan: Merge #21663: ci: Fix macOS brew install command
< bitcoin-git> [bitcoin] laanwj merged pull request #21663: ci: Fix macOS brew install command (master...210412-brew) https://github.com/bitcoin/bitcoin/pull/21663