< GitHub37>
bitcoin/0.12 a36d79b Alex Morcos: Add sane fallback for fee estimation...
< GitHub195>
[bitcoin] MarcoFalke opened pull request #7332: [wallet] Clarify rpc help message with regard to rounding (master...Mf1601-docAmount) https://github.com/bitcoin/bitcoin/pull/7332
< GitHub104>
bitcoin/master 3a9dfe9 paveljanik: Fix typo, wrong information in gettxout help text.
< GitHub104>
bitcoin/master 2cd004b Wladimir J. van der Laan: Merge pull request #7326...
< GitHub32>
[bitcoin] laanwj closed pull request #7326: [Trivial] Fix typo, wrong information in gettxout help text (master...patch-15) https://github.com/bitcoin/bitcoin/pull/7326
< GitHub158>
bitcoin/0.12 fa4ba40 MarcoFalke: Expand section "Wallet transaction fees" & fix format and typos
< GitHub158>
bitcoin/0.12 fa0a391 MarcoFalke: Add Replace-by-fee to release-notes
< morcos>
wumpus: thanks for taking a look at 7312. i liked the hack of reusing hashBlock b/c that meant whenever hashBlock was already being set/unset the abandon status would be kept in sync. but perhas that was not the best way to create a special value which flagged it?
<@wumpus>
CMerkleTx just doesn't look like the place for it
<@wumpus>
conceptually
<@wumpus>
why not a field on CWallet instead?
<@wumpus>
also it lacks documentation; so this field is 'the last transaction marked abandoned'
<@wumpus>
?
< morcos>
yes i'll definitely add documentation
< morcos>
the field is just a special flag
<@wumpus>
yes, but it has nothing to do with MerkleTx - it's a wallet internal detail
< morcos>
blockHash is the hash of the block that the tx is confirmed iin
<@wumpus>
I'm not sure it should be global static something
< morcos>
in the even blockHash.isNull that means it is not confirmed
< morcos>
recently we made it so that if the nIndex is -1
< morcos>
then hashBlock is now the hash of the conflicting tx
< morcos>
all that stuff is in CMerkle
< morcos>
So I was kind of overloading that hashBlock one more time to say, if it's 0 or 1 that means its not in a block and not conflicted, but 0 means regular and 1 means abandoned
<@wumpus>
but conceptually it is wallet state, right, not global state? say if there are multiple CWallet objects, would they interfere with each other?
< morcos>
its not a variable, its just a constant
<@wumpus>
oh!
< morcos>
hashBlock is the variable
<@wumpus>
let me recheck
< morcos>
i'm not arguing its right btw
<@wumpus>
right, I missed the const, oops
<@wumpus>
the field is hashBlock which already exists and is not a static
< morcos>
i was trying to encapsulate the constant somewhere that made sense, but perhaps it would have been better to just make it outside the class, or to just recreate it when we're checking it? i'm not sure
< morcos>
correct
< morcos>
so if you are ok with perpetuating hte hack, i'm happy to move the constant whereever you think it should live, and comment it
<@wumpus>
I think for most constants we use UPPER_CASE
<@wumpus>
that's what made me confused... the field looks the same as the constant
<@wumpus>
e.g. call it ABANDON_HASH
< morcos>
you're right. i'm a copy paste coder and was copying the constant "one" from signature checking.. : ) but yes happy to
< morcos>
but should i move it? it does seem like this is kind of a slippery slope from thread safety perspective right? of course these things are all heavily locked now but if in the future you wanted multiple threads accessing different wallet tx's it seems silly to worry about synchronizing around a constnat
<@wumpus>
but there is no sync problem if it is a const
<@wumpus>
disregard that, I was confused, I thought it was a global value that changed
< morcos>
i don't know, one of you guys scared me with that article on thread safety
<@wumpus>
no need to move it, just make clear it is a constant marker value
< morcos>
ok, well like i said, happy to capitalize, and comment, but please let me know if you think that is ok .. ok great. thanks
< morcos>
wumpus: ok pushed. thanks for waiting for this. i think we'll be very happy we have it around if there is another major spam attack or other surge of transactions. If not, no one has to use it.
< GitHub22>
bitcoin/0.12 2a3161b Wladimir J. van der Laan: Merge pull request #7332
<@wumpus>
ok, that should be all, going to tag 0.12.0rc1 in a minute
<@wumpus>
* [new tag] v0.12.0rc1 -> v0.12.0rc1
< MarcoFalke>
woohoo!
< JackH>
yay we have 0.12 incoming :)
< btcdrak>
w00t
< btcdrak>
wumpus: pm
< morcos>
wumpus, your release candidate isn't compiling for me, looking into it
<@wumpus>
linux build is still in progress here
< morcos>
ah backport failure of my pr, damn it, sorry i should have tested it in 0.12
< morcos>
will get you a fix
<@wumpus>
ok, probably better to revert it, this was a bad idea last minute
< morcos>
wumpus: the problem was simple, it just used a function that hadn't been backported
<@wumpus>
going to remove the rc1 tag again
< morcos>
give me 10 mins before making any decisions
<@wumpus>
- [deleted] v0.12.0rc1 sorry for the noise...
< morcos>
probably the right answer is #7154 should be backported to. it will be better UI for 0.12 and should fix the problem
<@wumpus>
nah, I'm not going to do any last minute UI backports
<@wumpus>
is there any quick fix?
< morcos>
yes, i can just inline the contents of the inmempool function
< morcos>
you don't want to do a UI backport because of translations?
<@wumpus>
yes
< morcos>
ok, i'll just take the part of his commit in that pull that creates InMempool
< morcos>
thats cleanest
<@wumpus>
makes sense, thanks
<@wumpus>
can do the GUI part for 0.12.1, but it's too late for 0.12.0
< morcos>
wumpus: ok i did it. but show me how you want to give it to you. my branch v12rebase adds the commit in the right place. and my branch forgetstuck12 has the commit just on top if you want to do the rebasing yourself.
< morcos>
in the v12rebase, i tried to comment the commit appropriately. i didn't really put detail in the commit message on the forgetstuck12 branch
< morcos>
i'm not sure how it is easiest for you to merge
< morcos>
also i tested
< MarcoFalke>
Did you test the v12rebase?
< morcos>
MarcoFalke: yes
< MarcoFalke>
Then just merge that into bitcoin/bitcoin
< morcos>
its not a commit its a branch with the ordering of commits
< MarcoFalke>
a06a8b488896dd83a320fff10d48300ca01e9ba4 looks fine, I mean.
< GitHub149>
bitcoin/0.12 a06a8b4 Jonas Schnelli: add InMempool() function
< GitHub149>
bitcoin/0.12 3d5cf69 Wladimir J. van der Laan: Merge pull request #7333...
< Luke-Jr>
at the very least, 7149 is still needed to fix a bug in 0.12 (although note even with it, there are still bugs I am working on fixing/testing)
< Luke-Jr>
could wait for rc2 I suppose
< MarcoFalke>
There is indeed more bugs
< MarcoFalke>
We need to leave some for later
< Luke-Jr>
yeah, I guess we should get rc1 out ASAP so users can begin *finding* [more] bugs :p
< morcos>
MarcoFalke: more bugs?
< MarcoFalke>
I will create pulls in the coming days
< MarcoFalke>
Some of those exist for a long time
< MarcoFalke>
It's not too critical
< MarcoFalke>
+1 Luke-Jr :)
<@wumpus>
of course there is always more work to be done, but we can't keep postponing 0.12 forever
< MarcoFalke>
Sure, there are 351 other bugs open at this time, I think
< MarcoFalke>
But nothing holding back a rc1
<@wumpus>
* [new tag] v0.12.0rc1 -> v0.12.0rc1 ok,. this time for real
< michagogo>
Release notes talk about BIP65 locking in in future tense...
<@wumpus>
hm, that probably shouldn't be there at al
< Luke-Jr>
XD
< michagogo>
(Also, is github being weird or did v0.11.2 have a handful of version numbers that stayed at 0.10?)
< GitHub52>
bitcoin/0.12 5771b71 Wladimir J. van der Laan: doc: Remove BIP65 mention from release notes...
< michagogo>
Luke-Jr: GitHub's v0.11.2...0.12 comparison shows a bunch of lines replacing 0.10 or 0.10.99 with 0.12
<@wumpus>
no, 0.11.x doesn't have any 0.10 versions, I don't think we've ever had a wrong version number slip into a (non-rc) release
< michagogo>
Hm, GitHub must be messing up somehow then.
< MarcoFalke>
GitHub diff is not git diff
< michagogo>
I'm also seeing changes that I *know* were in 0.11
< michagogo>
MarcoFalke: I know that, but how do you mean exactly?
< MarcoFalke>
GitHub compare will go back to the common branch point to compare, I think.
< MarcoFalke>
You can't compare branches in GitHub when they conflict each otehr
< michagogo>
Meh, really?
< michagogo>
:-/
< michagogo>
It certainty doesn't make that obvious.
< morcos>
wumpus: ok it passed extended rpc tests for me now. woo hoo!
< GitHub193>
[bitcoin] MarcoFalke opened pull request #7334: [qt] coincontrol workaround is still needed in qt5.4 (fixed in qt5.5) (master...Mf1601-qt55Workaround) https://github.com/bitcoin/bitcoin/pull/7334
< cfields>
MarcoFalke: that looks like it should be a runtime check?
< MarcoFalke>
jonas didn't like that
< cfields>
hmm
< MarcoFalke>
by runtime check you mean compile time check?
<@wumpus>
couldn't you have opened that last week?
< Luke-Jr>
wumpus: I didn't know they were missing until now :p
<@wumpus>
I don't agree on porting most of those, let's leave that for important fixes
< Luke-Jr>
O.o
< Luke-Jr>
so delay these for 0.12.1, or just close it and forget?
<@wumpus>
I mean something like "rpc: remove cs_main lock from `createrawtransaction`" I've done on 0.13 on purpose, it's not something that needs to be backported
<@wumpus>
it's a small improvement, but also has a small risk, not really user visible in any case
< Luke-Jr>
or I could remove that one, if it's uniquely undesired.
<@wumpus>
and certainly things that involve translation changes are too late
< Luke-Jr>
even if the current strings are outright wrong?
<@wumpus>
the Rename OP_NOP2 to OP_CHECKLOCKTIMEVERIFY makes sense ofc
<@wumpus>
String freeze was *december 1* for 0.12.0, more than a month ago
<@wumpus>
not something you'd propose inserting between two rcs
<@wumpus>
for 0.12.1 it's ok
<@wumpus>
but that will have to wait, then
< Luke-Jr>
yes, I know that. but usually I would consider "this string is wrong" to trump a string freeze (and the PR it's a backport of didn't even exist back then) :p
< Luke-Jr>
ok, so remove the cs_main lock thing, and delay the string change?
<@wumpus>
if there is a strong that is so wrong it could cause serious harm to users of course an exception can be made but that's shouldn't be the assumption.
< Luke-Jr>
k, removed those two then
< Luke-Jr>
wumpus: added the release-notes combine
<@wumpus>
it doesn't seem it will still make 0.12.0
<@wumpus>
(fine with adding 0.12 milestone, but that will mean 0.12.1 at first)
< Luke-Jr>
just added unit tests, so not much more I can do at this point to help it along
<@wumpus>
why does it include some commits from morcos last year? is it a continuation of one of his pulls? your description is very short for relatively many changes
<@wumpus>
and it's open since dec 1, don't know why no one has looked at it yet
<@wumpus>
maybe discuss at the meeting tomrrow
< morcos>
wumpus: 7149 is based off my code to keep the priority calculation the same as it was in 0.11 but to calculate it dynamically so it was not expensive to GetPriority
< Luke-Jr>
yes, the bugs go back a long ways, but morcos introduced them into the mining code with the CNB rewrite and also wrote those commits to fix it
< morcos>
I think the consensus other than Luke-Jr was the modified priority calculation that exists in 0.12/master right now is good enough.
< morcos>
This has the effect of not increasing prioirty of txs for inputs that were not already in the block chain at the time the tx was accepted to the mempool.
< Luke-Jr>
"good enough" isn't a strong reason not to fix bugs..
< morcos>
So it will still age and increase in priority but by a smaller amount. There may be some inaccuracy around reorgs to.
< morcos>
It only affects how priority is calculated for mining
< morcos>
If we were going to keep priority as it exists now for the long term. I might think it was worth adding this code. But it is somewhat complicated and I believe it would be better not to add this complication if we're not fully committed to maintaining priority
< morcos>
It was discussed at length previously
< Luke-Jr>
fixing this also means we can have working tests for it
< Luke-Jr>
(and it isn't that complicated)
< Luke-Jr>
in any case, I do plan to maintain priority