< Luke-Jr>
maybe it'd be easier to try to figure out 7521 here? [ gmaxwell morcos ]
< morcos>
i'm here
< sipa>
Luke-Jr: with or without 7521, an evicted transaction is not accepted back into the mempool
< sipa>
without it, however, it does get rebroadcast
< Luke-Jr>
ok, but I think that's the expected/desired behaviour
< Luke-Jr>
how is it ever going to get mined if the sender/receiver don't rebroadcast it?
< morcos>
If it got evicted, its not likely to ever get mined
< sipa>
yes, that should be fixed
< sipa>
we should retry to get it into the mempool, and when it does, rebroadcast
< sipa>
but not rebroadcast despite violating your own rules
< morcos>
sipa: so you don't like the fix in 7521? or you just think we should eventually make a better one?
< sipa>
morcos: i don't think it's enough, but it's an improvement
< morcos>
i think its really unlikely to make it back into your own mempool
< sipa>
morcos: i think mempool pressure goes up and down
< Luke-Jr>
morcos: stuck transactions are never good behaviour
< morcos>
it would be much better to just abandon it and try again with a higher fee
< Luke-Jr>
we don't have wallet RBF support in 0.12..
< morcos>
you don't need wallet support
< Luke-Jr>
…
< Luke-Jr>
morcos: what you're proposing there results in transactions that are literally stuck forever and unrecoverable
< morcos>
that is what abandontransaction is for
< Luke-Jr>
abandontransaction is not usable to users
< Luke-Jr>
it shouldn't be encouraged, much less requied
< morcos>
what about the attack i and gmaxwell described above
< * Luke-Jr>
gets a sense we had that discussion before
< morcos>
you send small very low fee txs to random addresses
< morcos>
for instance if you send 1000 sat/kB txs repeatedly they'll eventually be accepted and then evicted/expired with high probability
< morcos>
then the nodes you sent them to will continually rebroadcast them frequently
< Luke-Jr>
0.11 would do this too
< morcos>
both contributing to a bandwidth DoS attack and compromising their privacy
< sipa>
Luke-Jr: but with 0.11 you can't be certain
< Luke-Jr>
sipa: ?
< Luke-Jr>
0.11 never prunes its mempool, so it will always be there
< sipa>
Luke-Jr: with current 0.12, you can send a mempool command, and if the result doesn't contain the transaction, you know it's theirs
< Luke-Jr>
ah
< Luke-Jr>
so what sipa suggested, add to our own before rebroadcasting
< morcos>
i tried that first
< Luke-Jr>
if it fails, don't rebroadcast until it doesn't fail anymore
< morcos>
as i mentioned on the PR
< Luke-Jr>
also, how frequent is wallet rebroadcast? I thought it was some hours
< Luke-Jr>
morcos: the lock shouldn't be a problem, why do you think it is?
< morcos>
some time in the next 30 mins
< morcos>
it looks like
< Luke-Jr>
still rare enough to be okay locking I think?
< Luke-Jr>
(on that note, we should probably never rebroadcast to the same connection twice?)
< sipa>
morcos: i wonder if rebroadcast could just call "ProcessMessage" and send a tx message to the node...
< morcos>
well, my first attempt at figuring out how it would lock makes it seem like you would have to lock cs_main while you iterate through every tx in your wallet and then send the ones that need sending
< morcos>
i think you guys are trying to solve for a non-existant problem
< Luke-Jr>
sipa: my local hacks includes abstracting the "tx" message to a function so I can basically simulate that. maybe worth upstreaming for this
< sipa>
morcos: nah, lock cs_wallet, copy the matching ones to a stack-allocated vector, unlock cs_wallet, accepttomemorypool
< morcos>
if you have a very small mempool, then maybe you are right.. but at the size peoples mempools are now... if you get evicted/exprired... you really have no business being rebroadcast
< Luke-Jr>
morcos: it's not a problem because we don't have the bug you'd be introducing yet! :P
< morcos>
i'd be introducting?
< Luke-Jr>
morcos: right now, transactions never get stuck due to failure to rebroadcast
< morcos>
sipa: yeah ok, i mean i agree its solvable. but i just think its silly to solve
< morcos>
i think you're trying to reaccept and rebroadcast garbage basically
< Luke-Jr>
morcos: as soon as the fee rate goes up, you'd have a permanently stuck transaction
< Luke-Jr>
and nothing the user could do to fix it
< sipa>
morcos: there will always be transactions that are close to acceptable, and random variations push them below and above
< morcos>
sipa: i think we made mempools big enough that the bottom of the mempool basically never gets confirmed
< morcos>
i mean if we changed the rebroadcast interval to be once a week instead of once every 30 mins, then maybe it would make sense? but do people really want txs that might get confirmed in a week or two
< Luke-Jr>
morcos: if the mempool is filled with spam, it could be that ONLY the bottom gets confirmed ;)
< morcos>
it seems like its just a cleaner UI to be be like, sorry, that tx probably didnt make it
< morcos>
anywya, i certainly don't object to attempting to stick it in the mempool first
< Luke-Jr>
morcos: it's only an attempt every 30 mins. if it fails for a week, it will take a week..
< morcos>
i just think its a bigger change for no good reason
< morcos>
so i'll let someone else make that change
< Luke-Jr>
sorry, that tx probably didnt make it <-- we cannot say this until there is a recourse
< aj>
Luke-Jr: how would that work? bottom of the mempool = high priority, so the middle (low fee, low pri) is never confirmed? or?
< morcos>
personally i think we should turn off all wallet rebroadcast by default
< Luke-Jr>
aj: middle/top gets ignored by miners with better spam filters
< morcos>
i doubt it does any good
< Luke-Jr>
morcos: …
< aj>
Luke-Jr: how do you filter spam by anything other than fee/priority?
< morcos>
once you have propagated your tx, its either going to get mined or not
< Luke-Jr>
aj: some spammers use repeated patterns, for example
< morcos>
trying again (especially frequently) is not a worthwhile effort
< aj>
Luke-Jr: is there a blog post or something about that anywhere?
< Luke-Jr>
aj: no
< aj>
Luke-Jr: *arrgghhh*
< Luke-Jr>
or maybe there is, but I wouldn't know because I don't spend time on blogs
< aj>
Luke-Jr: s/or something/or an email thread, or a reddit post, or a paper, or.../
< Luke-Jr>
morcos: right now, it is an assumption that wallets must rebroadcast if they want transactions to get mined
< Luke-Jr>
morcos: any wallet that fails to do so is considered broken
< sipa>
morcos: due to non-uniform policies across nodes on the network, i think rebroadcasting actually helps confirmation
< sipa>
though that isn't relevant here; we still rebroadcast, just not after eviction
< aj>
Luke-Jr: or a github repo with code that discriminates between spam txes?
< Luke-Jr>
aj: I have a spamfilter branch that isn't entirely up to date.
< sipa>
Luke-Jr: do you use it?
< Luke-Jr>
sipa: spamfilter? of course
< Luke-Jr>
(merged into a current branch)
< aj>
Luke-Jr: !!!
< Luke-Jr>
obsolete is better than nothing at all still ;)
< morcos>
well like i said, i'm not opposed to trying to reaccept first and then relaying. i just didn't personally think it was worth it.
< Luke-Jr>
ok, so is someone working on this, or should I be?
< sipa>
i'm having a look
< Luke-Jr>
wumpus: I collected a bunch of bugfixes in master missing in 0.12 - I assume I should let them all wait for 0.12.1?
< Luke-Jr>
mostly typos, the only ones I'd really consider are:
< Luke-Jr>
* e54f412 peers.dat, banlist.dat recreated when missing
< GitHub22>
[bitcoin] luke-jr opened pull request #7522: Bugfix: Only use git for build info if the repository is actually the right one (master...bugfix_gitdir) https://github.com/bitcoin/bitcoin/pull/7522
< paveljanik>
Luke-Jr, I understand your attitude to it - the same applies to me :-) But during years I've got some decent knowledge of it and your notation - i.e. ]AC_...[ - with reversed parens got me 8)
< Luke-Jr>
paveljanik: it's de-quoting so it's substituted
< paveljanik>
I was even analysing our bitcoin_qt.m4 yesterday for missing quotes etc...
< Luke-Jr>
[] are usually called brackets in English FWIW; () are parens ;)
< paveljanik>
in Czech, they are round (kulaté) vs. rectangle [hranaté] parens (závorky) ;-)
< Luke-Jr>
interesting
< Luke-Jr>
{} are sometimes "curly brackets", but usually just "braces"
< Luke-Jr>
paveljanik: can I convince you to in 20160211_LibreSSL_compile_fix do: git rebase HEAD^ --onto 3cd836c1d
< paveljanik>
{složené} závorky
< paveljanik>
Luke-Jr, sure
< Luke-Jr>
what does sloené mean?
< paveljanik>
{} are slozene zavorky, where slozene means composed
< Luke-Jr>
i c
< paveljanik>
Luke-Jr, done
< Luke-Jr>
paveljanik: thanks. that will merge cleanly to 0.12 and master both
< Luke-Jr>
someone with repo push access: care to copy tags from my fork? branch-0.{9..12}
< Luke-Jr>
(these are the last-common-commit for the branch and master)
< Luke-Jr>
paveljanik: oh, your new LogPrintf is missing a space
< Luke-Jr>
quick, before anyone else notices! :P
< paveljanik>
:-))
< paveljanik>
force pushed
< Luke-Jr>
thanks, utACK'd
< paveljanik>
m4/autoconf is almost the same nightmare as sendmail's cf was.
< Luke-Jr>
paveljanik: and somehow every attempt to replace it has been a disaster
< GitHub26>
bitcoin/0.12 02d707f Wladimir J. van der Laan: Merge #7523: Fix of semantic failure "meet pay"...
< wumpus>
hm RPC/univalue seems to generate invalid json on special floating point values like NaN, -inf, +inf, not sure what should be generated in that case though (and luckily there is no place in the API where those are normally returned)
< gmaxwell>
The hashrate estimate thing left me wondering if it could return a nan in some case.
< gmaxwell>
but it didn't look like it.
< gmaxwell>
The ping time might be another place.
< gmaxwell>
hm guess we don't divide there at least.
< wumpus>
difficulty for an all-zeros target ;)
< wumpus>
in any case it should be fixed at some point, I'll report an issue, but doesn't look urgent.
< GitHub105>
bitcoin/master a0a17b3 Pavel Janík: LibreSSL doesn't define OPENSSL_VERSION, use LIBRESSL_VERSION_TEXT instead
< GitHub105>
bitcoin/master 621940e Wladimir J. van der Laan: Merge #7520: LibreSSL doesn't define OPENSSL_VERSION, use LIBRESSL_VERSION_TEXT instead...
< GitHub81>
[bitcoin] jloughry opened pull request #7526: fix spelling of advertise (shows up in the debug log) (master...advertize-advertise) https://github.com/bitcoin/bitcoin/pull/7526
< cfields>
wumpus: got a match
< wumpus>
cfields: awesome! so upgrading the base image did it?
< cfields>
wumpus: i just made a completely fresh one from debian and moved it over
< wumpus>
holy shit I built from scratch (without cache) get a different output now
< cfields>
maybe it has something to do with the fact that mine was a much older trusty image, so packages got held back or something
< wumpus>
I don't have them anymore I deleted them instead of moving them :(
< cfields>
i think i still have copies of everything
< wumpus>
but it certainly looks like there are more toolchain versions in play, that's the only explanation I can think of
< cfields>
yea. i've been poking for the last few hours on/off, while building. i don't see anything obvious that's updated since november
< Luke-Jr>
bisect?
< wumpus>
bisect ubuntu?
< Luke-Jr>
oh, recent OS update
< cfields>
wumpus: well, i'll fixup depends to make sure it detects any kind of system change
< cfields>
wumpus: for now, want to just take that last result, since it's the result of a fresh/clean build?
< Luke-Jr>
depends should work without determinism, right?
< wumpus>
cfields: indeed, having some toolchain version info part of the cache hash would make sense
< wumpus>
cfields: yes, I think so. We'll have to recommend everyone to update their base image and rebuild
< cfields>
wumpus: yea, i was planning to add `$cc -v | sha256` to the hash, but now i'm thinking it might make sense to take an additional optional seed as well. in gitian's case, it'll be the result of `dpkg -l` or so
< wumpus>
I'm not sure any change in dpkg should cause a rebuild
< cfields>
(without trying to add each tool by hand)
< wumpus>
right...
< wumpus>
we should at least log the tool versions though
< wumpus>
(somewhere in the cache directory)
< cfields>
well it should only be the toolchain, but i'm not sure we want to enumerate each binary that could change build output
< cfields>
yes
< wumpus>
I understand. Well maybe detection of every single tool change is too much work, but would be nice to have them somewhere to compare if something like this happens
< cfields>
wumpus: did you have to update your base to change the result, or simply nuke the cache?
< wumpus>
just the cache
< cfields>
ok
< wumpus>
the updates should happen automatically
< wumpus>
(which can take a long time, especially with LXC, as it starts off with an image with ancient packages)
< wumpus>
(but upgrades it before build every time)
< cfields>
right
< wumpus>
so probably a *mingw* .deb upgrade happened in last week
< wumpus>
probably yesterday
< wumpus>
hm no we can't actually know for sure - I don't know when my last cache was built. But it can't be too long ago.
< wumpus>
in any case: everyone should nuke cache and rebuild rc5
< cfields>
right
< cfields>
i'll make the toolchain detection top priority. will try to pr something today, once i/we track down what actually changed
< wumpus>
well top priority is getting rc5 out :) but yes would certainly be useful to have, even if it's just a dpkg -l >> $CACHEDIR so that if something like this happens we can compare the package state under which the old and new cache was built
< wumpus>
because the caching right now makes the list in the .assert only partially useful
< cfields>
yes
< cfields>
well
< cfields>
i suppose we should rebuild everything, then. might not be mingw related
< cfields>
doing osx now
< wumpus>
ideally it would list the state under which the cache is built in the assert as well in a separate section, but that would require changes to gitian. In any case having these kind of things tracable would be great.
< wumpus>
I don't see how it could be non mingw related
< wumpus>
but sure, I'll try rebuilding linux and osx as well
< wumpus>
if those differ it would make me even more interested in what changed
< cfields>
wumpus: native_protobuf differs already for me. looks like it's system-wide
< * wumpus>
gets his tinfoil hat
< cfields>
haha
< wumpus>
osx uses a compiler that isn't even part of an ubuntu package right?
< cfields>
i had considered that, but dismissed it so far :)
< wumpus>
I don't see how this is possible, I want to put 0.12.0 on hold until it is clear what causes this
< cfields>
wumpus: yes, but it builds some native packages with the system toolchain first
< cfields>
for ex: gcc builds protoc, which preprocesses, and apple-clang builds
< wumpus>
but it build *our* protoc, it would be weird if the output of protoc changes based on what compiles it
< cfields>
right
< wumpus>
suspicious, even
< cfields>
i only pointed out that native_protoc changed, not sure if it affects the end result
< cfields>
(presumably that one wouldn't, as you said)
< wumpus>
sure, let's hope for the best, I don't have build output either yet
< * wumpus>
looking at asm difference
< cfields>
i can't really think of anything that would change the osx result, beyond a gcc change that would really be cause for alarm, enough to affect the behavior of the osx-binutils
< cfields>
(cctools)
< cfields>
brb
< wumpus>
agreed. linux build difference could be explained by *both* a linux and mingw toolchain chainge, but macosx difference would be nearly impossible
< gmaxwell>
lets hope the protobuff wasn't backdoored. :)
< wumpus>
gmaxwell: in this case it would be the compiler that inserts a backdoor into protobuf </tinfoil hat mode> ;)
< wumpus>
well the compiler would insert a backdoor into protoc, which generates C++ code with a backdoor, which gets compiled in. And it's sneaky enough to result in an executable of the same size with some bytes different. Nahh... ;)
< cfields>
wumpus: the osx dmg result is the same, but the unsigned .tar.gz is different
< cfields>
wumpus: notice also that the comparisontool is different, which is just one file stuffed into a tarball
< cfields>
so i'm guessing it's either something related to file-ordering (no clue how), or something low-level like zlib
< wumpus>
linux result stays the same for me *happy*
< wumpus>
now building mac too, let's see if we at least get the same
< paveljanik>
BTW - this week I have read something about ls changes - quoting of filenames with space, different ordering etc...
< wumpus>
right, it wouldn't be impossible for a system tool like that to actually make a difference somewhere paveljanik
< paveljanik>
ie. ls from coreutils 8.25. But I think this new version was not in your builds...
< wumpus>
I doubt ubuntu stable will upgrade coreutils that soon, and AFAIK we don't use ls anywhere (at least directly), it's likely not that exact issue (but it could be a similar one)
< wumpus>
f153f1555b60edd61fa57a9b2f6b43d8d6449fd5c0e6a4ef0f4ba745c6376566 bitcoin-0.12.0-osx-unsigned.tar.gz didn't change for me cfields, osx output is the same
< wumpus>
so that points at a mingw toolchain change, and only a mingw toolchain change only. Not sure what causes your .tar.gz difference, but if the dmg is the same at least the executable is the same.
< wumpus>
from what I can see from the assembler difference it's exactly the same code, just using slightly different instructions to accomplish the same (e.g. mov shr and instead of testb setne). Typical of a compiler update.
< * wumpus>
takes off his tinfoil hat and goes to bed
< * Luke-Jr>
steals wumpus's tinfoil hat and runs away
< wumpus>
hehe
< midnightmagic>
yay other people having gitian problems besides me
< midnightmagic>
I bet it's just because I do late gitian builds after you guys are all asleep
< midnightmagic>
wumpus: but this sort of change makes it hard to reconstruct gitian sigs for earlier 0.12.0rc*
< GitHub136>
[bitcoin] knocte opened pull request #7528: autogen.sh: warn about needing autoconf if autoreconf is not found (master...warn-autoconf) https://github.com/bitcoin/bitcoin/pull/7528
< Luke-Jr>
since when is ~/.bitcoin/testnet3/bitcoin.conf ignored and why?
< wumpus>
midnightmagic: yes, the caching complicates that, as the package list in the assert is no longer that package state with which everything was built, that's why isuggested above to also log that
< wumpus>
Luke-Jr: that's always been the case, there has never been support for per-chain config files
< Luke-Jr>
also, is there any reason *not* to have regtest use non-testnet QSettings?
< Luke-Jr>
(this means QApplication appname change)
< wumpus>
I don't think regtest in the GUI is very well supported
< wumpus>
it works, but probably there's some flukes here and there
< Luke-Jr>
any complaint if I make it use its own appname/qsettings? :P
< wumpus>
I don't really care, no
< wumpus>
it's not something that affects end users
< wumpus>
so if that makes testing easier why not
< Luke-Jr>
well, IMO it's necessary for saving the network port number in QSettings
< Luke-Jr>
(#7107)
< wumpus>
just make sure it doesn't cause any other reversions, as we have hardly/none automatic tests for that stuff
< Luke-Jr>
does anything use appname besides qsettings?
< wumpus>
I don't know by heart, I suppose qt docs can help you
< * Luke-Jr>
shocked wumpus doesn't have the entire codebase memorised. :P jk
< wumpus>
hey that's unfair it was a aquestion about upstream :P
< midnightmagic>
wumpus: apparently there is an effort in debianland to build an archival package-state database which can be used to sync to specific sets of packages.
< Luke-Jr>
docs say it's just QSettings
< wumpus>
I know most of the bitcoin core codebase in detail, though I sometimes get confused by the wallet and some of the mempool workings
< Luke-Jr>
also, apparently it's read-only on Blackberry, but I don't know that we care
< wumpus>
I doubt anyone cares about blackberry no :)
< wumpus>
there's not even an android bitcoin core release, let alone blackberry
< Luke-Jr>
wumpus: GreenBits apparently runs Bitcoin Core Daemon on Android these days
< Luke-Jr>
(optionally)
< wumpus>
nice. yeah it's certainly possible, I've heard people have done it before, though I don't think it's documented anywhere
< wumpus>
probably overheats most phone CPUs, as well as wears out the flash tho :-)
< warren>
Luke-Jr: what? full validation with pruning I'm guessig?
< morcos>
So I've got a first pass at the feefilter idea coded up. I was thinking I'd also write a BIP for it. But seems like there might be some discussion of the best way to implement the idea.
< morcos>
Any suggestions as to whether I should just write the BIP and then have discussion, or show the code first or what?
< Luke-Jr>
warren: I assume
< Luke-Jr>
morcos: ?
< morcos>
Luke-Jr: The idea is that nodes can optionally broadcast a feefilter p2p message to let their peers know that their mempool is currently limited and they are not accepting txs below a certain feerate
< Luke-Jr>
morcos: this idea that fees are the only relevant metric is troubling.
< morcos>
This would save considerable network traffic in both invs that are never requested b/c they were previously rejected or requesting of txs that would fail due to insufficient fee
< Luke-Jr>
"feerate" is not a well-defined term btw
< morcos>
Well the idea I came up with is to make it optional
< Luke-Jr>
Core uses a very subjective feerate that discounts part of the transaction, for example
< Luke-Jr>
(ModSize)
< morcos>
So that if you are a node that is implementing tx prioritization, then even if you have a limited mempool, you have to at least see the hash first b/c the feerate might get modified
< morcos>
Luke-Jr: No, thats only for priority. FeeRate uses actual tx size
< Luke-Jr>
hm, it does?
< Luke-Jr>
I wonder if it should
< morcos>
It's a valid question
< morcos>
Partialy answered by segwit
< Luke-Jr>
anyway, it's something to consider in your idea
< morcos>
The way I was looking at the feature is that if you are just a relay node and you're not doing anything special with regards to priority (which is probably a significant % of the nodes) then this will help cut down on bandwidth significantly
< morcos>
If you are running different policy or are a miner and doing prioritization, then you just don't use it
< Luke-Jr>
it may bias the network toward a specific policy, which would be bad
< morcos>
If used properly it should basically have no effect other than cutting down on useless network traffic
< morcos>
I don't see why it would bias the network more than already is the case.
< Luke-Jr>
hm, that's a point
< Luke-Jr>
already there is lower bw use if you filter
< Luke-Jr>
anyway, might make sense to abstract it a little at least
< Luke-Jr>
eg, 1 byte for "feerate version" or smth
< Luke-Jr>
(maybe varint)
< morcos>
You mean have the p2p message be a bit more generic for future extension?
< Luke-Jr>
perhaps some way to say "trickle non-matching transactions"?
< Luke-Jr>
yes
< morcos>
Sure seems reasonable
< morcos>
Instead of FeeFilter 8byteFeeRate It could be InvFilter 1byteType 8byteFeeRAte
< Luke-Jr>
varints might be nicer for both fields.
< Luke-Jr>
perhaps some way to say "trickle non-matching transactions"? <-- this would nicely fit in with rate-limited priority relaying
< Luke-Jr>
(but perhaps not worth the implementation effort)
< morcos>
I'm not super familiar with the networking code, but seems like second data field type can depend on the content of the first data field. But also its not clear that its worth the effort as opposed to just defining different message types for future filters
< morcos>
Anyway, sounds like from you comments that perhaps a BIP would be a good next step
< Luke-Jr>
morcos: well, I mean I expect the feerate to be small ;)
< Luke-Jr>
no need to waste 8 bytes for 100000 when 2 is sufficient