< BlueMatt>
wumpus: fwiw, i vote we hold merge window for a day or two for things already pending incl the hd split (which I think is one more set of fixups, hopefully tomorrow, away, and is not as hard to review as it looks), bumpfee pending discussion tomorrow (I'm pretty happy with it if we fix the two pending issues - listunspent, which may just be a docs change, and getbalance, which may also just be a docs change), the compact-block-orphan-r
< BlueMatt>
ejects one (which looks pretty much mergeable to me) and preferably the extra message handler waker (9561, which is both trivial and a big gain)
< BlueMatt>
but I'd understand if thats not a popular opinion
< fanquake>
wummpus / sipa are you around?
< fanquake>
*wumpus
< fanquake>
Or anyone with admin on bitcoin/bitcoin, there is a user spamming the repo with links/comments.
< bitcoin-git>
bitcoin/master f13914a Matt Corallo: Make WakeMessageHandler public
< bitcoin-git>
bitcoin/master 241d893 Matt Corallo: Wake message handling thread when we receive a new block...
< bitcoin-git>
bitcoin/master 6696b46 Pieter Wuille: Merge #9561: Wake message handling thread when we receive a new block...
< bitcoin-git>
[bitcoin] sipa closed pull request #9561: Wake message handling thread when we receive a new block (master...2017-01-wakeup-on-new-block) https://github.com/bitcoin/bitcoin/pull/9561
< sipa>
BlueMatt: sorry, need sleep first
< BlueMatt>
I still vote we push back freeze a day or two, given today was a holiday in the us and many folks werent working all weekend
< BlueMatt>
:p
< BlueMatt>
and so I can shamelessly get folks to review #9535 :p
< bitcoin-git>
[bitcoin] fanquake closed pull request #9263: release notes: Explicitly mention the removal of free transactions, and do not commit to removal of priority in any given release (master...relnotes_freetxn) https://github.com/bitcoin/bitcoin/pull/9263
< morcos>
luke-jr: what is the point of doing the release notes on a wiki, if you are going to just merge changes that were explicitly NACK'ed when those same changs were proposed in a PR
< morcos>
all that is going to do is lead to a childish edit war and make it so none of us can use the wiki for release notes
< morcos>
if you know you have a controversial change you want to make you shouldln't just make it and hope no one notices
< morcos>
this is not a proper way of working with other people
< BlueMatt>
jonasschnelli: hey
< BlueMatt>
how much longer will you be around?
< jonasschnelli>
BlueMatt: 1-2h at least
< jonasschnelli>
hi
< BlueMatt>
ok, will circle back around after breakfast and hopefully we can finish this
< jonasschnelli>
Yes. I guess and hope I fixed most/all of you points... tell me, if there is more
< BlueMatt>
i think it should be good if you hit all the stuff i commented on, i gave it a pretty decent review the first go-around, but it needs another one when I'm awake post-breakfast
< jonasschnelli>
Yes. I think your review was very valuable...
< jonasschnelli>
We should not rush that change... but hurry up for 0.14. :)=
< BlueMatt>
heh, yea, I recommended pushing back yesterday because I'm confident we can get this one in
< jonasschnelli>
Yes. I mean if we find something after the freeze, there is time to fix it before the 0.14 release...
< BlueMatt>
yup
< sipa>
do we want to be a mentoring org for GSOC?
< BlueMatt>
does anyone feel like they have time?
< BlueMatt>
i mean in theory, maybe, but....
< sipa>
yeah...
< sipa>
i feel like we should make time for such projects
< sipa>
but easier said than done
< BlueMatt>
yea
< BlueMatt>
obviously agreed
< BlueMatt>
jonasschnelli: any chance we can s/(used for change outputs, only appears if HD is enabled otherwise there is no need for internal keys)/(used for change outputs, only appears if HD split is enabled, otherwise external keys are used)/
< BlueMatt>
and make the corresponding change the the logic
< BlueMatt>
should make the docs wayyy clearer
< jonasschnelli>
BlueMatt: Okay. Makese sense I guess.
< jonasschnelli>
I wonder if a simple "are there more bytes?" check would be more appropriate at this place
< jonasschnelli>
On the other hand, a failed deserialization at this point shoud always flag the CKeyPool item fInternal=false
< BlueMatt>
jonasschnelli: ok, the only remaining concern i have is performance - there are a bunch of loops over disk reads introduced for folks who upgrade
< jonasschnelli>
Yes. This could be optimised...
< jonasschnelli>
I expect BDB to be sort of fast...
< BlueMatt>
I think you may be surprised
< BlueMatt>
but if its an issue its an easy fix
< instagibbs>
that can be fixed post-freeze, yeS?
< jonasschnelli>
Yes. CKeyPool should have been fixed long time ago...
< BlueMatt>
instagibbs: if it turns out to be a major regression, I'd say yes
< jonasschnelli>
BlueMatt: I can test it with a 100k keypool wallet... and compare
< jonasschnelli>
Oh.. lets better start with 50k...
< jonasschnelli>
or with 5k,... CKey.Verify() is slow..
< BlueMatt>
heh
< sipa>
CKey.Verify() can verify 10000 keys per second
< sipa>
the slow part is the sync to bdb after every keypool change
< jonasschnelli>
okay.. I see..
< jonasschnelli>
BlueMatt: With a 5k wallet, its slightly slower on my Core i7 2.9 GHZ..
< jonasschnelli>
getnewaddress: real0m0.219s
< BlueMatt>
vs?
< jonasschnelli>
real0m0.104s
< jonasschnelli>
(0.13)
< jonasschnelli>
BlueMatt: I think it's acceptable but we should work on a fix/better-memory map
< BlueMatt>
I think its fine for now, I'll let phantomcircuit fix it when he complains
< bitcoin-git>
[bitcoin] TheBlueMatt closed pull request #9419: Stop Using cs_main for CNodeState/State() (master...2016-12-nodestate) https://github.com/bitcoin/bitcoin/pull/9419
< jonasschnelli>
BlueMatt: oh. Keep in mind, that 0.13er wallet has only 5k keys while the 0.14er wallet has 200% key (=10k)
< jonasschnelli>
So.. then the number look even better
< morph>
hi
< luke-jr>
morcos: YOUR changes were NACK'd.
< morcos>
luke-jr: i didn't have any release note changes except 9531 which were merged with only ACK's. But I'm not going to waste everyone's time arguing with you about this. I find you very difficult to work with, and so I will just avoid it where possible.
< morcos>
I've made my view point on the release notes clear.. If you want to go around changing things behind people's backs, I'm not going to take on the task of trying to stop you. But you should be aware your actions reflect poorly on all of us.
< luke-jr>
morcos: trying to use Core as a vehicle for political agendas is what reflects poorly.
< sipa>
luke-jr: stop it
< sipa>
there have been more than enough arguments
< sipa>
sorry, people disagree with you
< sipa>
accept it
< luke-jr>
disagreement means we should agree to disagree by having an option, not that you should just get your way and force your opinion on others
< luke-jr>
this would all be over if you would just leave it be.
< BlueMatt>
this may be true for some thing, but we dont generally support options that are used by +/- one person, especially if they have really big costs to doing so
< BlueMatt>
s
< luke-jr>
BlueMatt: the cost to leaving the code in place until it gets in someone's de facto way is literally zero.
< BlueMatt>
no it is not, it has not been, and it will not be
< luke-jr>
I already suggested a compromise that it can be removed at the first occasion of it being a burden to someone who wants to change something there.
< BlueMatt>
we're waayyyyy past that point
< sipa>
luke-jr: the discussion about it has dragged on for ages, and just the disagreement about it is draining people who want to work on the project
< sipa>
it is clear that thid code is useless at this point
< BlueMatt>
the compromise is that if you want to support it, you can add additional rpcs which give you access to the appropriate data so you can call prioritizetransaction (ie the fee-bumping command)
< luke-jr>
then morcos went and made that inflammatory comment
< sipa>
we're doing everyone a favor by just getting past it
< luke-jr>
sipa: so stop creating disagreement for no reason
< sipa>
luke-jr: i can say the same
< BlueMatt>
the disagreement is that only you disagree, at this point
< BlueMatt>
hell, even other miners disagree with you
< luke-jr>
except I'm not the one who keeps bringing it up and forcing the issue.
< sipa>
luke-jr: you're turning a piece of outdated logic from a mouse into an elephant, and i'm tired of having this discussion
< BlueMatt>
because everyone wants to just remove the damn code already
< luke-jr>
BlueMatt: some miners perhaps. others don't.
< BlueMatt>
oh? who other than you?
< luke-jr>
nobody is forcing the ones who don't want it to use it.
< BlueMatt>
(and dont say wizkid)
< luke-jr>
wizkid057 doesn't count now?
< BlueMatt>
the amount of work that has gone into maintaining this feature just for eligius is insane
< BlueMatt>
we maintain no other feature for one person/group
< BlueMatt>
if that were our policy we'd still have tonal numbering in bitcoin core
< luke-jr>
"still" implying we ever did
< BlueMatt>
well, ok, we'd have
< BlueMatt>
s/still//
< BlueMatt>
same thin
< BlueMatt>
g
< luke-jr>
not to mention there's no evidence it's only used by one person.
< sipa>
there is evidence that only a small percentage of the hash rate uses it (look at the feerate of blocks in transactions... it's clearly feerate sorted, except for a small percentage of txn at the beginning of a very occasional block)
< sipa>
furthermore, it is completely useless as a spam prevention mechanism without wallets targetting it
< sipa>
i'm done arguing about this
< sipa>
we'll remove priority mining in 0.15, as far as i'm concerned
< luke-jr>
that's simply not true at all. priority works without any targetting.
< sipa>
come on
< sipa>
a perfectly legitimate wallet's transaction would be treated completely arbitrarily under it
< sipa>
all it would accomplish is favor a few people with very old coi s
< sipa>
until they start spending recent change
< luke-jr>
*any* age/value weighs more than unconfirmed TXOs.
< sipa>
if you choose to keep arguing this, i will choose to ignore you
< luke-jr>
fine, remove priority. be a jerk and do it before there's even a slightly useful purpose in doing so. but then it's time to stop pretending Core is a politically-neutral reference implementation, since it's de facto being used as a vehicle to force network policy. all you're doing is proving the haters right.
< BlueMatt>
luke-jr: if there were no way to implement it in the supported apis, maybe you'd have a point...but right now it is /trivial/ to implement priority exactly how you like it in a little python script which loops over the mempool and does manual prioritization
< BlueMatt>
luke-jr: if you can honestly tell me you've implemented that, and it wasnt performant enough or there was some other issue preventing it from being practical, maybe I'd have some sympathy, but until then....
< luke-jr>
BlueMatt: so just busyloop over RPC mempool listing, maintain an entire copy of the mempool state outside bitcoind, and act as a proxy for explicit miner prioritisation? I can't even imagine a way to do this effectively; it's certainly a heck of a lot more work, less reliable, and far less efficient.
< BlueMatt>
less efficient? yes, lot more work? I dont believe so...go implement it and prove me wrong
< BlueMatt>
i dont think it needs state, just iterate over and clear prioritization as you go
< BlueMatt>
high_prio = stack(); for transaction in getrawmempool(): if calculate_priority(tx) is highest in stack: place in stack; for transaction not in stack but previously prioritzed: deprioritize; for transaction in stack: prioritize appropriately
< BlueMatt>
if thats more than 100 lines of python I'd be surprised
< BlueMatt>
and if its too ineffecient for you, please add an rpc which provides all the info needed for calculate_priority so that its super effecient
< BlueMatt>
I'd be more than happy to review that
< luke-jr>
to have such an RPC means having about half of the priority code stay in Core
< luke-jr>
because it comes from the input txouts
< BlueMatt>
dont think so? such an rpc could easily only provide things like foreach input: provide txout info
< BlueMatt>
and that is way more general than priority
< BlueMatt>
it allows folks to implement other crazy rules
< BlueMatt>
like "spends from address x"
< BlueMatt>
(well, admittedly you can kinda already do that by seeing the pubkey)
< luke-jr>
but fetching txouts is where the inefficiency came from?
< BlueMatt>
fetching txouts is the tightest loop in the above pseudocode
< BlueMatt>
so I'd think so
< BlueMatt>
wumpus: yo
< BlueMatt>
you still around?
< luke-jr>
BlueMatt: let's continue that topic (or not) post-freeze. either way, it will be easier to maintain priority in Knots than external. and in the meantime, if we haven't frozen yet, I should get back to reviewing stuff
< BlueMatt>
its unclear to me if we've frozen or not :/
< luke-jr>
well, finishing up review certainly can't hurt either way
< BlueMatt>
I mean either we froze, or we have 4 outstnding prs that either go in or dont (todayish, preferably)
< BlueMatt>
yea, fair point
< bitcoin-git>
[bitcoin] jnewbery opened pull request #9569: Setting -blocksonly sets -maxmempool to zero. (master...blocksonlynomempoolsharing) https://github.com/bitcoin/bitcoin/pull/9569
< wumpus>
BlueMatt: no, we've not frozen yet, that's when we split off the 0.14 branch
< BlueMatt>
wumpus: oh, I'm confused now...the release schedule says feature freeze today, branch in feb
< wumpus>
BlueMatt: I'm confused apaprently, yes branch is before first -rc, it has been the other way around but it means a lot of more backporting so this is okay
< wumpus>
the only other freeze there is today is "translation string freeze", but that makes little sense without feature freeze
< wumpus>
let's move the feature freeze (and translation freeze) to thursday
< BlueMatt>
my pending list is #8456, #9294 # 9499 and #9535...of those I believe only 9499 has string changes
< gribble>
https://github.com/bitcoin/bitcoin/issues/9377 | fundrawtransaction: Keep change-output keys by default, make it optional by jonasschnelli · Pull Request #9377 · bitcoin/bitcoin · GitHub
< BlueMatt>
yea, I'm not too worried about that one, will review soon
< BlueMatt>
I also plan to do a helgrind run again this or next week, whenever we decide which net pulls will make it, which is gonna result in a nontrivial number of "Convert X to std::atomic" commits
< BlueMatt>
the walletnotify one (#9479 / $9371) is also gonna take some work
< BlueMatt>
but i havent dug into that one as of yet
< BlueMatt>
morcos and y'all seem on it
< BlueMatt>
ok, so string freeze is whenever #9499 and #9461 make it, otherwise when feature freeze happens
< gribble>
https://github.com/bitcoin/bitcoin/issues/9499 | Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction by TheBlueMatt · Pull Request #9499 · bitcoin/bitcoin · GitHub
< gribble>
https://github.com/bitcoin/bitcoin/issues/9461 | [Qt] Improve progress display during headers-sync and peer-finding by jonasschnelli · Pull Request #9461 · bitcoin/bitcoin · GitHub
< BlueMatt>
jonasschnelli: can you fix the outstanding comments on #9461?
< gribble>
https://github.com/bitcoin/bitcoin/issues/9461 | [Qt] Improve progress display during headers-sync and peer-finding by jonasschnelli · Pull Request #9461 · bitcoin/bitcoin · GitHub
< BlueMatt>
'tf
< BlueMatt>
so I'm pretty sure, right now, if you receive a coinbase payout in a block, turn off your node, and then restart without checkblocks, your payout will not display
< BlueMatt>
because no NotifyTransactionChanged will ever fire to the gui
< BlueMatt>
i mean super edge-case-y but still
< sipa>
is that a recent regression?
< BlueMatt>
looks super old
< BlueMatt>
but didnt check
< BlueMatt>
restarting gui will fix it, though, afaict
< bitcoin-git>
[bitcoin] TheBlueMatt opened pull request #9570: Block Wallet RPCs until wallet is synced to our current chain (master...2017-01-fix-wallet-rpc-stale) https://github.com/bitcoin/bitcoin/pull/9570
< gmaxwell>
I thought the fundraw change change made it in already. darn.