< bitcoin-git>
[bitcoin] rojarsmith opened pull request #15521: Fixed some times can not remove "$SUFFIX-dirty" on version number cor… (master...master) https://github.com/bitcoin/bitcoin/pull/15521
< wumpus>
i think it's time to do a 0.18.0rc1 soon
< jonasschnelli>
In IBD we account the maxmempool to the dbache, though, I fail to find the codepart that makes sure we flush the database once in sync with a db-cache not adding the maxmempool
< jonasschnelli>
wumpus: ack
< wumpus>
PSA: please don't assign anything to 0.18.0 anymore unless it's a regression that needs to be fixed in a RC, thanks
< wumpus>
backporting #15463 seems a bit premature as it's not merged to master yet; let's wait for the final version to do that-#15486 still has ongoing discussion and review-same for #15402
< wumpus>
so I'm not sure there's anything that should block tagging rc1, say, right now, and getting it out and getting some testing; as for any major release it's extremely likely at least a few RCs will be needed to fix problems that people find along the way
< wumpus>
should we put the preliminary release notes in the wiki again? I think that worked quite well with earlier major releases?
< fanquake>
wumpus ack tagging an rc1
< fanquake>
also ack using the wiki, worked alright
< wumpus>
yep first need to merge the release note fragments I see
< wumpus>
will do that and then put the release notes on the wiki and tag RC1
< fanquake>
\o/
< fanquake>
0.18.0 release schedule running on time
< wumpus>
:D
< wumpus>
"the chainstate database for this release is not compatible with previous releases" this isn't true for 0.18 is it?
< fanquake>
off the top of my head, I don't think so
< fanquake>
if there's anything about Windows XP it can probably also be removed
< rafalcpp>
is anyone working on extracting boost::process to standalone library to be included and provided for platforms where system's libboost lacks ::process ?
< wumpus>
I know the format changed with 0.15, I don't think we need to mention downgrading to that
< wumpus>
will keep the upgrade notice for the first time running 0.15 or newer
< wumpus>
rafalcpp: not that I know of
< fanquake>
rafalcpp some boost:process related discussion in 15421 and 15382
< rafalcpp>
actually, why not just build boost from source code, on platforms where it is older than 1.64
< rafalcpp>
just compilation time is the concern?
< wumpus>
convenience of building as well
< rafalcpp>
wumpus: but the alternative, of using extracted boost::process, will it be much better?
< wumpus>
the depends system is nice but doesn't really work on every platform (say, BSDs etc)
< wumpus>
better in what regard?
< wumpus>
from what I heard boost::process used to be a separate library in the first place
< bitcoin-git>
[bitcoin] practicalswift opened pull request #15522: Document sizeof(size_t) assumptions in assumptions.h (master...size_t-assumptions) https://github.com/bitcoin/bitcoin/pull/15522
< rafalcpp>
wumpus: what do you mean it does not work on BSD? I think the process would be like (I assume) Gitian does it inside: git clone main boost, then checkout few of submodules (::process and its deps) then b2, make/make install to prefix, and tell autotools to puck that up
< wumpus>
there's no reason it wouldn't theoretically work, but just try it out, go to depends and do `make` (excluding qt) on say, OpenBSD, I think it fails somewhere along the way
< wumpus>
you're welcome to fix that of course! but until we can be *sure* that the depends build works on all the platforms that bitcoind builds on, "just use depends" is not a valid requirement
< wumpus>
this is why something that builds as part of our own tree is likely a more feasible approach
< wumpus>
it wouldn't be as much of an issue if 1.64 wasn't so recent making it an unreasonable minimum requirement. Though it might be fine if it's only used for optional functionality (e.g. as detected in configure).
<@luke-jr>
"just use depends" is never a valid requirement (outside of obsolete versions) -.-
< wumpus>
luke-jr: boost 1.64 is thus uncommon it would effectively be the case
< rafalcpp>
I'm not sure what do you mean by that depends - is this your custom script that downloads some dependencies (curl instead git submodule btw) and builds them? (from ./depends/)
<@luke-jr>
wumpus: but it's optional, right?
< wumpus>
fanquake: right, that
< wumpus>
rafalcpp: yes
< rafalcpp>
but the problem is that this depends/ method anyway is not working, independant of boost::process
< rafalcpp>
(*on bsd)
< wumpus>
that's not a problem at the moment because the version of boost (as well as other dependencies) on various BSDs is recent enough to build bitcoind
< rafalcpp>
I could make another small script to grab + build enough of boost to have boost::process for Bitcoin
< wumpus>
sorry if there's anything we don't want, imo it's another dependency-downloading and building script
< rafalcpp>
ok then what sollution is acceptable?
< wumpus>
make only *optional* functionality depend on boost 1.64
< wumpus>
that would be the first step
< wumpus>
if someone decides to backport boost::process functionality later or something like thta, that'd be great, but it can all start with that
< rafalcpp>
by backporting you mean OS distros doing that? I wouldn't count too much on that happening
< wumpus>
no, I mean extracting and backporting it, as you were suggesting only a few messages ago
<@luke-jr>
rafalcpp: include a copy of just boost::process in the Bitcoin source code, used when the system lacks it
< rafalcpp>
btw, the script is probably git clone && b2 && make install and telling automate to use it and that is all
< wumpus>
this just keeps going back and forth, we already discussed this at one of the meetings too: I'd say just go ahead with whatever requires boost 1.64 and make it optional functionality
< wumpus>
a lot of people don't build with the wallet, so if say, the new external signing support requires that library then it would be outrageous to require boost 1.64 in all cases
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #15524: [0.18] doc: Remove ppa from linux build instructions (0.18...1903-docPPA) https://github.com/bitcoin/bitcoin/pull/15524
< mmgen>
wumpus: you should make a release checklist, to avoid relying on memory for stuff like that every release
< wumpus>
if you alrady fetched the tag do: git branch -d v0.18.0rc1 && git fetch origin v0.18.0rc1
< achow101>
mmgen: there is. see doc/release-process.md
< wumpus>
mmgen: this is on the check list IIRC
< wumpus>
it's a new step though for this version
< wumpus>
happy I realized it so soon at least
< mmgen>
wumpus: ahh, now I see: setting CLIENT_VERSION_RC was added only recently
< wumpus>
yes, and ==0 happens to be 'final release' so that would be unfortunate
< mmgen>
mmgen: indeed!
< mmgen>
Sorry, s/mmgen:/wumpus:/
< dongcarl>
Wondering why all the release schedule issues are marked "good first issue"
< wumpus>
basically to make it something people stumble over immediately when looking for good first issues
< luke-jr>
(spambots seem to have stopped for a while now)
< wumpus>
and no one can pretend not to be aware of the release schedule :)
< dongcarl>
Ah I see
< wumpus>
I guess it's somewhat less relevant now that "pinned issues" exist, but it predates that functionality by far
< sipa>
wumpus: no the latest database change iirc was the txindex in 0.17, and that one is backward compatible even (old nodes just won't see the index created by new nodes)
< MarcoFalke>
wumpus: Are you going to create the "release schedule for 0.19.0"-issue? :)
< wumpus>
MarcoFalke: yes, though I think doing so now is overly optimistic :) let's wait for 0.18 to be -final before scheduling it
< MarcoFalke>
Ah, makes sense
< wumpus>
dongcarl: huh did I forget one?
< wumpus>
dongcarl: I don't see it
< wumpus>
oh, they placed it into the historical release notes directory? what???
< MarcoFalke>
whoopsie
< MarcoFalke>
wrong dir
< dongcarl>
lol
< MarcoFalke>
dongcarl: blame youself
< MarcoFalke>
. s/youself/yourself/
< wumpus>
anyhow: release notes are on the wiki now, please edit them there
< * dongcarl>
git blame self
< dongcarl>
Okay, so I'll add it, and PR to remove the file?
< wumpus>
yes, thank you
< dongcarl>
gotcha
< sipa>
wumpus: doc/psbt.md is not new in 0.18 (and it's been hardly touched for 0.18 so far, though i think we need to add deriveaddresses/importmulti/getdescriptorinfo to it before release
< sipa>
eh
< sipa>
i'm confusing psbt with descriptors
< sipa>
we need to add analysepsbt/mergepsbts/utxoupdatepsbt
< wumpus>
sometimes, things that have been backported to minor releases stick around in the release notes for the major one, but I don't think that's the case here
< sipa>
wumpus: just pointing out that the release notes wiki now says that doc/psbt.md is new while it's been there since 0.17; i'll improve the text
< sipa>
wumpus: added some todos to the wiki; i'll address them myself when i find the time, if someone doesn't beat me
< dongcarl>
Anyone know how to push to the wiki? I've got `git@github.com:bitcoin-core/bitcoin-devwiki.wiki.git` as my remote and git tells me permission denied
< sipa>
oh, that writeup was in the release notes of 0.17, and was only turned into a separate doc afterwards
< sipa>
thanks for getting my memory straight
< harding>
sipa: np. You did catch a problem in my first release note edits where I didn't catch some backports, which probably helped confuse you. That was corrected, and I've tried to be more careful since.
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #15527: doc: Remove pr release notes file in wrong dir (master...1903-docRelPr) https://github.com/bitcoin/bitcoin/pull/15527
< harding>
dongcarl: can you edit in the web interface?
< dongcarl>
harding: it allows me to open web interface editor, haven't tried submitting change tho.
< dongcarl>
But I'd obviously like to edit in my comfy editor
< harding>
dongcarl: maybe try a one-character edit in the web interface to see if it throws some useful error (and then undo your edit).
< dongcarl>
That worked...
< dongcarl>
Weird
< dongcarl>
Anyways, I'll just do it on the web then...
< MarcoFalke>
dongcarl: Sorry, I already submitted that hunk
< harding>
dongcarl: I just tried and it lets me edit via the web interface but git push for the exact same change gives me: remote: Permission to bitcoin-core/bitcoin-devwiki.wiki.git denied to harding.
< dongcarl>
Hmm... I'm a little confused why we're using the wiki page of an empty repository instead of just a repository...
< dongcarl>
harding: seems like a bug to me
< wumpus>
because it works...
< wumpus>
people can edit it and that seems to work pretty well, no PR overhead
< dongcarl>
Ah I see, PR overhead
< wumpus>
the wiki UI is also slightly more user friendly than editing files in a repository, of course, that doesn't matter if you only want to check it out locally
< wumpus>
it's an empty repository because that's the only way github will let you set different permissions...
< jonasschnelli>
gmaxwell: remember when we talked about the 3 rounds of chacha20 required for encrypted messages with less then 64 bytes?
< jonasschnelli>
you mentioned that reducing it to two may be possible since the first round derives the Poly1305 key and wastes 32bytes of that round
< jonasschnelli>
But the ChaCha20Poly1305@openssh construct uses two independent keyed cipher instances (main and header)
< jonasschnelli>
The header-instance encrypts only the packet length..
< jonasschnelli>
I think by reducing it two minimum two rounds of ChaCha20 (one for the poly1305 key, one for the actual message), we break the following property:
< jonasschnelli>
"By using an independently-keyed cipher instance to encrypt the length, an active attacker seeking to exploit the packet input handling as a decryption oracle can learn nothing about the payload contents or its MAC"
< gmaxwell>
we talked about this in here before.
< gmaxwell>
the mac setup run has extra data in the output, so there is no length overlapping.
< sipa>
i don't follow; where do the 3 rounds come from?
< sipa>
oh, one for length, one for data, one for mac
< gmaxwell>
and the mac run has 32 extra bytes that are just thrown away.
< gmaxwell>
there was also a mailing list discussion about this, and the way it was done this way IIRC from it was because openssl didn't provide access to the extra bytes.
< gmaxwell>
e.g. pure compatibility hack.
< gmaxwell>
no cryptographic justification for that redundancy.
< jonasschnelli>
gmaxwell: but what about the two independently-keyed cipher instance?
< jonasschnelli>
We could drop that and just use a single key with a single ChaCha2 cipher instance and reduce it two minimum two rounds
< jonasschnelli>
But I'm unsure if this brings disadvantages when the AAD (the length) is encrypted with the same key than the actual payload
< jonasschnelli>
The current BIP151 way is ECDH_SECRET->HKDF->k1 for AAD encryption, ECDH->HKDF->k2 for the payload encryption
< sipa>
i think the idea is that there are two independently-keyed chacha20 instances; one for the AAD, and one for the payload, but that the first 32 bytes of the payload one are used for the poly1305 key, instead of using the first 64 bytes (and throwing away the last 32)
< jonasschnelli>
we could do: instanceB->poly1305 key & AAD encryption, instanceA->payload encryption
< sipa>
which means you need one invocation of the AAD instance every 16 messages or so (as you only need 4 bytes from the AAD cipher output for each), and one invocation of the payload instance for every 64 bytes of data (offset by 32 bytes)
< jonasschnelli>
sipa: we do derive a poly1305 key per message though
< sipa>
that seems wasteful
< jonasschnelli>
well,.. I feel unconfirtable altering the AEAD construct further more
< sipa>
how fast is out chacha20 code?
< sipa>
*our
< jonasschnelli>
Not very optimized,.. I have benchs for the AEAD but not for ChaCha alone
< jonasschnelli>
# Benchmark, evals, iterations, total, min, max, median
< jonasschnelli>
I should have done <64 though for small
< sipa>
so this means 500 MB/s, or over 1M 256-byte messages per second
< jonasschnelli>
And thats for the 3-rounds @openssh version with no asm optimisation
< jonasschnelli>
I think aprox. the same performance as the current 4byte sha256 checksum
< sipa>
how much of this is due to the poly1305?
< jonasschnelli>
I just asked that myself...
< sipa>
can you run the benchmark with the poly1305 code commented out or so
< jonasschnelli>
I can add a bench for that though
< jonasschnelli>
I'll do that.. but need to fix the current working-tree first
< jonasschnelli>
sipa: but we would drift away from a poly key per message, wouldn't that mean a single message can't be authenticated by itself?
< sipa>
jonasschnelli: no
< sipa>
you just cache the output of the aad cipher
< sipa>
oh, i'm confusing myself
< jonasschnelli>
doesn't that result in eventually waiting for additional messages in order to check the MAC?
< sipa>
no
< sipa>
you can compute the chacha20 cipher output as far ahead as you want, there is never a need to wait for anything
< sipa>
poly1305 needs a 32 byte key; right now you generate a per-message (64 + N) byte output, and use the first 32 bytes as poly1305 key, throw away 32 bytes, and use the rest for the payload
< sipa>
the idea is that you just don't throw away those 32 bytes
< jonasschnelli>
Yes. I see that... but I don't see how more then the 3 bytes for the AAD (the packet length) could be used
< sipa>
likewise, you're now generating 64 bytes from the AAD cipher for encrypting the length, but only using the first 4; the idea is to just not throw away the rest
< sipa>
or the first 3
< sipa>
jonasschnelli: there's nothing special about the message counter
< sipa>
chacha20 is just a giant stream of bytes
< sipa>
where we use output of position (2^64*message + bytepos)
< jonasschnelli>
aha.. I think I know what you mean now
< sipa>
now, depending on the speed of things... this may all be overkill
< sipa>
so i'd like to know how fast poly1305 is
< jonasschnelli>
So 64 bytes from the ChaCha round that does the Poly-key/AAD output, use 32for the poly-key, 3 for the length, remains 30.... so we need to kick a round for the next poly key,...
< sipa>
if most of our time is spent in poly1305, then squeezing out extra chacha20 performance isn't really going to matter much
< jonasschnelli>
maybe it's on over-optimization
< sipa>
no no no
< * jonasschnelli>
listening
< sipa>
don't mix the aad and the payload
< jonasschnelli>
I don't
< jonasschnelli>
I just have problems processing your:
< jonasschnelli>
<sipa>which means you need one invocation of the AAD instance every 16 messages or so (as you only need 4 bytes from the AAD cipher output for each), and one invocation of the payload instance for every 64 bytes of data (offset by 32 bytes)
< sipa>
generate 64 bytes from the AAD cipher every 21 messages, and cache them. this output will be used to encrypt the lengths of the coming 21 messages
< sipa>
that's all you ever do with the AAD cipher
< jonasschnelli>
ah.. shit. right
< jonasschnelli>
I confused myself by deriving the polykey from the AAD instance
< jonasschnelli>
yes, yes. I see
< sipa>
then you generate 32+N bytes from the payload cipher for every message (with incrementing message counter), the first 32 of which you use for the poly1305 cipher, the rest for the payload
< jonasschnelli>
so the special length encryption property could come at very limited processing time
< sipa>
yes
< jonasschnelli>
this means we always do two rounds for an message >32 bytes... but the AAD chacha round runds only every 16 message
< jonasschnelli>
or 21 with 3 bytes length
< sipa>
yes
< jonasschnelli>
Cool... let me bench and specify this
< jonasschnelli>
Also wonder how much it would be possible to gain by using the optimized code from Zinc (Wireguard) which seems to evetually get merged to the linux kernel
< gmaxwell>
re benchmarking, benchmark target should be arm. if what we cared about was x86_64 we should be using AES-GCM. :)
< gmaxwell>
which is basically better in every dimension except being really slow on arm without hardware AES.
< gmaxwell>
(simpler to integrate, much more widely used, much lower power consuming (on x86_64), slightly faster)
< jonasschnelli>
gmaxwell: can't we expect ARM to support AES NI,.. I guess ARMv8-A has AES native instructions?
< gmaxwell>
someday. maybe. they made it optional, again. IIRC
< gmaxwell>
(of course, its optional on x86_64 too, but every modern chip provides it... arm has a long long history of optional features that are not proided. :()
< sipa>
x86 in practice only has two manufacturers :)
< gmaxwell>
(to be clear, that chacha20/poly and AES are generally close in performance on x86_64 ... so its not awful to not use AES. and AES-GCM is pretty bad compared to chacha20 on devices without hardware AES&CLMUL)
< gmaxwell>
So I certantly still support using the chacha20, but just saying, that the performance driver here should be lower end node hardware, not desktops that decrypt at gigabits/s. :P )
< sipa>
fair point
< gmaxwell>
Also, since most of our messages are small.. it seems likely that optimizing this will be a fairly large percentage speedup.
< gmaxwell>
(might be useful to get a histogram of a ordinary nodes send and recieved message sizes)
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #15530: doc: Move wallet lock annotations to header (master...Mf1902-walletLocks) https://github.com/bitcoin/bitcoin/pull/15530
< bitcoin-git>
[bitcoin] practicalswift opened pull request #15532: Remove sharp edge (uninit member) when using the compiler-generated ctor for BlockFilter (master...BlockFilterType) https://github.com/bitcoin/bitcoin/pull/15532