< wumpus> "graphics acceleration at 9.81 m/s^2" hahahah genius
< sipa> wumpus: based on an old linux joke "the best windows accelerator is one that works at 9.81 m/s^2"
< sipa> also, why are you awake?
< sipa> i have an excuse, it's 6:30 pm here.
< wumpus> I have no excuse
< wumpus> it's eh 03:35 here :D
< wumpus> the freebsd issue on stack exchange is weird, my gut feeling is that it has something to do with locale settings but I saw nothing of the kind at least with 0.13
< achow101> Did the release email go out?
< wumpus> no
< achow101> ok. Shouldn't you be asleep?
< wumpus> could announce the rc on mailng list, but an rc does not warrant a 'release email'
< wumpus> this is only a test, not a release yet
< achow101> i meant like the email saying that the rc exists.
< wumpus> yea, will send one, but I thought peopel here may be interested in it as well and it's easier to paste an url
< achow101> oh ok
< wumpus> good time to sneak out a rc2 mail though while the US is distracted with their clown show :)
< NicolasDorier> passing flag CLEAN_STACK to libconsensus seems to make crash the whole process.
< NicolasDorier> now crash
< NicolasDorier> when using libconsensus
< NicolasDorier> trying to deep what it going on
< NicolasDorier> dig
< wumpus> strange
< wumpus> on master or 0.13.1?
< jl2012> does libconsensus has CLEANSTACK flag?
< NicolasDorier> downloaded libconsensus from https://bitcoin.org/bin/bitcoin-core-0.13.1/test.rc2/
< jl2012> i thought it only has consensus critical flags?
< NicolasDorier> jl2012: was working before, and the flags in libconsensus are not reallyused, the flags is coverted into a SCRIPT_VERIFY flag without check
< wumpus> no, it doesn't
< wumpus> that doesn't crashing is a valid outcome ofcourse
< jl2012> using CLEANSTACK without WITNESS would fail assertion?
< NicolasDorier> the flags is converted into SCRIPT_VERIFY
< NicolasDorier> not really using the libconsensus flags at all
< wumpus> can you post a traceback?
< NicolasDorier> I'm checking if it is not my binding code between C# and C++ which is at fault... But it would be strange as the previous version was working nice. wumpus : how can I do a traceback on windows ? I may be able to use windbg, but not sure if the output of it would be of big use
< wumpus> don't ask me how to do debugging on windows :)
< jl2012> NicolasDorier: what would happen if you add a WITNESS flag to the test?
< NicolasDorier> trying that
< NicolasDorier> one sec
< NicolasDorier> jl2012: with the flag, no crash
< jl2012> so that's the reason
< NicolasDorier> yep, can reproduce crash if P2SH|CLEANSTACK not with P2SH|WITNESS|CLEANSTACK... why did the bitcoin C++ test worked fine ?
< jl2012> sipa: we should add WITNESS flag to all tests with CLEANSTACK
< jl2012> NicolasDorier: I'm also wondering the same
< jl2012> maybe it failed before the assertion at L1505 is triggered
< NicolasDorier> yeah, and why did it not failed before when using libconsensus OO
< NicolasDorier> checking
< jl2012> to fix it, search all tests with CLEANSTACK. They should all have P2SH and WITNESS
< jl2012> also, all tests with WITNESS must have P2SH
< NicolasDorier> jl2012: I'm step by stepping my C# code, and I confirm that this test should pass through those asserts
< jl2012> with all 3 flags?
< NicolasDorier> with 2 flags. So the assert is the cause of the crash, but why it did not crashed on C++ tests really bother me
< wumpus> yes, that is really strange
< NicolasDorier> jl2012: also I don't understand why WITNESS should be set at all cost if CLEANSTACK is set
< NicolasDorier> I think adding WITNESS to all CLEANSTACK test is the wrong way
< NicolasDorier> CLEANSTACK | P2SH should not fail
< wumpus> well the P2SH rationale is explained
< wumpus> "Disallow CLEANSTACK without P2SH, as otherwise a switch CLEANSTACK->P2SH+CLEANSTACK would be possible, which is not a softfork (and P2SH should be one)."
< wumpus> don't know about WITNESS
< NicolasDorier> yes for P2SH not a problem
< wumpus> NicolasDorier: it's adding the flags in the tes https://github.com/bitcoin/bitcoin/blob/master/src/test/script_tests.cpp#L163
< jl2012> Witness script is violation of cleanstack
< NicolasDorier> wumpus: oh thanks
< wumpus> that explains why the C++ tests do pass, updating the tests would have been clearer.
< NicolasDorier> I still think that CLEANSTACK | P2SH should not crash
< NicolasDorier> even without this test change
< jl2012> It's only a problem if we had a cleanstack softfork before witness
< NicolasDorier> jl2012: What I mean is that segwit should not break something that was working before, if it is not activated... Users of libconsensus were passing P2SH | CLEANSTACK
< NicolasDorier> now, if they just update the library, their code will crash
< NicolasDorier> even if witness is not activated
< jl2012> Then we need to remove that assert
< NicolasDorier> I think we should yes
< wumpus> but CLEANSTACK isn't even part of the libconsensus API
< wumpus> you've been relying on undocumented behavior, that can change from release to release
< NicolasDorier> mh good point
< wumpus> I don't think crashing with an assert is any good way of error handling , but still
< wumpus> the asserts are there to prevent bugs, e.g. "this combination of flags should not be used", and through libconsensus it shouldn't even be possible to pass them. Possibly libconsensus should do better input checking on the flag bits.
< wumpus> I'm not against removing the asserts if it doesn't actually protect against something dangerous, but the rationale in the comment seems pretty grave
< NicolasDorier> I would prefer removing the assert so we don't break code that were using this undoc API if not dangerous. But if we keep it, a comment about it would be very useful
< wumpus> one of the arguments against doing libconsensus in the first place was this - as the API is set in stone, it makes us less flexible to change things. Doubly so if even undocumented behavior is considered.
< wumpus> guaranteeing one bug-for-bug compatible interface (the consensus itself) ought to be enough of a challenge
< NicolasDorier> I agree. But the alternative (making an alt implementation) is even more dangerous I think. One way to fix the problem would be to add a "flags = flags & CONSENSUS_FLAGS"
< wumpus> but why do you really need this?
< NicolasDorier> if the user passed non consensus flags (like I already did), then the function would just strip the non consensus bits instead of crashing
< wumpus> yes I think the call should fail (with an error code, not an assertion) when unrecognized bits are passed in
< NicolasDorier> we can do that, I'm still a bit worried as I think lots of people passed SCRIPT_VERIFY_STANDARD to this call.
< wumpus> just ANDing them out can be risky too. There is no guarantee that the bits will aways map one to one to internal script verification bits
< NicolasDorier> yes I understand. So might be good to do it now before libconsensus is too much use
< wumpus> may warrant mentioniong in the release notes then
< NicolasDorier> is not
< wumpus> ugh, even our own tests make use of this undocumented API
< GitHub0> [bitcoin] laanwj opened pull request #8976: libconsensus: Add input validation of flags (master...2016_10_bitcoinconsensus_input_checking) https://github.com/bitcoin/bitcoin/pull/8976
< wumpus> NicolasDorier: I'm not entirely convinced of #8976 yet, at the least because it can't pass the combinations required to pass our own tests right now, but I hope it will start a discussion about what we really want the behavior to be here
< NicolasDorier> wumpus: We can also expose the whole SCRIPT_VERIFY to libconsensus (it is ok, as I doubt this will ever change)
< NicolasDorier> I'm kind of neutral, I think we can break the API and reject wrong flags, libconsensus is not too much used yet for doing that.
< wumpus> yes, though I remember back in the day when libconsensus API was drafted there were reasons not to do so
< wumpus> I think the point is that only consensus flags are offered, the rest is kind of arbitrary
< wumpus> but uhm, we'd have to change our approach to testing at least then
< NicolasDorier> yeah, it also make sense. The base of the problem being that the script evaluator has policy AND consensus code :D
< wumpus> I hope that will be separated in the future
< wumpus> and maybe add a libbitcoin_policy for the other things :-)
< GitHub91> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/c5875773561c...f2d705629b51
< GitHub91> bitcoin/master cb08fdb Pedro Branco: Add importmulti rpc call
< GitHub91> bitcoin/master 215caba Pedro Branco: Add consistency check to RPC call importmulti
< GitHub91> bitcoin/master f2d7056 Wladimir J. van der Laan: Merge #7551: Add importmulti RPC call...
< GitHub161> [bitcoin] laanwj closed pull request #7551: Add importmulti RPC call (master...feature/rpc-import-multi) https://github.com/bitcoin/bitcoin/pull/7551
< GitHub117> [bitcoin] jonasschnelli opened pull request #8977: [Wallet] Refactor wallet/init interaction (Reaccept wtx, flush thread) (master...2016/10/fix_wallet_init) https://github.com/bitcoin/bitcoin/pull/8977
< GitHub145> [bitcoin] jonasschnelli closed pull request #8723: [Wallet] Add support for flexible BIP32/HD keypath-scheme (master...2016/09/hd_flex) https://github.com/bitcoin/bitcoin/pull/8723
< GitHub86> [bitcoin] jonasschnelli reopened pull request #8723: [Wallet] Add support for flexible BIP32/HD keypath-scheme (master...2016/09/hd_flex) https://github.com/bitcoin/bitcoin/pull/8723
< jonasschnelli> Anyone interested to test the mempool statistics collector? This would be a basic step for GUI mempool stats: https://github.com/bitcoin/bitcoin/pull/8501
< wumpus> jonasschnelli: maybe release binaries
< wumpus> usually that's more effective to get GUI stuff tested
< jonasschnelli> wumpus: 8501 is non GUI RPC only... the GUI diff is large because of the flexible core stats collector.
< wumpus> ohh
< jonasschnelli> I don't want to scarify the flexible design to keep the GUI diff low. :)
< wumpus> bah I'll retest #8959
< jonasschnelli> Parts of the statics collector could probably be reused for sipas idea of evicting misbehave score
< jonasschnelli> 8959 is strange...
< jonasschnelli> Maybe a Qt bug
< wumpus> seems to work for me
< jonasschnelli> Could be a QT-OSX bug
< jonasschnelli> i'll test again
< wumpus> either that or macosx's triangles are y-axis-reversed :-)
< jonasschnelli> wumpus: looks like. :) I guess is a Qt bug
< jonasschnelli> *it's
< wumpus> turning the pyramid upside down
< jonasschnelli> we could add something in platformstyles... *duck*
< wumpus> do we have sorting triangles anywhere else?
< jonasschnelli> yes. TX table
< wumpus> are they reversed too?
< jonasschnelli> let me check
< jonasschnelli> Correct there for me
< jonasschnelli> wumpus: can you check on Ubuntu?
< wumpus> yes
< jonasschnelli> Hmm... so its a bug in our codebase
< wumpus> huh. The tx one on ubuntu is: up-pointing pyramid is descending, down-pointing pyramid is ascending
< * wumpus> confused
< * paveljanik> was always confused...
< jonasschnelli> So at least the "bug" behavior is identical.
< wumpus> tried with balance. same for date. up=most recent date first, down=oldest date first
< jonasschnelli> IMO its an upstream Qt bug
< wumpus> I don't know anymore, I've lost perspective how it is supposed to be. Should try some other qt app maybe
< wumpus> reverted my ACK
< jonasschnelli> If its the opposite direction on OSX its certainly an upstream issue. I don't think platforms have a different sort-pyramid-direction... :)
< wumpus> in principle the validity of #8959 is based on one thing: does order == Qt::DescendingOrder really sort descending?
< wumpus> we should inspect the sorted list insome other way
< wumpus> along with the input to the operator
< wumpus> jonasschnelli: ok, so the original code was correct
< wumpus> the arrow behavior is strange, but should not be corrected by changing the sorting code
< jonasschnelli> wumpus: look like, but with the original code, i get a strange initial state
< jonasschnelli> First click results in only changing the arrow. :)
< wumpus> it looks like initially the thing is not sorted at all
< jonasschnelli> dam those little GUI glitches
< wumpus> it only ends up in that sorting function *after* clicking the column the first time
< jonasschnelli> Ah. That could be.. so it takes the default arrow direction which could be different
< michagogo> I wonder if my attempt to make a gitian-building vbox appliance will work, or crash and burn.
< jonasschnelli> michagogo: isn't the idea that everyone should setup it's own VM (security)?
< wumpus> well if everyone would start using michagogo's image that would be a problem
< michagogo> jonasschnelli: yeah, but I had someone ask me for it... and that way I'd also find out if it's actually possible to set up
< jonasschnelli> michagogo: Yes. I think its generally a good idea and it might lead to better docs as well.. :)
< michagogo> Since so many people seem to have trouble with it, while I have a working setup...
< jonasschnelli> though, wumpus did create an awesome gitian doc
< michagogo> People say it doesn't work :-(
< michagogo> I've wondered a couple times if it's just hard to do/understand, or if something changed and it's actually impossible to set up from scratch these days
< jonasschnelli> Yes. Maybe we should provide a non LXC version
< michagogo> i.e. if the only reason it works for me is that I already have the environment set up
< michagogo> Maybe I'll turn on VBox's video recording feature
< jonasschnelli> heh... I guess same here. :)
< jonasschnelli> michagogo: you could turn your working system into an appliance (including your bitcoin private keys)
< michagogo> jonasschnelli: actually, someone asked for that first
< michagogo> But yeah, not gonna do that
< jonasschnelli> yes. better not
< michagogo> No Bitcoin private keys in there, but gpg, probably ssh, and also token for github, chrome, etc.
< michagogo> (I use the VM whenever I want to do/try something in Linux, not just for gitian)
< wumpus> < jonasschnelli> Yes. Maybe we should provide a non LXC version <- I think there should be only one guide, LXC or not, it's hard enough to keep one up to date
< Victorsueca> morning
< wumpus> morning
< btcdrak> wumpus: achow101 script work great with --kvm
< wumpus> the problem has always been that kvm doesn't work in some VMs, due to nesting, but I have no idea whether this is still the case
< wumpus> if not a relevant problem anymore we could just switch the guide to KVM
< Victorsueca> windows update force-restarted my computer last night while I was sleeping :/
< rabidus_> new spyware installed
< luke-jr> wee, master no longer builds
< luke-jr> guess importmulti wasn't ready :<
< luke-jr> wallet/rpcdump.cpp:811:54: error: no match for ‘operator!=’ (operand types are ‘CTxDestination {aka boost::variant<CNoDestination, CKeyID, CScriptID>}’ and ‘CTxDestination {aka boost::variant<CNoDestination, CKeyID, CScriptID>}’)
< michagogo> wumpus: I assume it's still an issue... nested virtualization is hard
< michagogo> But I do think anyone who's able should use KVM... AFAIK it tends to work much, much better
< michagogo> Hm, should I try making this with the Ubuntu Server ISO?
< luke-jr> does this actually compile for anyone? it looks like boost::variant *intentionally* omits operator!=
< wumpus> does what actually compile for anyone?
< luke-jr> wumpus: importmulti or master
< wumpus> yes - it passes travis and builds here locally
< luke-jr> :/
< luke-jr> how is CTxDestination.Get() != CTxDestination.Get() working?
< Victorsueca> Will try compiling master here
< wumpus> the intention would be to compare whether the two destinations are the same
< wumpus> I think the importmulti code could be cleaned up quite a bit
< luke-jr> hm, looks like boost added it in some newer version
< wumpus> looks like the variant comparison is mostly used for "consistency checks"
< GitHub145> [bitcoin] luke-jr opened pull request #8980: RPC: importmulti: Avoid using boost::variant::operator!=, which is only in newer boost versions (master...compat_importmulti_oldboost) https://github.com/bitcoin/bitcoin/pull/8980
< luke-jr> !(a==b) works
< gribble> Error: "(a==b)" is not a valid command.
< wumpus> that's crazy
< sipa> gribble disagrees
< luke-jr> heh
< Victorsueca> lol
< wumpus> hahahah
< luke-jr> why does C++ make it possible to have a==b && a!=b ? :p
< wumpus> I think most/all languages with overloaded operators allow that
< luke-jr> I knew Perl did, but I figured that was because it was Perl ;x
< wumpus> same with > versus <= and < versus >=. I think there's even legitimate cases for that, e.g. NaNs
< GitHub49> [bitcoin] laanwj pushed 1 new commit to 0.13: https://github.com/bitcoin/bitcoin/commit/5f6b312e51dadaf40ea68c0f85bbb4e51fa987f1
< GitHub49> bitcoin/0.13 5f6b312 Wladimir J. van der Laan: doc: Add missing credit to release notes...
< sipa> wumpus: from 0.13 release notes: "This is a new majir versiob release"
< sipa> *major
< luke-jr> there's a lot of stuff in there that could use work, but I assume we're merging from the blog post before it's final anyway?
< GitHub87> [bitcoin] paveljanik opened pull request #8981: Wshadow: Do not shadow argument with a local variable (master...20161020_Wshadow_rpcdump) https://github.com/bitcoin/bitcoin/pull/8981
< wumpus> yes, the 0.13 release notes definitely need some updating still
< wumpus> "This is a minor release, bringing various improvements and fixes, as well as activation parameters for the SegWit and NULLDUMMY soft-forks." <- I think michagogo wrote this, would be better I think?
< Victorsueca> NULLDUMMY?
< Victorsueca> wut
< sipa> Victorsueca: bip147
< GitHub48> [bitcoin] s-matthew-english opened pull request #8982: Eliminating Inconsistencies in Textual Output (master...patch-6) https://github.com/bitcoin/bitcoin/pull/8982
< michagogo> Hm, actually
< sipa> i wonder if release notes shouldn't be written and tracked in a separate repository
< michagogo> Do we call it two soft-forks?
< michagogo> Maybe it's more accurate to drop the s
< wumpus> michagogo: no, it should be called one, I think mentioning only segwit would be fine
< michagogo> since it's one softfork, with two components
< Victorsueca> maybe it would be better if we named bip147 something other than nulldummy
< wumpus> sipa: could be, though the release notes are pretty much on the same release cycle as bitcoin core so it'd not make much of a difference
< wumpus> e.g. the release notes need to be final at the same time the release is final
< wumpus> otherwise they can't be included with the release, they can't be uploaded to bitcoin.org and other places, etc
< wumpus> posted to the mailing lists
< wumpus> could do the editing of the release notes in another repo then include it before final, sure...
< wumpus> or heck ,even google docs
< wumpus> that's easier for collaborative editing I guess
< sipa> well it could remove the issue of retroactive changes
< sipa> and the ambiguity of whether to edit doc/release-notes in 0.13 vs doc/release-notes-0.13 in master
< wumpus> retroacive changes are always a problem
< wumpus> that's not an ambiguity: before final, it can be edited in 0.13 branch, after the release it is copied to master and can only be edited there
< wumpus> and is cleaned out on the 0.13 branch for the next release from there
< wumpus> but I really think retroactive changes should be avoided if possible - at final release the notes will be copied to tons of different places, editing on master is not very effective
< sipa> fair enough, it is less ambiguous than i thought
< sipa> but it's still confusing
< wumpus> I'm not against moving the release notes to another repository, but it leaves the same problem if we want to include them with the release
< wumpus> at some point it needs to be checked in as doc/release-notes.md
< wumpus> in principle only the state at the final tag matters, the filecould be non-existent before and after that
< Victorsueca> what about this: do the docs on a separate repo, when it's release time then clone to bitcoin
< wumpus> I don't think that's any less confusing though
< wumpus> maybe the current way of working should just be documented better
< wumpus> sometimes the answer to something that is confusing is to describe/document it better, not always to immediately change it, may well make it more confusing to other people, esp those used to how things were done
< wumpus> and to be honest I've never found anyone confused by this. Current 0.13 release notes? are on the 0.13 branch
< wumpus> current 0.14 release notes are on the master branch
< wumpus> those are for the next release on either branch
< wumpus> the advantage of doing it this way is that pulls can update the release notes atomically with the thing they introduce
< wumpus> not that anyone bothers
< wumpus> what we could do is rename the generic release-notes.md to release-notes-0.13.0.md on the 0.13 branch and release-notes-0.14.0.md on master, and update as appropriate. This will remove confusion for which version they are, at least...
< wumpus> should make sure that the file still gets included in the tarballs though
< wumpus> (and all the other distributions)
< GitHub133> [bitcoin] laanwj pushed 1 new commit to 0.13: https://github.com/bitcoin/bitcoin/commit/c9a5baddeef3d8721a7c71acf070f92a3d8d43a3
< GitHub133> bitcoin/0.13 c9a5bad Wladimir J. van der Laan: doc: Update blurb in release notes...
< luke-jr> ☺ deterministic git assembly of Knots' branch in 30 seconds :P
< Victorsueca> i'm currently testing if master can be built successfully
< luke-jr> Victorsueca: it will likely depend on your boost version
< Victorsueca> lastest probably, I installed it 2 days ago
< sipa> installed how?
< Victorsueca> with apt-get
< sipa> that way you get the version from your distro's repository
< sipa> that's very very unlikely to be the latest
< Victorsueca> it's ubuntu
< sipa> likely not even the latest at the time the distribution was released
< Victorsueca> do unit tests mess with the default data directory or they create some temp dir to do the tests?
< wumpus> they create temp directories
< wumpus> if they touch the data directory that would be a bug
< Victorsueca> so it's _safe_ to run the scripts in the qa folder
< wumpus> those are not the unit tests, but functional tests, but the same holds
< Victorsueca> wumpus: would be helpful if I run any of those and paste the outputs?
< wumpus> only if they fail, I guess
< wumpus> you can run all of them with qa/pull-tester/rpc-tests.py
< Victorsueca> wumpus: is it currently more needed to test master or 0.13.1rc2?
< sipa> unless you have an unusual setup, running rpc or unit tests won't teach us anything new
< sipa> using bitcoind/-qt in your own ways may, however (though always be cautious about things that could lose you money)
< Victorsueca> AFAIK there are few devs using windows
< sipa> ah, yes
< Victorsueca> I use Windows x64
< sipa> i thought you were on ubuntylu
< sipa> *ubuntu
< Victorsueca> I use ubuntu to cross-compile the builds
< sipa> do the rpc tests even work on windows?
< Victorsueca> not sure, worth trying I guess
< luke-jr> I kinda doubt they would
< Victorsueca> looks like my boost version was right, master built successfully
< luke-jr> wrong Python version
< Victorsueca> need python 3?
< luke-jr> yes
< michagogo> I might have successfully set up a machine for Gitian building
< michagogo> It was pretty much just as easy as I thought it might be
< michagogo> I even migrated over my apt-cacher-ng cache from my other machine
< achow101> michagogo: but does it actually build the binaries properly?
< michagogo> achow101: That's my next test
< michagogo> I made the base VM and that seems to work as far as sanity-checking
< michagogo> Snapshotting it now, and then I'll test an actual build
< michagogo> Gitian is actually broken atm (make-base-vm tries to copy a file that doesn't exist)
< michagogo> But there's already a PR that fixes it, so I just cherry-picked that in
< achow101> yeah, i noticed that.
< michagogo> And I turned on VBox's video capture option, so I have a video of everything I did
< michagogo> Hm. Depends was downloading clang+llvm-3.7.1-x86_64-linux-gnu-ubuntu-14.04.tar.xz from llvm.org, and it failed
< michagogo> then it tried to fall back to bitcoincore.org and got a 4040
< michagogo> -0
< michagogo> Oversight?
< timothy> hi, can you please consider adding man pages to linux tar too?
< timothy> instead of git repository only
< Victorsueca> running it on a ubuntu enviroment brings a similar error https://softnet.homenet.org/zerobin/?e1589c342242901a#RUf6mKsZzrb+JBznhaaAraDxb57s0DayDGRm1JgU5Bo=
< Victorsueca> so it's not windows fault
< Victorsueca> any ideas?
< michagogo> achow101: Seems to be working so far
< michagogo> Running an actual gitian build, it's built linux depends up until native_protobuf
< michagogo> Working on boost atm
< michagogo> OpenSSL...
< michagogo> Okay, if anyone's interested in taking a look, it should go up soon at https://1drv.ms/f/s!AvCguGMVwWzLgxJPeXdvaQVJ2WJq
< michagogo> It's being compressed atm, once it's done it'll start uploading
< GitHub167> [bitcoin] rebroad opened pull request #8983: Show block height and size when received (master...ShowBlockHeightAndSizeWhenReceived) https://github.com/bitcoin/bitcoin/pull/8983
< achow101_> michagogo: how did you get lxc to work? Did you have to change anything in gitian's scripts?
< BlueMatt> why do i see so many nodes failing version handshake?
< BlueMatt> on 13.1
< Lightsword> BlueMatt, only on 0.13.1 and not 0.13.0?
< BlueMatt> dunno, didnt try 13
< BlueMatt> still, ProcessMessage FAILED on version messages is new to me
< BlueMatt> did xt/classic/unlimited break something?
< Victorsueca> BlueMatt: not getting those errors here
< Lightsword> BlueMatt, got an example of one it fails against?
< sipa> BlueMatt: nExpectedServices could cause this
< BlueMatt> no, because it otherwise prints the peer's ip messages after version processing works
< BlueMatt> :/
< sipa> when a node does not have the services in its version message listed that we expected it to have, we disconnect
< BlueMatt> sipa: no, ProcessMessages(version, 102 bytes) FAILED peer=10
< BlueMatt> oh, yea, that might be it
< sipa> yes, ProcessMessages returns false in that case
< BlueMatt> since it returns false
< BlueMatt> huh, funny, I've seen a bunch of those on fresh nodes
< Victorsueca> BlueMatt: maybe it's gmaxwell's aggressive witness peer search disconnection non-witness nodes to free connection slots
< Victorsueca> disconnecting*
< BlueMatt> no, it is almost surely the nServicesExpected & ~nServices check
< sipa> Victorsueca: no, that's for chossing which peers to connect to
< sipa> BlueMatt: to be fair, we probably had no idea about what services were circulating
< sipa> maybe some reachable nodes became pruned
< sipa> but their ips kept circulati g
< tulip> BlueMatt: I was seeing that a lot too.
< tulip> I didn't think to take a packet capture until after it calmed down sadly.
< BlueMatt> tulip: naa, see comments above, its pretty clear what it is
< tulip> BlueMatt: ok, needs more sensible error messages. there's a couple of those hanging around with peer connection.
< tulip> if you use a proxy the errors it prints are basically noise, but that's solvable by just switching to another tmux window :)
< Victor_sueca> damn, power outage
< Victorsueca> and router has +30 second ping :D
< BlueMatt> tulip: -debug=net will show it
< BlueMatt> that error message should probably not need debug=net
< achow101> michagogo: did you upload it yet? All I see is a .webm file which I can't play
< NicolasDorier> wumpus: I think we can sort of having a compromise where allowing to validate policy rules with libconsensus without using undoc flag with the following way:
< NicolasDorier> void* libconsensus_createValidationContext();
< NicolasDorier> void libconsensus_setConsensusFlags(void* context, int flags);
< NicolasDorier> void libconsensus_setPolicyValidation(void* context, bool value);
< NicolasDorier> bool libconsensus_verify(void* context, har *scriptPubKey, unsigned int scriptPubKeyLen, CAmount amount, unsigned char *txTo, unsigned int txToLen, unsigned int nIn, bitcoinconsensus_error* err);
< NicolasDorier> It would make things easier to extend in the future and prevent to rely on undoc flags for script policy validation
< NicolasDorier> so the whole undoc flags would be wrapped into the "PolicyValidation" boolean
< sipa> NicolasDorier: but libconsensus' purpose is not to test policy
< sipa> the same risks don't apply, you can use any implementation for that
< sipa> and exposing that functionality means we need to maintain it, which makes it impossible in the future to split off policy validation elsewhere
< NicolasDorier> sipa: mmhh that's unfortunate, for the script evaluator as a user of libconsensus, I'd like to delegate those rules to the dll if possible :(
< NicolasDorier> but well, I understand the point that you don't want the burden of maintaining it
< sipa> well maybe you can create a libbitcoinscript
< sipa> that does everything scripting, but isn't consensus
< sipa> imho it's a mistake that in our own code the policy script evaluator is mixed with consensus logic
< sipa> but that's a controversial opinion, i'm aware
< NicolasDorier> yeah, well, not so controversial I think there are way to remove from the script evaluator all code belonging to policy evaluation. I think the main barrier would be to carefully review such solution :p
< sipa> it would imply we have two separate script interpreter implementations
< sipa> one just for consensus, one for everything
< sipa> the first one would only need to be touched for consensus changes
< NicolasDorier> sipa: It can be possible to inject into EvalScript an abstract class "PolicyEvaluator" whose implementation check every policy rule. And put the implementation outside of consensus
< wumpus> I agree with sipa, I'm not against a libbitcoin_policy or such, but mixiing it with consensus is a bad idea
< sipa> NicolasDorier: i don't want "abstract" anything in consensus code, to the extent possible
< sipa> NicolasDorier: that makes it much harder to reason about all code interactions
< NicolasDorier> I don't think it such case it would be hard to reason: If you want to execute only Consensus rule, you would inject a NullPolicyEvaluator implementation with empty method implementation.
< NicolasDorier> I mean it would be the easiest way I see to separate consensus code from policy code
< NicolasDorier> in script evaluator
< wumpus> as long as the injection is only used to inject e.g. debugging or policy where the outcome is no longer important for consensus that'd be fairly true, it starts to be a nightmare once you inject consensus rules that way
< NicolasDorier> wumpus: yes, only for injecting things that are only part of policy
< sipa> NicolasDorier: but how complicated would such an injector need to be to implement something like lows, or nullfail, or csv?
< wumpus> (e.g. as long as injection points are a no-op when validating for consensus)
< sipa> NicolasDorier: if for every new policy you need to add new injection places, it's no better than what you started with
< wumpus> in any case I'm still not sure how to handle the tests in https://github.com/bitcoin/bitcoin/pull/8976 - should I skip tests that use flag combinations that are not in the API?
< NicolasDorier> mh good point. I would say at least for consensus it is easy to see that a broken policy rule can't break the execution of the script with consensus flags, as the implementation of such class in "consensus mode" would just be empty
< wumpus> (when testing libconsensus)
< sipa> wumpus: i'd say so
< wumpus> ok!
< sipa> define a value that is the or of all flags that are in consensus
< sipa> if any others are set, skip the test
< wumpus> yes, I added a bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL that is all the flags in libconsensus ORed
< wumpus> let's see if that leaves enough tests :)
< NicolasDorier> for info, the checks in https://github.com/bitcoin/bitcoin/pull/8976 are meant for 0.13.1 ? just to know if I update NBitcoin to deal with it now or later :D
< sipa> NicolasDorier: there are good uses for a very powerful script interoreter that does more than consensus too... for example a debugger or execution inspector, or something that supports signing some general class of signatures, maybe not specific to bitcoin transactions, ...
< wumpus> I don't think it's urgent enough for a last-minute change to 0.13.1
< sipa> NicolasDorier: there woukd be a burden to maintain changes in two places (when they affect consensus) but imho that is less than the work in review we save due to "this code does not ever need to be touched, unless consensus changes"
< wumpus> agree, there are good uses for an extended script interpreter, e.g. it's a common request to have something that visually shows script execution
< wumpus> and it's easier (esp. organization and review-wise) to do such things if they *don't* need to touch consensus code
< NicolasDorier> ah so what you would say is making a new "EvalScript" which is copied from the current one. And remove from the current EvalScript everything policy related ?
< wumpus> yes, I'd like that
< NicolasDorier> indeed would be cool
< NicolasDorier> doing such a thing would be easy to review as well.
< wumpus> the only thing it makes slightly more difficult is assuring that a policy matches a consensus change, when putting something in policy that becomes a softfork later
< sipa> NicolasDorier: not just EvalScript. Everything in script/*
< sipa> (most of it would go away in the consensus version, of course)
< NicolasDorier> mh interesting indeed.
< sipa> I once tried to simplify CScript::operator<< because it is very inconvenient to use
< sipa> turns out, changing it would have been a hard fork
< sipa> which was so scary that i really didn't want to touch consensus' CScript anymore
< wumpus> yes dodged a bullet there
< NicolasDorier> in C++ is there a way to split the implementation of a class in several file ?
< sipa> yes
< sipa> but you shouldn't :)
< wumpus> the implementation can be split into multiple files, the definition must be in one place, generally
< NicolasDorier> well, we could split CScript what belongs to consensus and what not
< sipa> see core_io.h with core_write.cpp and core_read.cpp for exmample
< sipa> NicolasDorier: die
< NicolasDorier> ahah is this so contentious :D
< sipa> NicolasDorier: now the users of your class' dependencies depend on _how_ they use the class
< wumpus> the way to achieve things like that is to define a bare data structure and have function act on it, like in the old days
< wumpus> you can put the functions everywhere
< sipa> indeed
< sipa> leveldb uses a struct with a {char* ptr; size_t size} in many places
< sipa> allowing processing routines to be independent from storage
< wumpus> we need a slice abstraction too
< sipa> slice, that's it
< wumpus> golang has that pretty much as central data representation :)
< NicolasDorier> well, you can represent a script with a char* and then just have two type PolicyScript and CScript would be better than bare, mainframe time code :p
< wumpus> nearly every time an array is passed, a slice is really passed, the slices keep a reference to the underlying array so it can't go away
< sipa> there wouldn't even be a CScriot
< NicolasDorier> you could wrap your char* by one of those
< sipa> just an EvalScript
< sipa> there could be a CScriptBuilder with the << operators etc for convenience
< wumpus> right - script generation isn't part of consensus
< sipa> exactly
< sipa> it would just be utikity
< NicolasDorier> wumpus: by "Slice" abstraction, you mean a structure {*array, index, count} ?
< wumpus> NicolasDorier: yes
< wumpus> I think in golang it's a kind of shared pointer. Not sure though how they do memory internally
< wumpus> in leveldb it's just a *
< wumpus> so they don't 'hold' the memory, they just reference it, which is slightly more error prone but maybe lower overhead
< wumpus> in any case for one-off methods like consensus calls the distinction doesn't matter
< wumpus> consensus has no state and will never hold on to your slices
< wumpus> OK OK the statelessness not true if you include the caches
< wumpus> *current* libconsensus has no state
< michagogo> achow101: I don't think I did more than what gitian's README says
< NicolasDorier> in any case, I'm sold for the script evaluator code duplication which is specific to consensus. Not yet for char* everywhere though :D
< michagogo> (not as a "this is a list of steps to follow blindly", but as a way to understand how the system works and how to set it up)
< wumpus> not sure what you mean there, script is just a char*,size effectively. It has no internal structure
< michagogo> And yeah, the ova isn't fully uploaded yet
< michagogo> OneDrive says it's uploaded 1.2 out of 3.6 GB
< NicolasDorier> wumpus: I know, just for the use of code like GetSigOpsCount(char* script) insead of script->GetSigOpsCount() But I may be spoiled kid with my C# :D
< michagogo> Unfortunately it looks like it doesn't show a speed or time estimate, unlike Dropbox...
< wumpus> on the C# size you could re-add all the syntactic sugar that you want, esp if you use it for non-consensus
< NicolasDorier> yeah with extension method :))
< wumpus> (e.g. if you *want* GetSigOpsCount implies you're doing policy)
< NicolasDorier> ah GetSigOpsCount I though it was part of consensus my bad
< michagogo> The Linux gitian build worked on the new appliance: https://www.irccloud.com/pastebin/RpH8KGPY/
< wumpus> hm you're right I think, though if possible that would be used internally by libconsensus and not exposed
< achow101> michagogo: great! I guess gitian doesn't hate you. I followed the gitian readme almost to the word (I used vmware instead of vbox, but I don't think that makes a difference) and it still didn't work for me
< michagogo> achow101: Hm, odd
< michagogo> You said you weren't able to play the webm?
< michagogo> VLC seems to be playing it without a problem...
< michagogo> Looks like Chrome is doing just fine as well
< achow101> it didn't work in whatever default video player ubuntu uses. I'll try vlc
< michagogo> It's about an hour long
< achow101> yeah, it works in vlc
< michagogo> Shows the entire process, from the moment I booted the fresh VM from the 14.04.5 ISO until the moment I shut it down to export
< michagogo> Trial, error, and all
< michagogo> (also with some pauses while I looked stuff up outside the VM...)
< achow101> why do you need apt-cacher-ng
< michagogo> achow101: because when you're installing the same packages every time you gbuild, it's easier not to have to download them every single time
< michagogo> And once you're already setting up and using it for gitian, may as well point apt at it and have it go through there
< michagogo> (so that when you e.g. upgrade a system package, the same package is available to the gitian container when it needs to upgrade)
< achow101> ok
< michagogo> Lauda: You may be interested in this too
< michagogo> In the meantime it's up to 1.3 GB...
< michagogo> Actually, just realized I can check Resource Monitor
< michagogo> Looks like OneDrive is currently uploading at about 250-300KB/s
< michagogo> achow101: Looks like it should show up in 2-3 hours
< michagogo> (assuming a steady rate)
< Lauda> thanks michagogo
< wumpus> luke-jr, Victorsueca: the RPC tests work on windows, but are currently disabled there in travis because of timeout issues
< jonasschnelli> sipa, available?
< sipa> jonasschnelli: yes
< cfields_> BlueMatt: ping. #8969 ready for review/testing?
< jonasschnelli> When notifying about the header tip, can we not just use pIndexBestHeader instead of retrieving from setBlockIndexCandidates? https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L3010
< Victorsueca> wumpus: I tried it and there's some error with subprocess.py https://softnet.homenet.org/zerobin/?586fe4c0934c7d9c#fMLN5XtScV/cdqkwjN6dTkLU17hjkL0iiNGW81x7aNw=
< jonasschnelli> sipa: NotifyHeaderTip is only used by the gui and IMO it should just reflect whats inside pindexBestHeader... not?
< Victorsueca> wumpus: a similar error occurs even on a ubuntu env https://softnet.homenet.org/zerobin/?6b8aa1b1b6e5aca7#MhP7eiv7h8L7mHC9LMAIk6ytlzsmY2Hv+3t7I7WnUtQ=
< sipa> jonasschnelli: remember thinking about that, and that we should indeed just switch to using pindexBestHeader
< jonasschnelli> sipa: Okay. Let me try a PR. I'd like to fix https://github.com/bitcoin/bitcoin/issues/8984 with it as well
< BlueMatt> cfields_: sure! go for it
< BlueMatt> cfields_: though maybe first do 8968 first, and review those separately
< BlueMatt> cfields_: 8969 should be pretty simple without 8968
< cfields_> BlueMatt: got it, thanks
< BlueMatt> sipa: got a chance to rebase 8515? I got sucked into making fibre support variable bw depending on the remote node
< BlueMatt> :/
< cfields_> BlueMatt: ok. getting ready to update my stale PRs and catch up with your work, sorry for the delay. Mining is a rabbit-hole.
< BlueMatt> and fighting with hosts about their shitty routing
< BlueMatt> cfields_: no rush, so is debugging the shitty, shitty world of chinese internet
< cfields_> heh
< sipa> BlueMatt: will do
< cfields_> BlueMatt: ah, if it weren't so last minute, i think i'd be in favor of backporting 8968 to 13.1. holding cs_main there is really scary, considering how far down that can go
< BlueMatt> cfields_: yea, we talked about this in milan...decided against it because it was late even then
< cfields_> BlueMatt: have you done any traces to see the worst-case effects of holding it in 0.13? maybe there are small tweaks we can do for 0.13 if there are known possible deadlocks?
< BlueMatt> cfields_: it should never cause deadlock since cs_main should always be the first lock, I believe
< BlueMatt> maybe sipa want to comment since he was the one claiming so in milan
< cfields_> BlueMatt: specifically, it's holding it while grabbing cs_vSend/cs_vRecvMsg that worry me
< cfields_> agh, brb.
< GitHub157> [bitcoin] jonasschnelli opened pull request #8985: Use pindexBestHeader instead of setBlockIndexCandidates for NotifyHeaderTip() (master...2016/10/fix_gui_overlay) https://github.com/bitcoin/bitcoin/pull/8985
< wumpus> cfields_: do you really think it's worth delaying 0.13.1 release for? or do you mean 'it should be included if we need to do rc3 anyway'?
< cfields_> wumpus: no, i don't think it's worth delaying. Unless something nasty manifests. It's just in the category of "feels icky and scary".
< wumpus> I agree
< Victorsueca> wumpus: is my error related to what you said or it's a different thing?
< wumpus> Victorsueca: I don't really know
< wumpus> what does "El sistema no puede encontrar el archivo especificado" mean?
< Victorsueca> The system couldn't find the specified file
< wumpus> ohh, no that's a different problem. You could try setting the environment variable BITCOIND to the location where bitcoind is
< wumpus> looks like it's simply notable to find the daemon
< sipa> s/notable/unable/ ?
< Victorsueca> thanks, will try
< Victorsueca> should I set it to the directory where bitcoind is or directly to the bitcoind.exe file?
< wumpus> the file
< Victorsueca> wumpus: done, exactly the same error
< Victorsueca> ohh wait, didn't restart the terminal...
< sipa> wallet/rpcdump.cpp:1020:13: warning: ‘nLowestTimestamp’ may be used uninitialized in this function [-Wmaybe-uninitialized] int64_t nLowestTimestamp;
< wumpus> Victorsueca: I don't get it then, you'll need to do more debugging what process it is trying to execute in the first place
< Victorsueca> well, the bash is on C:\UNIX\bitcoinalpha
< Victorsueca> which is where I cloned the master branch of the git repository
< sipa> Victorsueca: i don't think this channel is right for helping you debug these things
< Victorsueca> sipa: well, maybe something is wrong with the rpc tests in which case it should be fixed
< Victorsueca> first i'm trying to figure out if I did something wrong or the python script has some bug
< wumpus> "maybe". But you should first try to debug it locally, before making allegations like that
< wumpus> the QA tests are run by many people on many platforms
< achow101> Victorsueca: what's the problem again? I'll see if I can replicate
< Victorsueca> achow101: I posted the link above
< Victorsueca> wumpus: few on windows AFAIK
< wumpus> no, really ,they are being run on windows
< Victorsueca> hmmm ok
< Victorsueca> any instructions on how to dump extra debug info?
< wumpus> I think you need general python debugging tools
< wumpus> there's nothing bitcoin specific about not being able to execute the right executable
< Victorsueca> well, don't know if this is relevant but trying rpc-tests.py on a ubuntu enviroment brings out the same error
< Victorsueca> I guess that would discard being something windows-specific
< michagogo> achow101: .6 GB to go
< sipa> Victorsueca: rpc-tests.py is succesfully run for every pull request on travis... if it doesn't work for you, you likely have something missing in your setup
< wumpus> it's interesting how everyone on windows seems to have a problm debugging. The only people I've seen succesfully debug on windows are people reverse engineering or looking for exploits :p
< Victorsueca> sipa: yep, likely
< wumpus> even a backtrace seems to be problematic
< Victorsueca> wumpus: that's because windows is made to fail, M$ doesn't care because you already paid for the license :P
< wumpus> Victorsueca: well in the case of linux much likely no one paid for anything, it seems to be something with transparency of APIs
< Victorsueca> wumpus: exactly, if linux was such a huge crap as windows nobody would use or even care about contributing into it so it would be dead
< Victorsueca> in M$ they already have the money so they can use to to make even more crap software
< sipa> referring to microsoft M$ isn't particularly meaningful - every company tries to make money
< michagogo> Hm, just saw this morning's (last night's, really) wallops
< michagogo> I wonder if we're in scope
< sipa> michagogo: link?
< michagogo> (the full message was: 00:04:00 <nhandler> We would like to introduce you to the freenode Community Team (http://freenode.net/news/community). Please join us in #freenode-community and let us know how we can make freenode a more useful place for your group.)
< wumpus> sipa: looks like it may be a false positive, nLowestTimestamp is supposed to be only set and used in the case of fRescan
< achow101> Victorsueca: wumpus: replicated the problem, found the issue (I think)
< wumpus> it's true that everyone wants money for everything on the windows platform; no one is going to support a free debugger for windows, if you want proper debugging you probably have to buy some expensive development package
< achow101> It seems the the RPC_TESTS_DIR returns the path with unix style e.g /mnt/c/... and it isn't liking that
< wumpus> it's a valid way to do things, but that means targetting open source to windows is hard, ideally we should sell bitcoin core on windows instead of offer a free download :-)
< Victorsueca> achow101: maybe adding shell=True would solve it?
< wumpus> achow101: but he tried to override BITCOIND already
< wumpus> no, don't add shell=True please, that's a security disaster
< Victorsueca> lol yeah
< sipa> BlueMatt: remabsed 8515
< sipa> *rebased
< sipa> sometimes i fail to understand my own fingers.
< achow101> Victorsueca: it's because both of us built using WSL
< achow101> the problem is with the SRCDIR variable. If you go to qa/tests_config.py you'll see why
< Victorsueca> achow101: no such file
< achow101> sorry, qa/pull-tester/test_config.py
< Victorsueca> ahh lol
< Victorsueca> variables are in unix-like paths
< achow101> but fixing there's another error after fixing that
< Victorsueca> achow101: which one?
< achow101> %1 is not a valid Win32 application
< achow101> that happens when I run it again
< Victorsueca> i'm still getting the same error tho
< GitHub0> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/f2d705629b51...0e228557f239
< GitHub0> bitcoin/master 7942d31 Luke Dashjr: RPC: importmulti: Avoid using boost::variant::operator!=, which is only in newer boost versions
< GitHub0> bitcoin/master 0e22855 Wladimir J. van der Laan: Merge #8980: RPC: importmulti: Avoid using boost::variant::operator!=, which is only in newer boost versions...
< achow101> check your path and make sure it is correct. Use forward slashes instead of backslashes
< GitHub21> [bitcoin] laanwj closed pull request #8980: RPC: importmulti: Avoid using boost::variant::operator!=, which is only in newer boost versions (master...compat_importmulti_oldboost) https://github.com/bitcoin/bitcoin/pull/8980
< BlueMatt> sipa: thanks
< gmaxwell> bleh why are we using boost::variant?
< wumpus> don't know, ask satoshi? that has been used for the base58 stuff as long as I remember at least
< sipa> nope, i introduced it.
< sipa> jonasschnelli: how do you unhide the catching up overlay?
< wumpus> I guess it's a convenient way to do that
< sipa> gmaxwell: well because it's exactly what we need for CTxDestination... a tagged union
< wumpus> I absolutely doubt it's the hardest thing to replace if we really were to want toget rid of hoost
< sipa> wumpus: absolutely
< wumpus> why is it such an issue for you gmaxwell ?
< sipa> though it's not trivial either
< sipa> as using a C union with non-trivial types in it is asking for problems
< achow101> Victorsueca: the second I error I ran into is because python.exe isn't being called where normally it will with linux
< achow101> tl;dr don't run the rpc tests on windows because windows does stupid things
< wumpus> yes i can't just be replaced with a C union, agreed, but thinking of some other construct to replce it doesn't seem ahrd
< Victorsueca> achow101: thanks, will put that on a bitcoin block so everybody knows ;)
< sipa> jonasschnelli: ah, clocking on the warning icon. that took me a while - isn't there a way to make it more obvious, like adding another top bar with "You're currently out of sync with the network. Click here for more information' ?
< wumpus> the usualy way in C++ would be something with subclasses
< sipa> yuck. that means the child classes need to know that they're part of a union
< wumpus> sipa: sure, feel free to make it more obvious
< sipa> wumpus: i don't know Qt :)
< wumpus> sipa: well evertying is a compromise, I'm sure some other solution could be thought up without that drawback
< wumpus> without having to go into boost template hell
< sipa> i guess my poreferred solution would be to reimplement boost::variant :)
< sipa> c++ really lacks a tagged union
< wumpus> bleh
< wumpus> the point of moving from boost is not to reimplement it, if that's what needs to happen we should just stuck with boost
< * sipa> is maybe biased by Haskell, where tagged union is pretty much the only way to construct new types
< cfields_> sipa: iirc c++14 adds std::variant and/or std::any
< wumpus> the only valid reason to change it if there is a more c++11 way of doing things, if not, let's just keep it as it is
< wumpus> we *don't * want to support half of bost as part of bitcoin core
< sipa> cfields_: aha!
< cfields_> sipa: bah, both c++17 :(
< wumpus> are we at c++17 yet?
< sipa> cfields_: oh, so we only need to wait until 2022 until we can use it in Bitcoin Core?
< wumpus> yes
< cfields_> heh
< cfields_> sipa: i think that's not quite fair, though. 14/17 are quite different from 11. afaik, gcc/clang are both ahead of the spec, rather than years behind as with c++11
< wumpus> knowing that it is supported in a future c++XX standard also removes all motivation to look for an alternative way to do it :)
< sipa> afaik c++14/17 change much less than what c++11 changed wrt to c++03
< wumpus> we only need time, and that elapses automatically
< cfields_> wumpus: yep, see std::filesystem :)
< gmaxwell> wumpus: because there is no analog in C++ afaik, and I recall the implementation being really ugly and slow-- though perhaps I'm wrong, and another boost dependency.
< cfields_> sipa: yes, true
< sipa> how many headers does testnet have?
< gmaxwell> wumpus: but it wuld be inaccurate to say it's such an issue.. it's now, I was just asking.
< wumpus> gmaxwell: sure, I'dcompletely believe that, though it's not used in any performance critical inner loops IIRC, so I don't think the performance is so important here
< wumpus> it's apparently just hard to support something like that in c++
< gmaxwell> wumpus: from a general principle of programming, using runtime determined types often undermines type safty. (often irrelevant on a case by case basis, but if I'm speaking in generalities...)
< wumpus> gmaxwell: well you're welcome to implement it in a better way! :)
< sipa> it's preferctly applicable in cases where you can say "an X is either a Y or a Z"
< cfields_> gmaxwell: yes, the more c++ish way of doing it almost certainly implies try/catch+dynamic_cast, which i find pretty nasty
< wumpus> yes, I don't think we use it in a type safety undermining way, we have visitors and such to handle all the cases
< sipa> cfields_: puke
< gmaxwell> (and yes, I also believe polymorphism is generally ill-advised based on the same argument)
< wumpus> cfields_: or as I said above, subclasses and the visitor pattern. NOt that that would really be any improvement in type safety
< sipa> found the C programmer.
< wumpus> sigh, let's not get into that discussino
< cfields_> wumpus: yes, that'd be much cleaner
< sipa> (though i would agree with saying that such constructions are often used in places where they're unnecessary and harmful)_
< wumpus> cfields_: sipa didn't like it because the subclasses have to know they're part of an union
< cfields_> wumpus: yes, that'd be the "tag" part :p
< cfields_> oh, i see what you mean
< wumpus> sure, polymorphism is certainly overused in some cases, espeically in the beginnign when it was new and everything had to be an object
< sipa> *switch topic* what is testnet's height?
< gmaxwell> sipa: yea, I don't mean that in a dogmatic sense. And I think in bitcoin core things are usually sensible. But I've had too much exposure to codebases where novices overuse these really exotic tools.
< cfields_> sipa: UpdateTip: new best=00000000000025c20dd75c51046acae15bdaa04151db3fc50d8d9cc673e6899e height=1009187 version=0x20000000 log2_work=68.584926 tx=11687999 date='2016-10-20 18:45:26' progress=1.000000 cache=6.6MiB(15650tx)
< wumpus> gmaxwell: yes - it's even worse in java code generally :)
< gmaxwell> sipa: on segwit nodes 1009190
< cfields_> as of a min or two ago
< sipa> cfields_: thanks. trying to sync headers over a really slow connection
< * wumpus> should start running a dedicated testnet node again
< jonasschnelli> <*highlight>[20:37:05] <sipa:#bitcoin-core-dev> jonasschnelli: ah, clocking on the warning icon. that took me a while - isn't there a way to make it more obvious, like adding another top bar with "You're currently out of sync with the network. Click here for more information' ?
< wumpus> btw: I've had, out of many 'no' responses, only two people admitting they're still running bitcoin core on windows 32-bit. Both expect to stop doing so in the next 6 months.
< sipa> thinking a bit more about it, std/boost::variant isn't exactly a generic tagged union, it's a union where the type is the implicit tag
< jonasschnelli> Yes. It's a bit hidden. What about clicking on the progress bar in the status bar?
< sipa> and a real tagged union would be much more useful
< wumpus> sipa: good point
< sipa> jonasschnelli: that'd be useful too
< sipa> (it was the first thing i tried)
< wumpus> so I think stopping support for windows 32-bit in 0.15 would make sense
< achow101> meeting?
< wumpus> in two minutes!
< sipa> one and a half
< achow101> aw man, my clock's fast
< Victorsueca> what meeting?
< jl2012> the last windows with 32-bit is windows 7?
< achow101> Victorsueca: dev meeting
< achow101> jl2012: windows10 has a 32 bit version
< Victorsueca> ^
< Victorsueca> achow101: is it going to be broadcasted?
< achow101> it happens right here on irc
< CodeShark> it's here
< wumpus> #startmeeting
< lightningbot> Meeting started Thu Oct 20 19:00:15 2016 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< wumpus> proposed topics?
< sipa> proposed cheer: yay for 0.13.1rc
< CodeShark> ^ :DDD
< jonasschnelli> heh
< wumpus> yay for 0.13.1rc, haven't heard of any real problems yet
< achow101> hmm. where's gmaxwell-ping-bot
< wumpus> not that it says much given how short it's been out, but ok, it's somewhat promising :)
< CodeShark> if only execution paths with problems tended to occur first :p
< wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
< jtimon> proposed topic: libconsensus: do we want to expose a verifyBlock function or a processblock one? do we want to expose verifyHeader and verifyTx ?
< btcdrak> here
< wumpus> ah yes I have another topic about the libconsensus interface: what to do with undocumented flags
< instagibbs_> present
< btcdrak> ping jl2012
< * paveljanik> here
< jl2012> yes
< wumpus> jl2012 was already here
< jtimon> wumpus: like bip113 ? or more like bip30 ?
< wumpus> #topic libconsensus
< wumpus> currently it's possible to pass non-consensus flags into libconsensus
< wumpus> e.g. policy only flags
< CodeShark> yes, that should be pulled out
< sipa> i believe we should not expose policy functions in libconsensus
< sipa> as that would make it impossible to later split off policy code into a separate codebase
< BlueMatt> sipa: ack
< sipa> and consensus code should end up being isolated
< wumpus> ok so that means you agree with #8976
< instagibbs_> was there thought otherwise? (I'm outsider to this work)
< CodeShark> yeah, doesn't seem very controversial
< jtimon> I agree, currently it is not possible (at least using bitcoinconsensus_verifyScript)
< instagibbs_> ah I see what you mean
< wumpus> no, I don't think so. But people may be relying on this. I don't know anyone that does though.
< wumpus> and NicolasDorier is okay with it
< BlueMatt> wumpus: dont have a strong opinion, but I'm fine with that
< wumpus> if there is to be a policy script library it'll need to be separate imo
< jtimon> btw, currently the flags in script/bitcoinconsensus.h and in script/interpreter.h need to match, that shouldn't be the case
< wumpus> this is lib*consensus*
< sipa> jonasschnelli: agree
< sipa> eh, jtimon: agree
< wumpus> jtimon: no, they don't need to match, that's just an artifact
< sipa> wumpus: is there are translation layer?
< wumpus> although it's no longer possible to change the flags now
< wumpus> ABI for libconsensus is fixed
< jtimon> wumpus: I mean, if I change bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS = (1U << 11) to some other number different from 11, things won't work
< wumpus> sipa: no, that was never added
< sipa> we should add one
< sipa> and keep the flags for the existing bits
< jtimon> agreed
< sipa> as passthrough
< wumpus> yes, there should be a translation layer, though the current bits can no longer be changed
< wumpus> agreed
< sipa> but new ones can fill up the holes
< wumpus> at the lesat we should add the error when invalid flags are passed
< CodeShark> is anyone using these flags as ints rather than just as an enum?
< wumpus> this will discourage people from doing what they're doing now, e.g. treating it as a pass-through
< wumpus> they're using it as an unsigned int because that's the type passed in, remember it's a C interface, enums don't support bit field combos
< sipa> CodeShark: we're already abusing enum here. it's a bit field, not an enum
< sipa> the enum is just an easy way to give the constants name
< jtimon> CodeShark: AFAIK there used always like if (flags & MY_FLAG)
< sipa> for almost all operations we do with these enums, they decay into integers
< wumpus> yes
< CodeShark> right - I meant, does it matter if we shuffle the bits as long as the integer values are not used anywhere outside the definitions?
< sipa> CodeShark: that breaks the ABI
< wumpus> you can't shuffle the bits because it would break the *binary* interface
< CodeShark> oh...
< sipa> you can't link with an older version of the library in that case
< CodeShark> ok
< CodeShark> gotcha
< wumpus> the idea is that you can jsut drop in the 0.13.1 consensus library in place of the 0.13.0 one and it will work
< sipa> without recompiling your client
< wumpus> arguably the SEGWIT bit can be moved until 0.13.1 is finalized, but meh
< sipa> indeed, meh.
< jtimon> these are the consensus flags I end with in that branch: https://github.com/jtimon/bitcoin/blob/0.13-consensus-flags/src/consensus/flags.h
< CodeShark> sipa: so you're saying we can reuse the bits that get vacated when we remove the policy flags
< sipa> CodeShark: yes
< sipa> as those were never part of the abi
< wumpus> CodeShark: the policy flags were never really allowed by the interface, it's a bug that it posibble to specify them at all
< sipa> *api
< wumpus> this is what #8976 fixes by adding input checking on the flags
< jtimon> and my preference would be to expose a GetConsensusFlags call in libconsensus too
< wumpus> what would that do?
< jtimon> to hide the bip9 and previous developments stuff
< jtimon> you call it, now you know which flags to use verifyScript, verifyTx or verifyBlock (or verifyBlock could call it internally)
< sipa> and you pass in all block headers?
< jtimon> no, the same CBlockIndex interface used for verifyHeader
< sipa> i really don't like turning our internal representation for headers into an index
< sipa> *into an interface
< CodeShark> sipa: agreed
< CodeShark> it could be done better
< jtimon> other ideas to interface with storage ?
< CodeShark> without exposing all that crap
< jtimon> CodeShark: how?
< sipa> just have an API where you can create a blockindexstore, and you give it headers
< sipa> which are copied into the blockindexstore
< CodeShark> yes
< jtimon> so libconsensus remains coupled with our storage?
< sipa> that's my personal preference
< CodeShark> well...hmmm
< jtimon> I wasn't planning on taking any headers from callers
< kanzure> i don't mean to go all pointy hair or anything, but what is current expectation around libconsensus separation milestone timelines
< sipa> kanzure: nobody even agrees what libconsensus means.
< kanzure> perfect
< CodeShark> the header storage engine is quite trivial, actually
< CodeShark> not sure it needs an abstract DB interface...but on the other hand...
< wumpus> I think the previous conslusion was that libconsensus should remain coupled with the current caching layer, but not with leveldb
< jtimon> ok, so you guys want the libconsensus++ luke-jr wanted (ie storage included), I would be fine with having both one with storage included and one without it
< BlueMatt> we already discussed this in milan
< wumpus> so the *memory* storage is part of libconsensus
< wumpus> the disk storage is not
< BlueMatt> that
< CodeShark> ok
< jtimon> BlueMatt: yeah, you and me discussed it a little, with no conclusion
< wumpus> right,we discussed that in Milan
< sipa> jtimon: my fear is that it's very hard to create a stable API for storage of headers... things like BIP9 cache and the skiplist for example are things that would break the API, but such changes may be needed in the future
< kanzure> meaning of "storage" has to be carefully defined
< sipa> they're perfectly compatible with a store into which you can load headers, though
< jtimon> sipa: so let's break the API, users of libconsensus++ may have a more stable API, but less flexibility and control too
< jonasschnelli> We are speaking of a in-memory only store, right?
< BlueMatt> i think the first target is this, and then, if at some point we decide we want to support no-headers-in-memory, we can add an api for storage
< wumpus> jonasschnelli: yes. as currently used for the headers
< BlueMatt> but i think that is further out on the horizon
< wumpus> BlueMatt: agreed, that's no issue right now
< BlueMatt> also because refactoring every use of CBlockIndex into an api right now would be a ton of work/review/diff
< sipa> the criterion would be that we ourself want to use it
< wumpus> later on it may make sense to not support storing all headers in memory, but let's let's not try to do everything atonce
< wumpus> s/not support/support not/
< sipa> if the API somehow needs to miss features, e.g. because adding some cache is hard, that goal is already broken
< CodeShark> no need for premature optimization here
< CodeShark> header storage is not a bottleneck ;)
< sipa> CodeShark: it's not premature optimized. it's breaking existing optimization
< kanzure> CBlockIndex usage is not necessarily libconsensus-only, right?
< jtimon> I was speaking both memory and disk storage, for all I know, some callers may have the headers on disk and maybe others have the whole utxo in memory
< sipa> storage isn't, but block index traversal needs to be past
< sipa> *fast
< kanzure> i mean that's only because nobody wants to maintain a CBlockIndex and also libconsensus, ya?
< sipa> kanzure: i have no clue what you're talking about
< Victorsueca> kanzure: maybe we could define "storage" as the port where you request/send data that needs to be stored but is not immediately required
< sipa> storage... port?
< sipa> wth?
< wumpus> I think a good goal should be that we could use libconsensus ourselves, at least it will have one client then :)
< kanzure> (i was responding to "refactoring every use of CBlockIndex into an api".)
< CodeShark> let's not get sidetracked
< wumpus> please, let's not split hairs over definitions
< Victorsueca> sipa: not a network port obv
< wumpus> any other topics?
< cfields_> wumpus: +1. As a side-effect, that also forces devs to become familiar with it.
< gmaxwell> sorry, went to sleep during API discussions. I agree that the library should be used by bitcoin core if it is to exist. :)
< gmaxwell> (I also support it existing, to be clear!)
< wumpus> cfields_: right, so it's possible to fork bitcoin core but still use the same libconsensus, for example
< jtimon> BlueMatt: an interface for CBlockIndex wouldn't requiore a ton of review, just some review. Remember you can use wrappers for the old stuff and only libconsensus needs to use the interface (uppper layers can remain using CBlockIndex directly), please see https://github.com/bitcoin/bitcoin/pull/8493
< wumpus> gmaxwell: if some topic isn't interesting to you, you don't need to be loud about that
< jonasschnelli> Would it hurt if the libb*'s blockstorage be completly decoupled from the CBlockIndex, new structures, etc. as a first step?
< sipa> jonasschnelli: we're not even talking about block storage
< jonasschnelli> sorry, heards
< CodeShark> we're still on headers :p
< jonasschnelli> headers
< jtimon> conclusion, nobody seems to want the libconsensus I've been trying to move towards to, and as an external caller I wouldn't want a libconsensus++ (coupled to bitcoin core's storage and concurrency)
< kanzure> external caller == other wallet?
< jtimon> you guys want a processBlock function, not a verifyBlock one
< sipa> jtimon: i think exposing verification functions at different levels is useful
< jtimon> kanzure: yep, a wallet, an alternative implementation, whatever
< sipa> exposing headers, individual block, total block, ...
< sipa> jtimon: but the question is about whether or not to abstract out the state needed for that
< jtimon> sipa: cool, BlueMatt seem to think it's not
< sipa> those are independent questions
< jtimon> in any case, you don't want a verifyHeader independent of storage
< kanzure> jtimon: yes to me it sounds like you need to pass to libconsensus a reference to something that implements storage. but iirc there are some concerns about consensus-coupled storage backend details.
< sipa> i think it would hurt our own usage of it, as it means fewer opportunities to improve memory usage
< jtimon> sipa: I think "is it needed" it's not the question, nothing is needed, the wuestion is what we want to do
< sipa> what i want to do is have the consensus code abstracted out
< wumpus> it is needed if it is used by us
< sipa> modularized
< jtimon> kanzure: I highly doubt libbitcoin will ever use a libconsensus that's coupled to bitcoin's storage and concurrency, for example
< BlueMatt> jtimon: I have no problem exposing a verifyblock function that makes no external state lookups, but I dont think its so useful
< sipa> that doesn't mean we need to abstract out every data structure it uses
< BlueMatt> jtimon: I dont see much use for a verifyblock function that makes external state calls when you could just do processnewblock
< kanzure> jtimon: well the complicating detail here is that folks probably just want to use core's current storage implementation details (etc) to cut down on work, i think.
< CodeShark> breaking out the storage engine can be done independently
< wumpus> BlueMatt: yes, I remember we discussed that before, too :)
< kanzure> what is the concurrency coupling that jtimon mentions in particular
< sipa> CodeShark: we're not really talking about (disk) storage here, just in-memory representation of data structures
< BlueMatt> wumpus: yes, this is exactly the discussion we had in milan
< jtimon> BlueMatt: right, I believe some callers don't want the library to do the processnewblock because they want to do certain things themselves (for example, libbitcoin AFAIK)
< BlueMatt> jtimon: have you spoken much to these folks? (I dont know, just asking, would love to see their responses)
< jtimon> kanzure: if it's to cut down on work we can have both, that was my conclusion from a previous discussion with luke
< wumpus> can we worry about that later? I think a good first goal would be to make the libarary useful for us, I agree with sipa that tha doesn't need abstracting every data structure
< sipa> yes, abstracting the data structures can still happen later
< wumpus> I think this is typical bitcoin scope creep
< CodeShark> yes - modularization is what's most important, then we can further optimize each unit
< kanzure> sounds like an emphasis on code movement first
< jtimon> BlueMatt: mostly only to erik from libbitcoin, but yeah, probably we should ask around before trying to guess what they want
< wumpus> nothing will ever get done this way and we keep repeating the same discussions
< wumpus> +tons of huge code changes that will take ages to review
< jtimon> I want libwally to use verifyHeader, its main author has seen #8493 but I don't know what he would think about a version with "storage included"
< sipa> jtimon: one reason for me is that for some things, performance is in fact consensus critical, and requiring the user of the library to implement their own optimized data structures is essentially requiring them to implement some portion of consensus-critical code
< wumpus> I just think it'd make sense to aim for a near-term goal of exposing something, instead of trying to refactor the whole code base first
< CodeShark> the focus for now should be on separation of units and removing dependencies
< jtimon> well, all I want is to have a common and clear idea of what libconsensus should be
< sipa> while we already have optimized data structures, and not abstracting them out leaves more opportunities for future such optimizations
< wumpus> it should be a libarary that we ourselves can use for consensus validation
< wumpus> sipa: exactly! *not* exposing something as part of the API leaves flexibility to improve things later
< wumpus> sipa: putting it in the API/ABI effectively sets it in stone
< jtimon> CodeShark: but then people won't "see the point" of taking consensus code out of main...
< CodeShark> ?
< CodeShark> there
< morcos> I think it would be nice if we proceeded down _a_ path right now but left ourselves open to revisiting some of these decisions as we get further along.
< morcos> For instance I disagree a bit with the flexibility when it comes to cache optimization for pcoinstip.
< CodeShark> there's a very simple justification for it - moving the code out of main.cpp means far simpler merging of code changes
< jtimon> CodeShark: ie #8337 #8329
< morcos> But we don't have to answer all those questions right now
< sipa> jtimon: i think everyone agrees that modularizing consensus code is a good idea, independent of whether it's exposed as a library, or has refactorings for data structures or not
< sipa> morcos: agree
< kanzure> jtimon: you are referring to friction with code separation changes? i think some of that friction can be ameliorated by having it a prioritized shared goal (like segwit was).
< CodeShark> kanzure: indeed
< jtimon> sipa: the thing is some dependencies remain "hidden" or hard to see until you separate stuff or try to move the "consensus files" to the consensus module to expose something
< sipa> jtimon: well things may not be easy
< jtimon> kanzure: I would love that
< CodeShark> let's not get hung up on how the lib API will be exposed and start working on moving code into separate units
< Chris_Stewart_5> ^
< BlueMatt> ^
< jeremyrubin> ^
< sipa> v
< jtimon> CodeShark: I'm happy with that
< jtimon> I tried that too
< Chris_Stewart_5> always one sipa..
< jtimon> but some people asked for the final API...
< sipa> i'd really just want to see a proposal of a directory structure
< kanzure> jtimon: iirc you in the past have had problems with some pull requests because others would complain about additional merge/rebase work for their unrelated changes.
< sipa> which explains code responsible for what belongs where
< jtimon> sipa: I gave you one: all consensus fles except those in crypto to the consensus dir
< sipa> jtimon: that's not an answer
< jtimon> it is one you don't like
< sipa> there is code shared between consensus and non-consensus
< sipa> what happens to script? is it split up again?
< sipa> where does the signing code go?
< jtimon> but I really don't see how can we make a subrepository or subtree out of libconsensus otherwise
< kanzure> sipa did you read jtimon's libconsensus doc by any chance
< sipa> do we duplicate consensu logic?
< jtimon> sipa: signing code is not consensus
< sipa> sigh
< sipa> i know that
< jtimon> it remains in the common package
< jtimon> and the script dir
< sipa> ok, i'll shut up about it
< wumpus> I think this is monipolizing the meeting. ANy othe topics to be discussed?
< sipa> i can't seem to get my point across
< jtimon> no, you brought this point more times
< CodeShark> can we set as a goal to prioritize some moveonly PRs?
< sipa> yes, and i have never seen you give an answer to my question
< CodeShark> sipa, jtimon, let's save that for after the meeting
< CodeShark> can we agree for the time being to define a directory structure and prioritize moveonly PRs?
< jtimon> sure I really want to understand his concerns better. It seems related to the discussion we had around luke's script "debugger"
< wumpus> I can't prioritize moveonly PRs. There's just too much happening
< CodeShark> wumpus: the idea is that they should be super simple to review
< sipa> CodeShark: you underestimate it
< michagogo> achow101: looks like it finished uploading, if you want to try it
< sipa> moving the code is easy. deciding where things belong is not.
< wumpus> but if people prioritize reviewing them, sure
< kanzure> oh is review the bottleneck? i keep thinking it's "lots of other rebase work" for other pulls.
< jtimon> wumpus: I think it being a priority for reviewers is more important
< wumpus> kanzure: that's also an issue
< sipa> kanzure: imho the bottleneck is no agreement about what should be done
< michagogo> Wait, it's Thursday... sorry, didn't realize meeting was happening
< * michagogo> scrolls up
< jtimon> kanzure: I strongly feel review is the bottleneck
< instagibbs_> proposed topic: rc2 issues, etc?
< wumpus> kanzure: though not always a strong one, usually it's fairly easy to rebase over pure moves. As long as people agree that they should be done.
< kanzure> oh. alright.
< jtimon> CodeShark: also I think you understimate the potential for disagreements
< Chris_Stewart_5> How do we keep this from being discussed for the next 3 meetings is my question. What can actually be done persuade people one way or the other?
< CodeShark> can we at least agree to start discussing a directory structure? ;)
< CodeShark> (after this meeting, of course)
< jtimon> CodeShark: ACK
< wumpus> Chris_Stewart_5: you tell me
< morcos> Someone should schedule a small in-person retreat for people who really want to work on libconsensus, to make some headway
< kanzure> Chris_Stewart_5: perhaps sipa could make a proposal if we ask nicely.
< jtimon> I already buried my hopes of libconsensus becoming eventually C
< wumpus> Chris_Stewart_5: preventing the topic from being discussed is quite easy, but I think that would be seen as censorship :)
< Chris_Stewart_5> This is ALL libbconsensus right? Is tehre any concrete proposals?
< instagibbs_> 14 minutes in case anyone wants to discuss anything else
< jtimon> Chris_Stewart_5: afaik the most concrete proposal right now is #8493
< wumpus> instagibbs_: I asked in the beginning, don't think there's any issues with rc2 yet
< Chris_Stewart_5> Thanks, i'll take a look at it.
< kanzure> ah 8493 is expose verifyheader. ok.
< instagibbs_> wumpus: ok great
< CodeShark> so any other topics?
< Chris_Stewart_5> rc2 issues?
< wumpus> which rc2 issues?
< BlueMatt> the lack of rc2 issues =D
< Chris_Stewart_5> *crickets*
< wumpus> 19:46 < wumpus> instagibbs_: I asked in the beginning, don't think there's any issues with rc2 yet
< kanzure> i think he was asking to hear for any
< wumpus> if there are, feel free to say so
< achow101> what about killing off windows 32 bit builds?
< jonasschnelli> I guess in 0.15
< wumpus> that's not an urgent topic really
< wumpus> yes, 0.15 I'd say too
< wumpus> I asked around and from 50 or so 'no' responses, there were two actual users admitting they were still using bitcoin core on windows 32-bit
< sipa> are there likely some who don't know?
< wumpus> they both expected this to stilll last only 6 months or so no longer
< wumpus> old hw
< gmaxwell> I haven't encountered any issues in RC2 yet, though its a bit new. The PR of mine that was merged appears to be working.
< jonasschnelli> If we have no other topic, we can discuss if we want to adjust the GUI default confirmation target down to the RPC interfaces value of 2 blocks
< wumpus> so that would time with 0.15, which is fine with me, no hurry
< Victorsueca> gmaxwell: the lastest travis build seems to have failed for master
< instagibbs_> jonasschnelli: yeah i suggested this a bit ago
< jonasschnelli> 25 blocks as "normal" confirmation speed sounds ridiculous
< achow101> jonasschnelli: I think it's a good idea
< instagibbs_> i dont understand why having different command line and GUI behavior like that is "good"
< morcos> jonasschnelli: to be honest, i actually agree we should make the target less than 25, but I think 2 is too low, and i think we're going to bikeshed where it should be
< jonasschnelli> Yes. I fear that as well. :)
< sipa> it seems reasonable that the default in gui and rpc is the same
< instagibbs_> morcos: "match between command-line and GUI" is a starting point :)
< BlueMatt> jonasschnelli: I'm for 25
< BlueMatt> 25 is good
< morcos> the issue is that when there is a break between blocks or network congestion, to be really sure you get confirmed very quickly, say in a couple of blocks
< jonasschnelli> And the slider should probably not touch nTxConfirmTarget!
< jonasschnelli> global!
< morcos> then you might have to pay areally high fee
< MarcoFalke> jonasschnelli: This should be uncontroversial. And actually it is a bug in the current code, as the default already says 25, but the slider is "mirrored", so it ends up on the wrong side.
< morcos> which is priobably not what you want
< MarcoFalke> I never fixed it because I wanted to clean it up "in a go" with all the other nasty things the gui does with the "fee"-globals
< morcos> more intelligent fee estimation would maybe gauge the current conditions against historical conditions or something.
< gmaxwell> Is anyone working on improving the estimator generally, right now?
< jonasschnelli> I think there are some users fooled by the fact that RPCs sendto* uses different fee estimations then the "default" GUI send method.
< gmaxwell> It could use improvement, though I think bumping is more important.
< jonasschnelli> I mean only different confirmations targets
< morcos> MarcoFalke: i dont think it was a bug. i noticed it when it went in and i thought it was on purpose, i think we discussed it at a time
< MarcoFalke> ok
< jonasschnelli> Someone should review the new bumpfee PR.
< morcos> gmaxwell: i mentioned on that issue that i had worked on it.. 6-9 months ago.. but abandoned it. i have some custom code that does a lot more
< morcos> but nothing that really got polished
< wumpus> yes, we *must* have a bumpfee for 0.14
< kanzure> morcos: had you looked at bramc's work on fee estimation?
< morcos> kanzure: i've been shying away from algorithms which require replacement. i think its a great idea, but not how most users expect their default transactions to work. I think most users want them to get confirmed relatively quickly on the first try.
< morcos> But an algorithm for bumping when you guess too low makes sense.
< gmaxwell> in any case, in my expirence the core estimator usually produces fees which are generally too high (compared to what gets confirmed in the next block, also compared e.g. to 21inc's estimator), having a higher confirmed target helps reduce the impact.
< morcos> so to be clear, if someone else has an idea of how it should be improved, please go ahead. and i'm happy to help. but its just one of those things that there is no "right" answer for so it can get frustrating
< gmaxwell> I was just wondering if there was anything ongoing at the moment.
< gmaxwell> (since the subject came up)
< jonasschnelli> What about changing the default confirmation target to 6 for RPC interface and the GUI once we have the bumpfee cmd?
< Victorsueca> as example, most of my transactions where I set the fee slider to 25 it confirmed on the next 2 blocks
< morcos> gmaxwell: yes, exactly and thats by design. i interprested the question of what does it take to be confirmed in X blocks as meaning you really want to be very sure it'll be confirmed wihtin the target
< jonasschnelli> Victorsueca: users made different experiences with that
< gmaxwell> morcos: And that is what it must do, esp when there is no bump. :)
< morcos> thats why i hesitate to set the default to 2
< jtimon> 2 mins
< morcos> jonasschnelli: i like 6, and i'd be fine with that....
< instagibbs_> overpaying is fine until we have bump, heh
< gmaxwell> I would be too.
< Victorsueca> jonasschnelli: yeah, I seen too lots of people blaming slow transactions even with recommended fee, but not my case for some reason
< CodeShark> wouldn't overpaying tend to raise fees up?
< CodeShark> for everyone
< wumpus> only if everyone is going to do that
< morcos> CodeShark: ah, that is one of bramc's concerns. i think the existing algo is pretty resistant to that
< jtimon> CodeShark: I could think of some kind of overpaying race
< jonasschnelli> Most important is probably review of mrbandrews's bumpfee (https://github.com/bitcoin/bitcoin/pull/8456)
< morcos> yes unless literally everyone does it
< MarcoFalke> We should just set it to a random value *ducks*
< instagibbs_> Assuming everyone is transacting at the same exact time, sure. But there's time preferences in real life, week/weekend patterns
< instagibbs_> etc
< wumpus> instagibbs_: +1
< instagibbs_> bitcoin businesses will do settlement on sunday evening to avoid fees
< sipa> TINGELINGELING
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu Oct 20 20:00:08 2016 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< instagibbs_> I mean they *do* today, not just hypoethical.
< kanzure> am interested to see resolution to jtimon/sipa concerns at some point
< jonasschnelli> I think we want resolution on the long term plan since 2 years or more. :)
< jtimon> so sipa I believe I have answered your question in previous times, just maybe you didn't like it or maybe I didn't make things clear that I considered implicit or obvious or something
< sipa> jtimon: i'll write my question done in a more detailed and clear way, then
< kanzure> jtimon, perhaps illustrate with an old pull request that shows a code move that matches what you are trying to explain
< wumpus> at the least having a bumpfee feature would make it possible to correct for too low fee, that'd already help a lot, no matter how good the estimation it's going to be wrong some of the time
< instagibbs_> urgh, my 0.13.1 rc1 node has *0* segwit connections
< sipa> kanzure: NO
< sipa> kanzure: i don't want to see code
< gmaxwell> morcos: one interesting question I've pondered is that -- is that fact that the estimators are blind to the market price mean that price increases are a pressure to increase fees (in real value) for everyone? I think they are.
< morcos> wumpus: yes agree entirely
< gmaxwell> instagibbs_: yea, apply rc2 directly to forehead.
< sipa> kanzure: the code to move things is huge, but trivial
< jtimon> I believe some code duplication is unavoidable once bitcoin core only talks to libconsensus' C API, for example, the primitives
< kanzure> there was an argument proposed where instead of only making libconsensus, someone (sipa?) said libconsensus + also some other library, and bitcoin core and also libconsensus consume from the other library.
< instagibbs_> gmaxwell: yeah, a little surprising to me how unaggressive behavior was
< kanzure> (or i misunsterood a comment from a few minutes ago)
< sipa> kanzure: hmm, at this point i do not suggest creating another library
< Victorsueca> I think the fees thing would cause what I call a "cocktail effect": people are at a party and everybody has cocktails on their hand, they speak between them and that causes background noise which difficults listening to your counterparty, he speaks louder so you can listen properly, but so does everyone else making the background noise louder which forces your friend to speak even louder
< instagibbs_> Speaking of which what is the intention of having outgoing connections that can't serve you data you want at all?
< jtimon> for generic signing and script-level policy, my preference would be to expose something like luke's "debugger" since the other option seems to duplicate the code for the interpreter
< morcos> gmaxwell: hmmm... i think what SHOULD happen is that if some people are a bit more price sensitive (in real value) that when they start paying a bit less as the price goes up, then the fee estimates ought to start edging down...
< sipa> jtimon: i'll write some things down... now lunchtime
< jtimon> sure, np
< wumpus> Victorsueca: but if many people are trying to use the system at once the fees are supposed to rise, that's fine as long as it's not an infinite feedback loop
< gmaxwell> morcos: yes. Though that control loop may be rather slow.
< Victorsueca> wumpus: that's the problem, if people offsets the fees a bit higher to be sure it confirms quickly it would cause a infinite feedback
< instagibbs_> wumpus: and I don't think that's likely as long as there exists different time preferences alone
< instagibbs_> which clearly exist
< wumpus> Victorsueca: some people would find the fee unacceptable and choose a longer confirmation period
< wumpus> not everyone cares for transactions to confirm very quickly, and wants to pay a premium for that
< Victorsueca> wumpus: right, I guess that force would balance the whole thing
< jtimon> kanzure: yeah a lot of this code can be easy after the discussions: not only trying different movement posibilities one by one is painful but also seems to burn reviewers really fast, maybe you take the effort to verify a moveonly commit but then someone doesn't like something about it and you're back to square one
< wumpus> jtimon: we have a problem with review everywhere, unfortunately
< CodeShark> as sipa said, deciding where the code should go is the hard part - moving it is easy
< wumpus> jtimon: after all the time we've not really managed to find morepeople that will review code changes
< jtimon> kanzure: by libconsensus++ I mean another library that calls the basic libconsensus, but it also handles storage and processes blocks
< wumpus> jtimon: sometimes I despair a bit
< CodeShark> the review that's needed is on the directory structure and unit definitions - the moveonly PRs are easy
< Victorsueca> CodeShark: move it all to /dev/null lol that would be easy
< jtimon> CodeShark: well, after the easy moveonlies
< Victorsueca> unfortunately the real thing is not that easy
< wumpus> jtimon: I mean there's 119 pulls open, by far most are blocked on review and testing, and sometimes about finding anyone else that cares about the change at all
< jtimon> there's little parts that you want to take, mostly from ConnectBlock
< kanzure> wumpus: so the reason why i thought the bottleneck was rebasing was because, there's this effect where lots of moves pile up and make for more rebase work, and then anyone with an open pull has an incentive to propose delaying moves until their PRs are in, hehe
< wumpus> jtimon: I have honestly no idea how to better organize this, or to find people that will help
< jtimon> wumpus: I understand, we have a big bottleneck in review in general
< CodeShark> the majority of the work that's needed here is not in writing code, though
< CodeShark> nor reviewing code
< CodeShark> it's more conceptual
< kanzure> i suppose my last message is more re: large refactors, more than it is about moves.
< wumpus> kanzure: that's certainly a dynamic that happens, large changes were delayed until after segwit to avoid giving segwit devs extra work
< CodeShark> how should we pull out different units? what are their interdependencies? how should our directory structure look?
< kanzure> wumpus: sure. yes.
< jtimon> my intuition is that we simply need more developers. at first they may be more review demanding, but later they review too
< CodeShark> we're talking design decisions here, though - not so much code
< wumpus> jtimon: we have plenty of people writing pulls, though much fewer thtat do useful review
< CodeShark> once we agree on the design decisions, writing and reviewing code will go much more smoothly
< wumpus> maybe we should add a rule that for every pull you submit youshould thoroughly review another one :p
< morcos> wumpus: 5
< instagibbs_> heh :) review without writing is hard though
< kanzure> wumpus: unfortunately that degrades into a quid pro quo situation
< CodeShark> but if we keep bikeshedding on design decisions while reviewing PRs it will never happen
< kanzure> or a quid pro quo review scheme or something
< jtimon> CodeShark: my thinking is this: if eventually we want libconsensus to become an independent project ala libsecp, then we need a dir for it
< jtimon> it can have subdirs, sure, but I'm not convinced it will need them
< CodeShark> the directory structure needn't be cast in stone - it just needs to be something that works well enough for now
< CodeShark> anything is better than stuffing everything into main.cpp :p
< Victorsueca> do you still have any of those bitcoins that where collected years ago for ad campaigns? maybe you could use them to somehow bring people into contributing
< jtimon> sorry for showing code, but this is what I would do first "for cheap": https://github.com/bitcoin/bitcoin/pull/8328
< CodeShark> once we start working on how to break out a library and expose an API perhaps we have better ideas on file system structure
< CodeShark> but that's still a ways off
< wumpus> CodeShark: re: splitting up main see https://github.com/bitcoin/bitcoin/pull/8969
< wumpus> Victorsueca: ad campaigns?!
< kanzure> Victorsueca: core itself did not run ad campaigns.....
< wumpus> Victorsueca: I can tell you the bitcoin core project has 0 bitcoins
< kanzure> wumpus: oh it's broke! :)
< jtimon> that kind of thing also forces us to discuss little things like: should utilmoneystr be in libconsensus? so far I think everyone thinks it shouldn't except maaku
< Victorsueca> I mean the thing that was collected on r/bitcoin
< kanzure> r/bitcoin is unrelated
< Victorsueca> yeah, but they engage people into giving idea on what to spend the remaining
< Victorsueca> ideas*
< MarcoFalke> What happened to the coins gavin used to give out for testing patches?
< instagibbs_> he gave them away
< wumpus> either they were from the bitcoin foundation or Gavin paid that out of his own pocket, I don't know
< jtimon> CodeShark: I'm happy with breaking main.cpp in smaller pieces, maybe get git blame to work with it some day, but I don't think "block connection logic" belongs in libconsensus
< CodeShark> I'm not even thinking libconsensus yet :p
< jtimon> ok, BlueMatt, let's say I'm an SPV wallet that only wants to use libbitcoin for verifyScript, verifyHeader and getConsensusFlags
< jtimon> how does verifyHeader gets the block if I'm never calling libconsensus' processBlock? I just have headers and txs, never full blocks
< sipa> if you have a blockheaderstore you can provide it with the headers
< jtimon> I see
< BlueMatt> jtimon: you can do processheader
< BlueMatt> which i guess is what sipa said
< jtimon> yeah, both options are equivalent, yeah, you could do that
< sipa> right... though i wouldn't mind having a way to just verify a header without processing it
< sipa> just a different level api
< sipa> but you'll still need a way to load headers into the black box state object in that case, independently from processheader
< sipa> which you may need anyway... when loading your header index from disk, perhaps feeding them to processheader one by one is not necessarily the best approach
< jtimon> why can't this black box object be just an implementation of the function pointers API? that was we can offer both tastes for every storage-dependenant call
< jtimon> anyway, rewarding dir struct, I'm happy to hear any other ideas besides my simplistic "everything required to validate consensus rules in the consensus dir"
< Victorsueca> jtimon: maybe inside the consensus dir make a subdir for those required to validate the consensus rules that where there since the beginning
< jtimon> it feels bad to break the script dir in 2, but I don't feel bad about moving the primitives, for example
< Victorsueca> then another subdir for those required to validate some BIP-specific rules
< jtimon> Victorsueca: that's most of them, I think handling deployments with flags is just fine
< Victorsueca> hmmm
< jtimon> currently most functions are about validating some rules for a given structure (ie script, tx, header, block), separating stuff per bip would be less clear and maintainable IMO
< Victorsueca> maybe a subdir for network consensus (such as median time, consensus height, handshakes...) another for transaction validation consensus, another for block validation consensus, another for soft-fork consensus....
< Victorsueca> I think I just duped libraries on different subdirs right?
< Victorsueca> we should also consider if a library would fit on more than one subdir then on which one would be more intuitive to find it
< jtimon> well, some of that stuff is already separated as files
< jtimon> for others that isn't I think a single file is probably fine
< jtimon> anyway, I'll go back to try to find what's wrong with my new testchain...
< Victorsueca> then I think we should put everything in the consensus folder, if there's not much to classify splitting stuff would only make things more unclear
< jtimon> maybe a script subfolder, but it would be just script.o, interpreter.o and script_error (which maybe should just turn into consensus_error)
< achow101> michagogo: what's the password for bob with your vm?
< Lightsword> hmm, my testnet node is stuck
< Lightsword> GBT won’t start since it’s stuck on downloading blocks
< achow101> michagogo: also, is build.sh your build script?
< michagogo> achow101: It's gitian
< michagogo> (that's in the description, too)
< michagogo> And yeah, that's the script I use for building
< michagogo> It requires a bit of customization (basically, add your name)
< michagogo> It does the whole build process
< michagogo> Run it, and it takes care of fetching the tag you give it, building, committing, pushing, and even creating the PR
< michagogo> (that last part also requires you to create a GitHub token)
< michagogo> Lauda: ICYMI, my prepackaged gitian builder VM is up at https://1drv.ms/f/s!AvCguGMVwWzLgxJPeXdvaQVJ2WJq
< michagogo> Along with a video showing how I made it, start to finish
< michagogo> (1 hour, from initial boot from Ubuntu 14.04.5 ISO, including trial and error)
< michagogo> (and looking stuff up outside the VM)
< michagogo> achow101: Did you get a chance to try it? Did it work for you?
< michagogo> And also, I've got to say... having just set it up from scratch, I really don't understand how/why people have trouble with it :-/
< molz> michagogo, can't see the file, have to have a MS account
< michagogo> molz: Really?
< michagogo> Hm.
< michagogo> I seem to be able to open it in incognito...
< michagogo> Oh, hmm.
< molz> oh i can download it
< michagogo> Okay, do you have a better way for me to get it to you?
< michagogo> Maybe I'll make a torrent of it -- does someone have a seedbox that can take ~3.6 GB?
< molz> michagogo, i'm downloading Gitianbuilder.7z
< michagogo> Ah, it's working? Great
< molz> michagogo, sorry i was afk, but no, i got "network error", can't download Gititanbuilder.7z