< GitHub59>
[bitcoin] promag opened pull request #7506: Use CCoinControl selection in CWallet::FundTransaction (master...enhancement/use-coin-control-selection) https://github.com/bitcoin/bitcoin/pull/7506
< GitHub131>
[bitcoin] luke-jr opened pull request #7509: Common argument defaults for NODE_BLOOM stuff and -wallet (master...common_defaults_0.12) https://github.com/bitcoin/bitcoin/pull/7509
< GitHub178>
[bitcoin] luke-jr opened pull request #7510: Read/write bitcoin_rw.conf for exposing shared Daemon/GUI options in the GUI (master...rwconf) https://github.com/bitcoin/bitcoin/pull/7510
< GitHub1>
[bitcoin] paveljanik opened pull request #7511: [WIP] New ax_pthread.m4 from upstream - draft 3 (not final), for testing on all platforms (master...20160211_WIP_test_new_ax_pthread) https://github.com/bitcoin/bitcoin/pull/7511
< sipa>
wumpus: there is no release note entry for the new fundrawtransaction RPC
< wumpus>
sipa: correct
< sipa>
list of all new RPCs: disconnectnode, setban, listbanned, clearbanned, getblockheader, fundrawtransaction, estimatesmartfee, estimatesmartpriority, abandontransaction, importpubkey
< sipa>
i forgot about most of these already...
< wumpus>
me too - could mention them shortly in the release notes, for documentation they can check 'help <rpc>', the release notes are already long enough that no one will see it anyhow :)
< sipa>
maybe that's a reason for having more frequent releases? *ducks*
< wumpus>
meh, I can't take the release stress more than twice a year
< sipa>
that's why i duck :)
< wumpus>
maybe we could appoint someone to do the hammer-people-with-deadlines part for me
< wumpus>
yeah :)
< sipa>
damn, there are so many release note entries that maybe they should be organized per topic
< wumpus>
I wonder if everyone would ever agree on a sorting :)
< wumpus>
I mean, sorting by category would make some sense, on the other hand e.g. not all wallet changes are of the same importance to most users
< sipa>
ah, ok
< wumpus>
on the other hand the bottom list is already sorted by category so there is precedent
< wumpus>
and if anyone wants to sort it this would be the time, it's unlikely there will still be major changes, well apart from 7346 but that one should be merged
< GitHub123>
bitcoin/0.12 4b8d2bc Luke Dashjr: release-notes: Cover priority changes correctly, removing mentions of possible futures
< GitHub123>
bitcoin/0.12 73a0375 Luke Dashjr: release-notes: Mention possibility of priority removal in 0.13, uncertainty of priority calculation being changed back, and request community input
< GitHub123>
bitcoin/0.12 d0dbb6d Luke Dashjr: release-notes: Remove suggestion to use 0.11
< morcos>
heh, i wonder if i should explain what estimatesmart does? nah..
< sipa>
morcos: i honestly have no clue what it does
< morcos>
the idea is i might add more functionality to it in the future, so didn't want people to start depending on it yet. but the main advantage is if you ask for an estimate at a target of N and there is no estimate, it starts incrementing N until it can give you an estimate and then returns the target it was able to find an estimate at as well
< morcos>
but it also does a couple other smart things like recognize mempool limiting min fees
< morcos>
estimatesmart is what all the internal code uses now, but didn't want to break functionality that depended on the old API
< GitHub71>
[bitcoin] laanwj opened pull request #7517: test: script_error checking in script_invalid tests (master...2016_02_test_script_errors) https://github.com/bitcoin/bitcoin/pull/7517
< wumpus>
the whole shitload of ways that various ways of setting settings can interact
< wumpus>
and whether the right one takes precedence
< wumpus>
we don't have any tests for that, certainly not for the GUI
< Luke-Jr>
I got the param. interaction part, it's the backward compatibility part I don't understand.
< wumpus>
that settings set through QSettings in a previous release still work
< Luke-Jr>
k
< wumpus>
I agree that's not necessarily a problem as long as the set of settings set through QSettings and through your mechanism is disjoint
< wumpus>
but I was just trying to be general because I know you won't let it stay that way
< Luke-Jr>
I had no real plans to change that, actually, but you're probably right that at least some should, and regardless testing QSettings seems reasonable
< wumpus>
I think gui-only settings should at least stay in QSettings
< Luke-Jr>
yeah, I agree with that
< Luke-Jr>
wumpus: any thoughts on eliminating the enum? or is that just necessary?
< wumpus>
I'm not sure. I like having an enumeration of settings, that's better than having arbitrary strings. Although something could be said for splitting GUI-only and core options.
< Luke-Jr>
seems like a lot of unnecessary code just to translate enumeration->string
< Luke-Jr>
but I guess so far the 2 I added aren't that simple anywayh
< wumpus>
the problem with the current core option handling is that it is sloppy, and part of that is due to using arbitrary magic strings everywhere
< wumpus>
I don't necessarily want to import that into the GUI too :-)
< Luke-Jr>
hm, so maybe the solution is to use an enum everywhere?
< wumpus>
possibly, I do think the options handling needs to be revamped at some point
< Luke-Jr>
perhaps that would be best off as part of the setconfig RPC idea
< Luke-Jr>
since it needs a registry of params anyway
< wumpus>
so that it can report misspelled or otherwise invalid options, better value checking (at init time) etc
< wumpus>
exactly
< wumpus>
thinking about it, a global enumeration will not actually be possible; different sub-parts of the application can have their own settings, e.g. the wallet, and need to be able to register them independently
< Luke-Jr>
wumpus: a registration class can abstract the enum part?
< wumpus>
ok
< GitHub129>
[bitcoin] promag opened pull request #7518: [RPC] Add changeAddress option to fundrawtransaction (master...enhancement/fundrawtransaction-with-changeaddress) https://github.com/bitcoin/bitcoin/pull/7518
< morcos>
anybody want to answer a question about CWalletTx's and save me deciphering a bunch of code? I'm trying to figure out whether there is an easy way to calculate the fee of the tx from a CWalletTx or whether it would be easy to store this information if its not currently saved.
< morcos>
But the Credit/Debit stuff confuses me
< wumpus>
it confuses me too
< wumpus>
but the code to compute the fee should be thre already?
< morcos>
what i first looked at seemed to only compute the fee if the Debit > 0 (or vice versa or something)
< Luke-Jr>
someone -m -dev pls
< Luke-Jr>
also -i probably etc
< cfields>
wumpus: hmm, noticed after pushing that my rc5 win doesn't match yours
< cfields>
others match
< cfields>
checking now
< sipa>
morcos: the wallet only knows how to compute the fee for transactions where all inputs are from us
< morcos>
sipa: sigh, thats going to be annoying for my feefilter code
< sipa>
we could pass it down, though
< sipa>
but it wouldn't work in a potential future SPV mode
< morcos>
what do you mean by pass it down?
< sipa>
through the signal that informs the wallet of new transactions
< morcos>
and save it in the database?
< sipa>
yes
< sipa>
what do you need it for?
< morcos>
i want to have a filter on RelayTransaction
< morcos>
everywhere other than the wallet that that gets called is right after ATMP so i can return the fee from ATMP
< morcos>
but the wallet resends txs
< sipa>
RelayTransaction could be moved to main, where it has access to the UTXO set
< morcos>
you dont' want to relook it up though everywhere.
< morcos>
maybe that would be ok for the wallet resend though
< morcos>
oh, sdaftuar points out that in all the other use cases when you look it up, you'll find it in the mempool... so maybe that would be ok too
< morcos>
ok i'll play around
< michagogo>
cfields: can you pull my sigs? Having a PR outstanding is breaking my script
< michagogo>
(I have rc5 sigs ready to push up, I think)
< cfields>
michagogo: i don't see a PR?
< michagogo>
Hm?
< * michagogo>
looks
< michagogo>
Oops.
< michagogo>
There we go
< michagogo>
The problem is that my script relies on FFs to avoid things breaking
< michagogo>
Which breaks things when there I try to use it multiple times before one session is fully resolved
< cfields>
ah ok
< * michagogo>
looks to see at what point his script crashed
< cfields>
michagogo: rc5 sigs would be great when you've got them. wumpus and i have a mismatch
< cfields>
i re-ran and got the same result as before
< GitHub160>
[bitcoin] paveljanik opened pull request #7520: LibreSSL doesn't define OPENSSL_VERSION, use LIBRESSL_VERSION_TEXT instead (master...20160211_LibreSSL_compile_fix) https://github.com/bitcoin/bitcoin/pull/7520
< wumpus>
<wumpus> huh, I'm getting tons of non-final transactions on my node, is block 397955 up to date? <- never mind, I was confused, I'm logging reject messages from *other* nodes, and some of them are rejecting the transactions I relay with non-final
< wumpus>
all probably pre-0.12 versions that still request transactions during IBD
< sdaftuar>
i think 0.12 also requests transactions during IBD if i remember right
< wumpus>
ok, I was pretty sure there was a pull to change that, but I may be confused
< sdaftuar>
it's in master, just missed the 0.12 cutoff i think
< michagogo>
wumpus: midnightmagic has always had weird stuff going on.
< michagogo>
I don't know if anyone ever figured out what's different in his setuo
< michagogo>
setup
< wumpus>
michagogo: indeed
< morcos>
sipa: what were you wondering about with regards to BIP68 and the wallet. the previous implementation had code to indicate if your wallet txs were non-final which i removed. but that should be quite rare as it won't end up in your mempool in the first place
< cfields>
strange, i have tons of single-byte differences
< midnightmagic>
the host is a straight Ubuntu LTS update + upgrade machine. There are KVM and LXC running alongside the gitian built. But it's not like it's some custom-rolled machine.
< gmaxwell>
Is there a strong argument against not to have the varrious pruning effects-- like unsetting node network-- not happen until pruning actually happens? E.g. today you can't set pruning to 200GB and operate as a completely unpruned node until the history gets that large.
< michagogo>
gmaxwell: off the top of my head, I think you'd need to kick all your peers when you hit the limit
< michagogo>
And considering some of the changes are in the form of things you can't do anymore (e.g. rescan), leaving that functionality active for a while and then suddenly turning it off with no warning could be a problem
< michagogo>
IMO pruning is a big enough change that no part of it should happen without the user's explicit action
< midnightmagic>
cfields: does your build completely finish? like does it build all the artifacts including the signed bins for win and osx?
< morcos>
gmaxwell: got a privacy related question for you regarding ResendWalletTransactions
< morcos>
Shouldn't your node not relay a wallet tx that your own node doesn't accept into its mempool?
< morcos>
crap, this might be a regression
< gmaxwell>
morcos: hm. We didn't use to. Did we manage to break that?
< morcos>
before 0.12 GetDepthInMainChain was < 0 for things not in your mempool right?
< morcos>
but now it is 0 unless it is conflicted?
< gmaxwell>
bleh. hah.
< morcos>
what do you want to do?
< morcos>
it seems to me the proper fix is to test if its in your mempool and if so re-relay, and if not , re try to accept it into your mempool
< morcos>
(and relay if successful)
< sipa>
morcos: aw
< cfields>
midnightmagic: ah, i remember that problem
< gmaxwell>
I believe that is the correct behavior.
< cfields>
midnightmagic: long story short, nuke your gitian cache
< morcos>
thats what you get when i try to find ways to avoid calculating the fee on wallet txs
< morcos>
:)
< cfields>
midnightmagic: it's due to not checking something that we need to check, i'll fix
< morcos>
gmaxwell: but are we going to fix that for 0.12.0 ?
< gmaxwell>
lets fix and see what the fix looks like? It's a non-trivial privacy regression.
< morcos>
ok, i can do it right now.
< morcos>
i don' tthink its too much code, but i'm worried that it might need a bit of testing
< warren>
Do we generally have an option to turn off transaction sending while allowing the wallet to create transactions?
< gmaxwell>
resending is horrible for privacy period; a regression wouldn't be the _end_ of the world, but I would rather not have it.
< gmaxwell>
warren: yes.
< gmaxwell>
warren: walletbroadcast=0
< GitHub14>
[bitcoin] morcos opened pull request #7521: Don't resend wallet txs that aren't in our own mempool (master...testBeforeRelay) https://github.com/bitcoin/bitcoin/pull/7521
< morcos>
Ok I think this is the idea. Not sure if you'd want it to look slightly different, was trying to keep the changes minimal.
< morcos>
gmaxwell: ^
< gmaxwell>
morcos: In what case is that force needed? I think for creation it doesn't attempt to relay if it can't be mempooled?
< morcos>
yeah, it shouldn't do anything. i think its safe to remove that and just use the mempool.exists() test to see if we need to try to accept it
< morcos>
i'm happy to change it around a bit, but i wanted to sort of point out the two different ways this was called and emphasize that behavior is only changing in one of them
< midnightmagic>
cfields: ahhh, yes thank you. I had updated, and rebuilt the gitian base image; the next was blowing everything away and starting right from scratch. I'll update the issue for that and report it fixed if that proves to be the solution.
< cfields>
midnightmagic: yea, the issue is that you built the 0.12 branch before it switched to trusty
< cfields>
the solution is adding the current compiler info to the dep packages hashid
< morcos>
gmaxwell: I'm missing required locks. Need to add those. Want me to remove the bool?
< gmaxwell>
morcos: would be a smaller diff without it.
< sipa>
morcos: you can use IsTrusted, perhaps?
< morcos>
sipa: you mean InMempool() ? I can, but it locks mempool.cs even though exists locks it itself. sigh...
< morcos>
But I was referring to cs_main
< morcos>
which is needed for AcceptToMemoryPool
< morcos>
It looks like I'd need to lock it for the entire ResendWalletTransactionsBefore function (so its not locked after cs_wallet)
< morcos>
Is that bad?
< midnightmagic>
cfields: okay. if you have a PR or something I'd be glad to leave the system in a broken state and verify the fix for you
< morcos>
Alternatively we just skip trying to resend wallet txs that are no longer in our mempool, this would actually be most similar to old behavior i guess (although we didn't use to end up with those due to eviction beforehand)
< cfields>
midnightmagic: no worries, no need to keep broken. it's easy enough to test. thanks though :)
< morcos>
Also there would be this unfortunate string of ERRORS resulting from RBF replacements I think where you try to resend the old one that was RBF'ed
< morcos>
Perhaps simpler to just do that then.. Check mempool.exists() and leave it at that? better to use InMempool?
< midnightmagic>
cfields: okie doke.
< morcos>
gmaxwell: ok thats a much smaller diff. :) but now if something gets evicted (or never made it in to your mempool), it will not automatically try to reaccept or rerelay, so this could be worse behavior depending on your point of view.
< morcos>
this doesn't affect trying to reaccept wallet txs on startup though
< morcos>
have to reboot, back in a few
< morcos>
gmaxwell: sipa: i pushed the simple version which doesn't try to reaccpet, that would be my preference, it seems like you might not always want to automatically resend things that are no longer in your mempool for some reason.
< gmaxwell>
you'll still try puting it back in on restart at least.
< morcos>
Yes, unless you have abandoned it. Which seems reasonable.
< gmaxwell>
my that is a smaller patch.
< morcos>
I'll let you ask wumpus for an RC6
< gmaxwell>
I am going to need offerings. Does anyone have a goat?
< midnightmagic>
I know people who have goats!
< midnightmagic>
Goats can eat poison ivy without passing it to their milk *and* will clean roadside brush like scotch broom! :-D
< sipa>
not sure that's worth doing another rc for
< gmaxwell>
I think it would also be fine to include it in a release without RCing it, assuming we test it.
< morcos>
sipa: i don't feel strongly, but i do think it would be a pretty big privacy difference
< morcos>
actually, also it'll be a network bandwidth issue
< morcos>
everytime you have tx's evicted
< morcos>
you'll periodically resend them to all your peers
< morcos>
forever?
< sipa>
until you abandon them, which is not in the GUI?
< morcos>
correct, not in the gui
< sipa>
... i think it should be in the gui
< morcos>
you're lucky its in at all, i had to fight for that
< sipa>
but let's keep that for 0.12.1
< sipa>
yeah, i know
< gmaxwell>
I don't think it's actually a network bandwidth issue, at least not materially. If you're personally generating enough abandoned transactions... well...
< morcos>
gmaxwell: its not just one persons txs. look at it on the network scale
< morcos>
all txs that are ever evicted are still locally bouncing around both inv an tx
< morcos>
so right now for instance tx traffic in a period roughly equals number of nodes * number of txs in that period
< morcos>
going forward it'll equal small constat * number of txs that were ever evicted every X minutes
< morcos>
sorry, not very rigorous, but you get the idea
< morcos>
i might be changing my mind to think this is definitely worth fixing
< gmaxwell>
Right, -- this is a consquence of eviction generally. It isn't that bad because it's only thos which were evicted but are now placable (or do you just mean without the fix)
< morcos>
yes i mean without the fix
< morcos>
you could even sort of make an attack with this right by sending 1k sat/kB txs of small amounts to lots of random nodes
< morcos>
then they'll do the work of rebroadcasting them all the time for you
< gmaxwell>
Even on the privacy side, I was not thinking in terms of recieved. Thats a release blocker.
< morcos>
oh yes, good point. want to identify to which node an address belongs
< gmaxwell>
(corner case bad behavior in a already privacy weak part of the system where only you can trigger it is one thing... where it can be remotely triggered is another)
< morcos>
sigh, eviction is complicated
< gmaxwell>
really retransmission is a general privacy disaster. Even with this check-- if someone retransmits something you know they already sent you and should have had in their mempool: they are the origin.
< morcos>
yeah actually, retransmit is kind of weird
< gmaxwell>
this would be fixed if we moved more towards mempool reconciliation instead of transmission... also would be greatly improved if broadcasting was done in some private way... both things I want to work on in the not so far future... but for now, lets at least not make things worse.
< morcos>
I suppose disabling that is too big a change
< morcos>
There is already a manual way to do it
< gmaxwell>
well you can all of this transmission it by setting walletbroadcast=0 so at least there is that.
< morcos>
I don't think I'd ever realized how silly the resendwallettransactions is though. it's emphasized by the fact that we think its better not to do it if its not in your mempool anymore. well if its still in your mempool, why the hell wouldn't it still be in other mempools
< gmaxwell>
because you've connected to different nodes. it's stull stupid.
< gmaxwell>
s/stull/still/
< morcos>
It seems like a simple privacy fix (even if not for this release) would be to just default that to off. walletbroadcast=0 requires outside work of some kind. but taking your chance with initial broadcast only is a reasonable middle ground. shoudl at least be an option.
< morcos>
anyway, ok so if we think we NEED to do something for 0.12. what is that something? the small change i made?
< gmaxwell>
I believe so!
< gmaxwell>
(am testing it)
< Luke-Jr>
paveljanik: search for autoconf/m4 manuals; admittedly, figuring out quoting is mostly trial-and-error for me
< morcos>
i guess what i said before about integrating all previous evicted txs is an exaggeration of how bad it would be, because one they become conflicted they'd no longer get resent