< GitHub110>
bitcoin/master 5e10922 Luke Dashjr: Combine common error strings for different options so translations can be shared and reused
< GitHub110>
bitcoin/master de9e5ea Wladimir J. van der Laan: Merge pull request #7257...
< GitHub163>
[bitcoin] laanwj closed pull request #7257: Combine common error strings for different options so translations can be shared and reused (master...reduce_opt_ts) https://github.com/bitcoin/bitcoin/pull/7257
< GitHub99>
bitcoin/0.12 ff9b610 MarcoFalke: [wallet] Add regression test for vValue sort order...
< jtimon>
wumpus I'm still on vacation and kind of waiting on #7091 before continuing with the "document-with-words-and-pictures" I promised to some people, but...what is the "right time for refactors and moveonlies"? I really don't want to miss it for the consensus encapsulation moveonly again
< jtimon>
it was supposed to be after major version forks, right? or is it after the major released is actually done (to avoid interfering with backports)?
< morcos>
jonasschnelli: yes i think its that same thing we discussed in november. sipa gave me an idea for how to hack it in, i should have something in a few hours.
< jonasschnelli>
morcos: Okay. Perfect.
< GitHub122>
[bitcoin] jtimon closed pull request #7238: Blocksize: Some small preparations for a blocksize hardfork (master...6526-6625-remainings-0.13.99) https://github.com/bitcoin/bitcoin/pull/7238
< jtimon>
sipa see my question to wumpus, do you know when is the right time for refactors/moveonlies? after 0.12 has been forked or after 0.12 has been finally released?
< MarcoFalke>
jtimon, I think the best time is between 0.12.0-final and 0.13-branchoff. You don't want to conflict any backports, so better wait until 0.12.0 (or even until 0.12.1 is out).
< MarcoFalke>
If the refactor is not controversial, it should also be backported, imo.
< jtimon>
MarcoFalke: I agree on both accounts (refactors before forking and backport disruptive refactors)
< wumpus>
yeah we could do some move-only things for 0.13 soon, together with c++11-ication I do expect some code churn anyway
< wumpus>
that specific commit looks sensible to me, good to have that out of main.cpp
< jtimon>
great, so when is the "refactor window"? now or after releasing 0.12 ?
< wumpus>
now - after releasing 0.12, doesn't matter, 0.12 branch is forked off so 0.13/master can have some more intensive/risky changes
< jtimon>
I see, I thought maybe we didn't wanteed to do that before releasing 0.12 for easier backports
< wumpus>
nah, that's true, but backporting over move-only isn't that involved
< wumpus>
if possible I want to tag 0.12.0rc1 tomorrow so 0.12 release isn't far away
< jtimon>
well, my plan was to do the moveonly after #7287
< jtimon>
(to avoid needing FormatStateMessage() from consensus/consensus.cpp temporarily)
< wumpus>
I mean you could say the same for the segwit stuff, moving consensus related files around gives work there, but otoh rebasing that over moveonly isn't going to be rocket science
< jtimon>
ok, thank you
< jtimon>
I'll open it soon (among other things, as an alternative to CodeShark's #6774 )
< CodeShark>
#6747
< CodeShark>
no moving consensus stuff yet...just the soft fork activation mechanism
< CodeShark>
making it easier to deploy testnets with specific bips applied
< CodeShark>
and leaving thr activation mechansm fully open ended
< Luke-Jr>
wumpus: btw, in case you missed it the other day: I don't plan to do any further rebasing, so just let me know when PRs are ready for merging and I can do the appropriate merge commit
< btcdrak>
wumpus: just to confirm, dev meeting tonight right? 7pm UTC?
< morcos>
jonasschnelli: sipa: wumpus: any preference for name of forgettransaction? forget, abandon, respend, archive, discard, ditch, vacate ? i think i vote for abandon
< jtimon>
CodeShark: and you really don't need to touch the old softfork stuff to implement bip9, that's actually one of the complains rusty had about your implementation (he didn't in his)
< MarcoFalke>
or discard
< jgarzik>
Luke-Jr, +100
< morcos>
Luke-Jr: jgarzik: I'm much newer at this than most, but someone will need to tell me how I'm supposed to review a PR with merge conflicts and if I just review it off the branch its based off of, how do I know the merge conflicts will be solved properly.
< jgarzik>
morcos, "merge conflicts solved problem" is by definition a moving target. Review as checked out from submitted branch. When it's ready to merge, attend those issues at that time.
< morcos>
I think it makes much more sense for most patches to be rebased, maybe only not for big long standing branches, of which there are not too many, but then will need to be reviewed twice in my opinion.
< jgarzik>
morcos, It's a subjective judgement of the distance between submitted branch and currently. For example: mempool janitor PR is submitted. Later, some other major mempool changes are merged into master. It is fair to ask the author to merge with master.
< jtimon>
morcos: I'm sure Luke-Jr would be happy to let you rebase his commits if you think it's very important
< jgarzik>
luke-jr's linked Linus post did a good job of explaining
< jgarzik>
*fair to ask the author to merge with master on his branch, making his branch current.
< jgarzik>
(to be clear)
< jgarzik>
The focus is on the submitted branch
< morcos>
yes i read linus' arguments and i think they applied to a substantially different work flow than we have on a much more mature code base, but whatever, i defer to wumpus
< jtimon>
I think it's also fair for an author to get tired of rebasing
< MarcoFalke>
Especially if there is major merge conflicts, there are different ways to solve them. Shouldn't the merge commit be part of the PR then?
< jgarzik>
MarcoFalke, yes, correct
< jgarzik>
That's the ideal workflow
< MarcoFalke>
jtimon, I don't think there is a difference in merge/rebase in regard to solving conflicts
< Luke-Jr>
the difference is whether it breaks decentralised development
< jtimon>
MarcoFalke: unless I'm missing something, Luke-Jris just tired of rebasing and wants to do it only one last time before Bitcoin Core is ready to accept his patch
< MarcoFalke>
No
< jgarzik>
jtimon: read what luke-jr just said
< jtimon>
jgarzik: MarcoFalke Luke-Jr yep, it looks like I'm missing something
< jtimon>
mhmm, I have rebased cfields' commits in the past (although after doing it for several months with no success I gave up)
< wumpus>
morcos: I think I prefer 'archivetransaction'
< wumpus>
morcos: it sounds the least destructive
< wumpus>
btcdrak: yes re: meeting
< jgarzik>
+1 wumpus RE archivetx
< wumpus>
Luke-Jr: that's up to you, I think rebasing PRs can be useful, I like a linear-ish history, but if you can do an alternative with merges that accomplishes the same that's the same to me...
< morcos>
wumpus: ok i'm happy to change to whatever people prefer but the reason i didn't go with archive originally was it sounded something like record keeping to me, and didn't indicate that it would have an economic effect.
< morcos>
of course i guess it doesn't actually have an economic effect, it just changes your guess as to what the economic effect of the tx is so you can double spend
< wumpus>
morcos: I'd interpret archive as in 'remove from the active set and stash in the archive somewhere', but sure there's more ways to interpret it, the most important is that the RPC is documented
< MarcoFalke>
What is "the archive somewhere"?
< btcdrak>
morcos: rusty kindly tested #6564 with both #6312 and #7184 variants so we should make a decision about closing one of those soon.
< wumpus>
I do think archive can potentially give confusion, e.g. people will think they can archive old transactions as well
< wumpus>
MarcoFalke: mark it as non-active, but it stays stored in the wallet, hidden
< morcos>
wumpus: so to be clear the way im thinking about this the tx is still going to appear in your list of txs and everythign and it will still show 0 confirmations, but it will no longer effect the spentness of the txin prevouts
< btcdrak>
morcos: I can add your diff commit to #6312 or we can close it in favour of #7184 which is cleaner imo
< wumpus>
morcos: ok
< wumpus>
morcos: I do think it should no longer be sorted at the top, at least
< morcos>
wumpus: ha. ok something else for me to learn about the wallet then
< wumpus>
but hiding it completely (unless some flag) is fine with me too... it's effectively out of the picture
< wumpus>
showing it is just confusing, and sorting it at the top gives priority to it
< morcos>
wumpus: i actually think that is a bit dangerous. it could still be included in a block or coudl prevent a respend from relaying if its in other nodes mempools
< wumpus>
sure it could be included in a block, but in that case it must be shown again like any transaction that's in a block
< morcos>
wumpus: once we have RBF implemented, i think we'd require that you pass the RBF requirements to the extent you can determine them to respend it
< morcos>
wumpus: so you may want to be aware that you have this potential double spend out there instead of literally forgetting about it
< wumpus>
still hiding it when it reached you through a block would indeed be very wrong
< wumpus>
but is it always 'out there'? what if it was never broadcasted at all?
< morcos>
wumpus: well i'll probably do the PR first without looking at sorting, just to get something for people to look at anyway
< MarcoFalke>
wumpus, archive means "something is final, I can put it in a box and it won't change" but that is not the case here.
< morcos>
wumpus: ah! i've been thinking about that. we are narrowing the number of cases where thats possible i think. although i guess if you have broadcast off
< wumpus>
a feature to hide transactions is extremely frequently requested, people get annoyed at all the stale transactions that drift to the top
< wumpus>
MarcoFalke: well, feel free to propose a better name then
< wumpus>
I won't partake in bikeshedding
< wumpus>
+English is not my native language
< MarcoFalke>
mine neither
< MarcoFalke>
I think the pull itself is more important
< wumpus>
absolutely
< morcos>
ok i'm going with abandon until i here otherwise, back to work
< wumpus>
morcos: ok
< morcos>
s/here/hear/ and English is my only language
< * Luke-Jr>
regrets English being his only fluent language
< morcos>
btcdrak: I have a pretty strong preference for #7184, but i really want to hear what sipa thinks before making a decision.
< morcos>
Luke-Jr: same
< midnightmagic>
Luke-Jr: learn mandarin :)
< GitHub180>
[bitcoin] jtimon opened pull request #7310: MOVEONLY: Move consensus functions out of main (master...consensus-moveonly-0.13.99) https://github.com/bitcoin/bitcoin/pull/7310
< GitHub59>
[bitcoin] jtimon opened pull request #7311: MOVEONLY: non-consensus: from pow to chain: (master...consensus-pow-moveonly-0.13.99) https://github.com/bitcoin/bitcoin/pull/7311
< jtimon>
wumpus ^^ the moveonly PRs
< jtimon>
regarding #6597 I think sipa is wrong, but as said that's not libconsensus related
< Yoghur114_2>
this may be too trivial to ask here, but in building the segwit fork - I now appear to require the libevent-dev package, which I didn't have
< Yoghur114_2>
any clue as to why this is the first time ./configure fails while I've built the project many times since that added test?
< Yoghur114_2>
on ubuntu 14.04 that is
< wumpus>
for building master (or anything derived from it) you need libevent-dev it replaces boost asio
< phantomcircuit>
Yoghur114_2, libevent-dev has been required since the rpc stuff was changed
< Yoghur114_2>
what rpc stuff changed? that might explain why my interface has borked
< sipa>
Yoghur114_2: 0.12 and master use libevent for RPC handling jnstead of boost::asio
< Yoghur114_2>
aah ok, I get it now - thanks guys
< Yoghur114_2>
I shall now continue to discover I am too poorly versed at interpreting the actual code :)
< phantomcircuit>
Yoghur114_2, please point out what/where you are confused, it's helpful to know where there should be more comments
< Yoghur114_2>
it isn't the comments, it's me: I'm a Java developer by trade
< Yoghur114_2>
:)
< Yoghur114_2>
so i'll be having a hard time understanding anything even if it's documented to all hell
< Yoghur114_2>
phantomcircuit: ok I've identified the confusion more precisely (for the record): it was a git thing. What I didn't understand was why a 0.11.2 build didn't trigger this test: https://github.com/bitcoin/bitcoin/blob/master/configure.ac#L717 - which I thought was added january 20th of 2015 according to the history, however, that commit was part of a PR that only got merged into master last september - after master branched off from .11
< sipa>
Yoghur114_2: we started working on the 0.12 branch nearly a year ago
< sipa>
0.11.2 was released from the stable branch
< sipa>
0.12 is now about to mature into a release
< Luke-Jr>
cfields: /usr/include/boost/thread/future_error_code.hpp:36:53: error: ‘enum_type’ is not a member of ‘boost::future_errc’
< Luke-Jr>
this is latest stable boost on Gentoo
< cfields>
Luke-Jr: thanks, will have a look after meeting
< cfields>
Luke-Jr: i can't find any reference to future_error_code.hpp. what boost version?
< cfields>
(at that path, anyway)
< Luke-Jr>
cfields: 1.56.0-r1
< alexpi____>
hji
< cfields>
Luke-Jr: got it, thanks
< cfields>
Luke-Jr: can you paste the whole nasty error please?
< Luke-Jr>
cfields: that leaves me at /usr/include/boost/filesystem/operations.hpp:492: undefined reference to `boost::filesystem::detail::copy_file(boost::filesystem::path const&, boost::filesystem::path const&, boost::filesystem::copy_option, boost::system::error_code*)'
< Luke-Jr>
which is bound to be an ABI issue
< cfields>
hmm, weird. that's the one that this should've fixed
< cfields>
Luke-Jr: did you autogen/reconfigure ?
< Luke-Jr>
not since patching the header
< cfields>
nm, shouldn't matter anyway. the right things are obviously defined
< cfields>
well, one thing at a time...
< Luke-Jr>
complete rebuild from autogen doesn't have a difference
< cfields>
i guess i should take that patch upstream
< jgarzik>
git clean -dfx
< jgarzik>
when changing deps
< Luke-Jr>
jgarzik: that would destroy so much data of mine..
< Luke-Jr>
jgarzik: also, this is using system deps
< cfields>
though in our case, 1.60 is the only version affected :\
< cfields>
oh wait
< cfields>
yea, 1.56. sorry.
< Luke-Jr>
fwiw, the actual symbol is boost::filesystem::detail::copy_file(boost::filesystem::path const&, boost::filesystem::path const&, boost::filesystem::copy_option>>>>>>::enum_type<<<<<<<<<<, boost::system::error_code*)
< cfields>
Luke-Jr: can you see which object it comes from?
< Luke-Jr>
?
< cfields>
hmm, i thought nm would tell you that. sec.
< Luke-Jr>
cfields: why are you defining FORCE_BOOST_EMULATED_SCOPED_ENUMS in bitcoin-config.h rather than BOOST_NO_SCOPED_ENUMS and BOOST_NO_CXX11_SCOPED_ENUMS ?
< cfields>
Luke-Jr: because i prefer to only define local things in that header when possible, so that usage is more explicit
< Luke-Jr>
this is caused by some other header pulling in the relevant header before the latter get defined
< Luke-Jr>
it's not a local thing, though, it's a boost configuration
< cfields>
Luke-Jr: i'll admit, i went back and forth on it in this case. looks like you may be right
< cfields>
Luke-Jr: i don't see a path that would cause it to be included somewhere else first, though
< Luke-Jr>
cfields: can't assume boost doesn't do it internally..
< cfields>
Luke-Jr: simple test, comment out the filesystem.hpp include in walletdb.cpp. fails to build as expected for me.
< cfields>
Luke-Jr: ok, great. i think you're probably right about the global define.
< Luke-Jr>
wallet/db.h includes #include <boost/filesystem/path.hpp>
< Luke-Jr>
this is included from walletdb.h
< Luke-Jr>
so we also need to include bitcoin-config.h before that
< cfields>
yup, got it. i grepped for filesystem.hpp and missed it.
< Luke-Jr>
note that test_bitcoin is hanging
< Luke-Jr>
nm, it finally finished
< cfields>
any chance the header patch isn't needed now that things aren't defined two ways?
< Luke-Jr>
hmm
< * Luke-Jr>
testing
< Luke-Jr>
cfields: seems to work!
< cfields>
weird
< cfields>
probably some voodoo wrt template instantiation
< cfields>
either way, glad that's all it takes :)
< cfields>
you going to pr it or want me to?
< Luke-Jr>
go ahead, I just started a bisect
< cfields>
ok
< cfields>
thanks for tracking it all down!
< Luke-Jr>
cfields: np, thanks for doing all the hard part
< morcos>
sipa: wumpus: this has nothing to do with my abandon code, but i discovered in testing.
< morcos>
what should happen if you have a tx in your mempool that becomes conflicted b/c of a double spend in a block (so its other inputs are now freed to respend, after #7306)
< morcos>
and then that block gets reorged out and the double spend goes away?
< morcos>
(i should say i _think_ it has nothing to do with my abandon code)
< Luke-Jr>
cfields: do you have access to bitcoincore.org?