< wumpus>
I've always been sceptical toward thread_local use and wouldn't mind if it was removed, though I'm kind of surprised by the enormous performance impact
< wumpus>
we don't do thread name lookups for debug messages that aren't logged do we?
< bitcoin-git>
[bitcoin] rvagg opened pull request #18826: Expose txinwitness for coinbase in JSON form from RPC (master...rvagg/txinwitness-for-coinbase) https://github.com/bitcoin/bitcoin/pull/18826
< bitcoin-git>
bitcoin/master fac0cf6 MarcoFalke: rpc: Do not advertise dumptxoutset as a way to flush the chainstate
< bitcoin-git>
bitcoin/master 00c1a4d MarcoFalke: Merge #18809: rpc: Do not advertise dumptxoutset as a way to flush the cha...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18809: rpc: Do not advertise dumptxoutset as a way to flush the chainstate (master...2004-rpcNoFlushAdvert) https://github.com/bitcoin/bitcoin/pull/18809
< bitcoin-git>
bitcoin/master de8905a Gloria Zhao: test: use unittest and test_runner for test framework unit testing
< bitcoin-git>
bitcoin/master a66ba6d MarcoFalke: Merge #18576: test: use unittest for test_framework unit testing
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18576: test: use unittest for test_framework unit testing (master...framework-unittests) https://github.com/bitcoin/bitcoin/pull/18576
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #18828: test: Strip down previous releases boilerplate (master...2004-qaPreviousReleases) https://github.com/bitcoin/bitcoin/pull/18828
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #18829: ci: Merge C++17 build with one of the existing ones (master...2004-ciSpeedupCxx17) https://github.com/bitcoin/bitcoin/pull/18829
< bitcoin-git>
bitcoin/master aaaacff MarcoFalke: ci: Merge C++17 build with one of the existing ones
< bitcoin-git>
bitcoin/master e5b9308 MarcoFalke: Merge #18829: ci: Merge C++17 build with one of the existing ones
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #18829: ci: Merge C++17 build with one of the existing ones (master...2004-ciSpeedupCxx17) https://github.com/bitcoin/bitcoin/pull/18829
< sipa>
oy
< wumpus>
#startmeeting
< lightningbot>
Meeting started Thu Apr 30 19:00:20 2020 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< wumpus>
fwiw I'm fine with the current tarball which is just a dumb dump of the complete git repository
< luke-jr>
or -dirty added, I forget the details :/
< luke-jr>
wumpus: without a leading directory..
< MarcoFalke>
Looks like sipa has two pulls in high prio :eyes: NoT AllOweD!!!111
< sipa>
oh?
< wumpus>
MarcoFalke: oh no!!!
< sipa>
oh yes
< MarcoFalke>
nooooooo
< sipa>
pick one
< luke-jr>
wumpus: ie, if you extract it, you get the contents dumped into your current directory
< MarcoFalke>
luke-jr: The previous releases had the prefix? If so, it makes sense to add it back
< wumpus>
luke-jr: that's kind of an unexpected change from before
< sipa>
yeah, agree
< sipa>
no opinion on the pre/post configure aspect
< wumpus>
luke-jr: is it possible to add the prefix without going through the whole extract and repack dance again?
< luke-jr>
wumpus: yes, the first commit of my PR does that
< wumpus>
I was kind of happy to get rid of that
< wumpus>
okay!
< wumpus>
will take a look then
< jonasschnelli>
hi
< luke-jr>
I was also tempted to use tar --update to merely append to the git-archive tarball, but that seems dangerous considering timestamps
< wumpus>
okay everything should be tagged / added to high prio
< MarcoFalke>
luke-jr: Maybe split out the first commit into a pull, so that gitian builds can be performed on it more easily?
< hebasto>
Agree ^
< luke-jr>
MarcoFalke: well, like I said, IMO both should be fixed - so unless there's a strong objection to distcleaning, I'd rather focus on that for now?
< luke-jr>
I don't see the motivation in deviating so significantly from best practices in that regard..
< wumpus>
well for what it's worth there seems to be not disagreement at all on adding the prefix so that should be straightforward even to get into rc2
< luke-jr>
is there disagreement on the rest? or just people didn't feel like doing it before?
< jonatack>
I wonder if it makes sense to remove #16442 until it's rebased, the longer it stays in the blockers without action, the more reviewers might be tuning it out
< wumpus>
the other change requires a unpack and configure and re-pack, I don't really like that, I was happy to just use git to create the archive
< MarcoFalke>
jonatack: Yeah, generally we remove stuff that needs rebase
< wumpus>
jonatack: sgtm
< sipa>
if luke's PR is all it takes to get us back to post-configure (without other hacks) it seems reasonable to take it
< sipa>
my impression was that we got rid of the post-configure thing because it was hard to maintain
< sipa>
would that still be the case with this approach?
< MarcoFalke>
Yes, the issue was that files were missing from the tar.gz
< fjahr>
hi
< luke-jr>
That issue remains fixed with this approach
< wumpus>
it at the least makes the tarball build process more complex again, which potentially introduces bugs and inconsistencies again
< wumpus>
in any case if the PR gets enough ACKS it can go in
< hebasto>
split in two prs is also a way
< wumpus>
at the least I hope this fixes it and we're not going to have lots of back and forth on this
< wumpus>
which is the feeling I get if people talk about version hacks
< cfields>
I
< cfields>
I'm still having trouble understanding why this was all changed.
< wumpus>
because we wanted all files in the tarball
< wumpus>
and a safe way to do that is 'git archive', it doesn't accidentally include any junk files nor does it exclude anything
< luke-jr>
cfields: the previous problem was that `make dist` missed a lot of files
< cfields>
sure, understood.
< cfields>
but now we're talking about bootstrapping and appending anyway. but we're appending anyway. We could've just bootstrapped as before and appended the git archive... the reverse of this I suppose.
< wumpus>
doing the same with make dist would involve including everything in the makefile, either through wildcards or worse, through enumeration, something impossible to keep up to date
< cfields>
as long as it works, I suppose.
< wumpus>
yes, that' pretty much what luke-jr does in his PR
< ariard>
I agree it's not ideal to export standardness among libconsensus and maybe we should a clear distinct libstandard
< wumpus>
is that a topic?
< * MarcoFalke>
lets do libbitcoincore
< wumpus>
#topic Standardness in libconsensus
< ariard>
but people on higher layers definetely need to be sure that their ttxn are going to propage on the currently deployed network
< ariard>
wumpus: I should have submitted before sorry
< wumpus>
ariard: np
< luke-jr>
ariard: maybe stuff like c-lightning should just call RPC?
< ariard>
luke-jr: you want to do this in your test framework
< ariard>
and you may not have a full-node to test
< luke-jr>
test frameworks can run a bitcoind
< wumpus>
MarcoFalke: the problem is defining a sane interface
< wumpus>
has always been; the idea to extend libconensus has been there for very long
< ariard>
luke-jr: it's make integration harder
< MarcoFalke>
ariard: Doesn't lightning depend on Bitcoin Core or a different full node impl anyway?
< luke-jr>
ariard: well, I don't see a sane way to address user choice of policies..
< ariard>
MarcoFalke: actually no, you can be connected to an electrum server but still want to verify a single-sricptpubkey
< wumpus>
yes, c-lightning depends on bitcoin core (at run time, not at build time)
< luke-jr>
another sane way*
< wumpus>
oh did that change?
< ariard>
beyond c-lightning or any LN implementation
< cfields>
at the very least I think we'd want it broken out into a new function that's clearly non-consensus-trustworthy.
< ariard>
cfields: yes, let's not confuse what is standard and consensus
< luke-jr>
there's other utility RPCs that would make sense to export somewhere too
< sipa>
i see no problem exposing a Bitcoin Core library that implements the same policies as Bitcoin Core; it's just a more convenient interface to do something you can already do with running a node
< luke-jr>
libbitcoincore isn't the best name, but I can't think of a better one XD
< MarcoFalke>
lol, I was trolling
< wumpus>
I don't see a problem either, just make it a different function from consensus validation
< sipa>
it seems convenient to add that to bitcoinconsensus, but i agree it's confusing the name of the lib
< ariard>
okay we should rename bitcoinconsenus to something different?
< MarcoFalke>
ariard: is elecrum server not a full node?
< luke-jr>
I suppose we could put it in the same lib, but add a second .pc file in case we split it later..
< wumpus>
no, don't rename bitcoinconsensus
< luke-jr>
otoh, do callers of these functions need consensus code at all?
< ariard>
MarcoFalke: I meaned a electrum client serving as a verification backend to a LN node
< ariard>
like a mobile
< ariard>
you still want to verify scriptpubkey if you can even if you can't verify full set of consensus/standard ruels
< * luke-jr>
saves the rant against people not using their own full nodes for another time
< wumpus>
how do the other mobile LN wallets do that? they definitely don't all require a full node
< MarcoFalke>
Why would you not use the electrum server and instead hop through the electrum client?
< ariard>
luke-jr: my mobile connected to my full-node may know network outage and still need to update a channel state
< luke-jr>
doesn't Lightning use very specific templates anyway?
< luke-jr>
so long as the standard Lightning scripts are being used, they should all be fine, no?
< ariard>
wumpus: you may use BIP157 or any other light client protocols, even a custom one
< ariard>
luke-jr: we use specific templates but witness must enforce MINIMALIF among others
< sipa>
i think we're straying from the topic now
< wumpus>
in any case, I'm not against adding a standard check function, just call it differently, I don't think it matters it's in libconsensus *if* you document it and name it in a clear way
< ariard>
right, we do agree exporting a standard *opaque* flag? it's just a way to do it
< sipa>
ariard: if you use the standard template, they are minimalif automatically?
< wumpus>
no, I don't want to do it with a flag, just add a function
< wumpus>
don't touch the consensus function for non-consensus checks
< ariard>
sipa: don't follow there what you mean by standard template?
< sipa>
ariard: lightning uses specific scripts, no?
< sipa>
bolt 03 or something
< ariard>
okay so add a verify_standard function but get it in bitcoinconsensus library?
< wumpus>
yes
< luke-jr>
ariard: add a new .pc file too IMO
< wumpus>
doesn't seem worth it to add a new library for, but should be a separate function imo
< wumpus>
luke-jr: nah, there will be so much overlap between both
< ariard>
luke-jr: it may makes intgration harder, dunno who is using this right now apart of rust-bitocin ecosystem
< luke-jr>
wumpus: it's like 4 lines?
< wumpus>
either the two libraries have to dpeend on each other
< wumpus>
or share code
< luke-jr>
wumpus: just a new .pc, not a new lib
< sipa>
luke-jr: what is the difference?
< ariard>
I think important point is document well in shared-libraries.md that it's standardness
< luke-jr>
sipa: .pc is like 4 lines of text that says which linker/compiler flags to use
< luke-jr>
so for now it would just -lconsensus
< luke-jr>
if down the road it gets big enough to split up, we can change to -lsomethingelse
< ariard>
and you may have to call verify_standard and then verify consensus only to know invalidity cause of your transaction
< wumpus>
yes, the important point is to name it clearly
< luke-jr>
and other stuff using it will just work
< wumpus>
and document it clearly
< wumpus>
no need to split it out into yet another library or mess around with .pc files IMO
< sipa>
ariard: btw, bitcoin core internally also calls script verification twice to determine cause of failure (once with and once without standardness rules enabled)
< sipa>
at least in some cases
< ariard>
yes exactly in mempool acceptance, that what I had in mind
< sipa>
that said, i'm still a bit confused why you need this
< cfields>
how about just make a copy of bitcoinconsensus.cpp, make the policy changes to the new one, and maintain them in parallel?
< cfields>
That's how we always say we'll handle stuff like this, anyway :)
< sipa>
heh
< luke-jr>
I would assume it's a new .cpp file anyway
< ariard>
sipa: you want to verify your witness builder and you may have a lot of different cases in LN
< sipa>
ariard: if you follow BOLT 3, that shouldn't be needed, right?
< sipa>
(i understand the use for testing, but the point of having a lightning standard seems to be that it removes the need for doing this in production)
< MarcoFalke>
regtest should have all standardness flags like mainnet, so testing against regtest during development should be sufficient (haven't checked)
< ariard>
you may also want to check in production, witness may contain input from different parties
< cfields>
If we're doing a new policy function, it would be nice to return a bitfield of failed (non-fatal) checks rather than a true/false. That may require too much interpretor refactor to be safe, though.
< sipa>
cfields: hard nack
< luke-jr>
ariard: you shouldn't care about what the inputs are..?
< cfields>
lol, that was quick.
< sipa>
cfields: yeah, sorry - i feel very strongly about the fact that we absolutely should not expose all nitty details of how we implement standardness
< sipa>
over time certain flags might be merged together or so
< sipa>
or split up depending on conditions
< sipa>
or things may get tested in different order
< sipa>
so that the first failure reported changes
< luke-jr>
a bitfield would require testing all even after one fails ;P
< wumpus>
yes, agree with that, the problem with these kind of interfaces is that they're hard to update
< cfields>
ok, fair enough.
< sipa>
if we expose a way to test policy, it is a "test things against current bitcoin core policy", not a "learn about all the things that may be wrong"
< luke-jr>
policy is inherently volatile, even with the same version
< luke-jr>
there is no "bitcoin core policy", there is "this node's policy" :/
< ariard>
yeah we shouldn't commit to policy for sure, but you're making expactations about network mempools
< luke-jr>
ariard: which is something we don't have code for at all right now
< ariard>
to be sure your time-sensitive tx is going to reach some miners pool
< ariard>
luke-jr: yes that's an issue, have you followed thread about RBF pinning on the ml
< luke-jr>
I haven't
< ariard>
tl;dr: by pinning transaction in the mempool due to loosely understood RBF rules by LN people you may delay a time-senstivie tx
< ariard>
and therefore steal people funds
< wumpus>
which makes me doubt this a bit in the first place, this function accepting your transaction only means that one version of bitcoin core accepted it at one point, it doesn't tell you about the current network
< sipa>
and can't take even your own local mempool's state into account, if you have one
< wumpus>
right
< ariard>
wumpus: you should test against late core version standardness, I doesn't _guarantee_ but give you confidence it's going to broadcast?
< ariard>
*it doesn't
< sipa>
ariard: i'm... unconvinced
< luke-jr>
considering we don't have code for what you really need, maybe this should just be an entirely new library separate from Core?
< luke-jr>
perhaps even adapting on-the-fly
< sipa>
ariard: there is no reason to assume that checking against script standardness rules makes anything actually harder for an attacker
< ariard>
sipa: on standardness verification or documenting better mempools rules?
< sipa>
ariard: because there may be relay/validity policies that are outside script, and equally easy to attack
< ariard>
sipa: I agree it doesn't exclude all attacks vectors but you reduce surface
< sipa>
ariard: my point is that this sort of approach should be used for testing (is the stuff my code produced acceptable to the network)
< sipa>
but in production there is no way around querying an actual node for acceptability
< ariard>
let's say an attacker can degrade acceptability of your transaction by sending you non-standard stuff
< sipa>
anyway, this is not an argument against exposing a standardness check... just be aware that its use is limited
< ariard>
if you can verify this, without a full-node access, you should be able to do so
< sipa>
ariard: if an attacker can just move to an equally-cheap attack you can't detect, have you gained anything?
< sipa>
(if you have an argument why that is substantially harder to attack non-script stuff, you would have a point)
< ariard>
sipa: I see a difference between standardness on the script and standardness on mempool acceptance
< ariard>
like in RBF pinning as exposed on the ml, attacker has to commit a tx which may confirm in some block
< ariard>
and so loose a fee by trying the attack
< sipa>
ok, so there is an extra cost to bypassing script... that's a good argument
< luke-jr>
hmm, got d/c - apparently logs did too
< luke-jr>
sipa: no code we have can determine that
< luke-jr>
[19:45:27] <luke-jr> considering we don't have code for what you really need, maybe this should just be an entirely new library separate from Core?
< luke-jr>
[19:46:33] <luke-jr> perhaps even adapting on-the-fly
< sipa>
you're welcome to write that :)
< ariard>
okay so extending bitcoinconsensus with a verify_standard function for now, with a clear documentation what's its limited and how it should be used?
< ariard>
and I also propose to open an issue explaining different ways an attacker may exploit standard rules and different cost between them
< ariard>
like for all this weired multi-party time-sensitive cases
< luke-jr>
sipa: inability to configure it is a reason not to expose it
< luke-jr>
defaults should only be just defaults
< wumpus>
I don't think standard rules are supposed to ever be an attack vector
< luke-jr>
ariard: I still think NACK on a policy function in lib
< ariard>
wumpus: but sadly they are, go through ml post
< wumpus>
isn't this only a problem for 0-conf?
< sipa>
you don't even need RBF etc for that; LN very inherently relies on predictability of confirmation
< ariard>
luke-jr: it's just a question on how to make them practical between -testmempoolaccept and verify_standard
< luke-jr>
wumpus: Lightning is all about unconf
< luke-jr>
ariard: testmempoolaccept is practical ☺
< ariard>
wumpus: it's more subtle you blind your counterparty to avoid them having their alternative tx timing-out some output
< luke-jr>
ariard: but will never tell you network-wide policies
< wumpus>
luke-jr: I thought channels were only opened once the opening transaction confirms, same for other changes
< ariard>
luke-jr: if you wnat to run your test framework quicly asking for a full-node running isn't practical
< luke-jr>
wumpus: yes, I thought so too
< luke-jr>
wumpus: I don't really understand the practical issue
< ariard>
like I'm travelling and want to be able to test without network access
< luke-jr>
ariard: yes it is
< luke-jr>
regtest works fine without network
< ariard>
luke-jr: it makes running test slower
< wumpus>
I'm not sure you traveling without network access warrants adding a new function to libconsensus :)
< wumpus>
I'm sure you could just hack something that works for yourself
< luke-jr>
ariard: Bitcoin Core does testing this way; it works fine
< ariard>
wumpus: yeah yeah right, it's more running test smoothly :)
< sipa>
it is far less practical for an external project to write a test that needs to spin up a bitcoind
< sipa>
than it is for us to spin out the thing we're testing and building ourselves
< luke-jr>
sipa: why?
< sipa>
come on
< luke-jr>
you just use PATH instead of specifying the binary path…
< wumpus>
it might be more practical than linking against bitcoin core at compile time though :)
< ariard>
luke-jr: longuer dependencies chain
< luke-jr>
ariard: ⁇?
< luke-jr>
depending on bitcoind vs depending on libconsensus is the same chain
< wumpus>
no, the depenency chains are the same in both cases, the only difference is compile time versus run time dep
< wumpus>
yes
< ariard>
luke-jr: you're asking people developping and testing higher stuff to have a full setup bitcoin env
< luke-jr>
ariard: 1) regtest doesn't need any special setup; 2) I would expect every developer to have a full setup Bitcoin env anyway
< luke-jr>
I mean, if you really want to, you *could* just copy Bitcoin Core's test suite…
< sipa>
this reminds me of gpg insisting to not having a library because they needed to control pinning of stuff in memory, which IMO led to how terrible gpg integration in everything is
< sipa>
(that's not the only reason, to be clear)
< luke-jr>
sipa: I'm not sure that's a fair comparison
< ariard>
luke-jr: but at the end doing the RPC call, it's just slower to run a test suite
< luke-jr>
he's using code for something it isn't intended to be used for, in a test-only environment..
< luke-jr>
ariard: premature optimisation
< sipa>
for development, having an API for something is so much more practical than needing all kinds of logistical mess to spin up binaries and whatnot
< sipa>
for some things, that's inevitable going to be needed anyway
< sipa>
but not everything
< luke-jr>
sipa: it's for testing though, not development
< sipa>
what's the difference?
< luke-jr>
he already needs to spin up lightningd presumably
< luke-jr>
which itself also uses RPC
< ariard>
luke-jr: no need to spin up lightningd, some test framework aren't using an external process at all (Rust-Lightning do so)
< jnewbery>
Does anyone know if GETBLOCKS is used by anything on the network? I've checked getpeerinfo on my node and I haven't received any
< jnewbery>
and more specifically, is anything using the GETBLOCKS hashContinue method for initial sync?
< jnewbery>
Is it even possible to IBD using a pre-headers-sync node?
< luke-jr>
jnewbery: why wouldn't it be?
< sipa>
yes, you can sync using pre-headers-sync; bitmex tested this a few months ago even i think
< phantomcircuit>
jnewbery, it's of course possible to sync pre-headers
< MarcoFalke>
We should add a test for that
< sipa>
jnewbery: hashcontinue is essential to getblocks-based sync
< sipa>
MarcoFalke: yes
< BlueMatt>
oh missed that whole conversation, yea, I dont think we can avoid exporting standard script verification flags
< BlueMatt>
sadly its really critical
< wumpus>
I have one peer that sent getblocks, it identifies as /Satoshi:0.14.0/ (no idea if it really is)
< BlueMatt>
to, like, any real bitcoin use where you want to verify that something will be accepted, which is, largely, the point of libbitcoinconsensus as it exists today (given it cant do full verification)
< BlueMatt>
and, really, I think we may be the only users of libbitcoinconsensus lol
< luke-jr>
we don't use it..
< luke-jr>
or you mean c-lightning "we"?
< ariard>
we, Rust-Bitcoin ecosystem
< BlueMatt>
I have no idea what c-lightning does
< wumpus>
probably not as it's the only message ever sent besides pong and version/verack :)
< sipa>
wumpus: 138.197.142.142 ?
< ariard>
they export some part of consensus code directly in their codebase last time I ccheck
< luke-jr>
one of my servers has 4 peers using getblocks
< luke-jr>
/bcoin:2.0.0-dev/
< wumpus>
sipa: no, 167.172.51.113
< sipa>
wumpus: looks like a spy node
< sipa>
interesting
< luke-jr>
/Satoshi:0.14.0/ 138.197.142.142:33310
< sipa>
mine is also a claimed 0.14.0 that has only sent getblocks/pong/verack/version
< jnewbery>
0.14 that sent only getblocks and pong is clearly a spy
< sipa>
bcoin uses getblocks i think, or at least did until recently
< luke-jr>
both of these peers have 2 entries in getpeerinfo for some reason
< jnewbery>
luke-jr: how does that happen?
< luke-jr>
the bcoin peer has used: addr, getblocks, getdata, headers, inv, ping, pong, reject, sendcmpct, tx, verack, version
< elichai2>
I'm trying to add OSX to libsecp travis and it complains on bash syntax which is perfectly valid(in linux hehe), if anyone here has Mac-OS and is free to help me debug it then I'd appreciate it :)
< elichai2>
(or if there's some service that gives me shell access to OSX)
< sipa>
elichai2: bash implements more than POSIX shell requires
< elichai2>
sipa: can I just get bash on OSX?
< sipa>
elichai2: no clue
< elichai2>
thanks, I'll try that avenue in travis :)
< sipa>
(i believe it's called macOS now)
< achow101>
anyone want to send me a new blank wallet that was created on a big endian arch? I'm digging into some BDB disk format stuff and want to check if it's arch independent
< elichai2>
So it seems that macOS(:P) does use bash, but bash 3.2, while most linux dists out there use 5+