< GitHub12> [bitcoin] theuni opened pull request #8707: net: fix maxuploadtarget setting (master...fix-maxupload) https://github.com/bitcoin/bitcoin/pull/8707
< dcousens> Giszmo: hey mate you around?
< dcousens> nvm, PM
< GitHub101> [bitcoin] theuni opened pull request #8708: net: have CConnman handle message sending (master...connman-send) https://github.com/bitcoin/bitcoin/pull/8708
< GitHub140> [bitcoin] rebroad opened pull request #8709: Allow filterclear messages for enabling TX relay only. (master...AllowFilterclear) https://github.com/bitcoin/bitcoin/pull/8709
< wumpus> do we really need a zillion pulls updating copyright messages :/
< jonasschnelli> wumpus: heh. Indeed.
< wumpus> omg #8653 tells people building for windows to disable hardening :(
< wumpus> any pulls that I should review that make me happy instead of depressed?
< jonasschnelli> https://github.com/bitcoin/bitcoin/pull/8653 is indeed silly...
< jonasschnelli> Can't you not just use a PPA with a mingw compiler that suppors c++11 including threads?
< wumpus> why do the only people with an actual clue about security on windows seem to be the blackhats that try to steal your coins
< jonasschnelli> haha
< wumpus> well switching to a posix-compliant compiler is not silly
< wumpus> although it' a bit weird that it is needed
< wumpus> it's working around a mingw bug indeed
< jonasschnelli> I never had problems on any of my machines crosscompiling with minwg even after the c++11 switch...
< wumpus> apparently the non-POSIX version doesn't give you c++11 synchronization, but piling on a posix emulation layer is probably not ideal either
< jonasschnelli> I guess per default it installs the posix comp. mingw32-gcc/g++
< wumpus> I've had no issues at all either
< wumpus> and have built for windows on 16.04
< jonasschnelli> But why is the --disable-hardenin required?
< wumpus> so either it installs the posix one by default, or this is a new problem
< wumpus> I don't know, ask the guy, seems like really bad advice
< wumpus> 'our security system reports a problem sir'.. 'just disable it, it must be a false alarm'
< jonasschnelli> Yes. The advice comes also without a concrete reason...
< jonasschnelli> "You may also need to disable..."
< wumpus> heh.
< wumpus> I understand lazy developers, but being lazy with security issues areound bitcoin is a deadly sin
< GitHub32> [bitcoin] laanwj pushed 10 new commits to master: https://github.com/bitcoin/bitcoin/compare/2a0836f6d5e7...7e9ab9555cab
< GitHub32> bitcoin/master d2cd9c0 nomnombtc: add script to generate manpages with help2man
< GitHub32> bitcoin/master 6edf2fd nomnombtc: add gen-manpages.sh description to README.md
< GitHub32> bitcoin/master eb5643b nomnombtc: add autogenerated manpages by help2man
< GitHub42> [bitcoin] laanwj closed pull request #8608: Install manpages via make install, also add some autogenerated manpages (master...man_automake2) https://github.com/bitcoin/bitcoin/pull/8608
< Eliel> wumpus: I have this theory that there's not an actual shortage of people who have an actual clue about windows security but the motivations for actually learning about it are because they think the skill will make them money, so unless you're willing to pay a lot, it's not generally available.
< wumpus> Eliel: yes, that's how my reasoning goes, too. One popular way to make a lot of money from knowing windows internals is exploiting them or working for companies that sell add-on security products, but helping secure open source software is not one of them at least
< wumpus> it's a different kind of culture, in Linux people show that they know about e.g. kernel internals by working on them in public, in windows it's all much more hush hush
< midnightmagic> the Sysinternals guys (used to?) do *incredible* workshops
< wumpus> I'm sure an issue "ASLR doesn't seem to work properly on " Linux/*BSD or even MacOSX would be solved in a day
< wumpus> for windows you get one reply from a clueless user 'look at the DLL flags' .. .yea, we hadn't thought of that yet
< wumpus> yes the sysinternals guys are extrememly clueful, too clueful to be allowed to exist outside Microsoft so they were bought up :)
< luke-jr> jonasschnelli: being able to build from source is important; it's not enough to just have hacks that work for gitian
< wumpus> which 'hacks that work for gitian'?
< jonasschnelli> Yes. Please elaborate... :)
< jonasschnelli> As far as i know we are using the default unpatched mingw compiler in out ubuntu vm
< midnightmagic> <3 the sysinternals guys. very cool what they do. srsly, if they have workshops, attend them, just being around them makes you smarter. :)
< dcousens> wumpus: it doesn't seem that GetNextWorkRequired was called with a NULL argument since 2010 when pIndexBest =NULL was the default in main.cpp
< dcousens> but, I haven't checked *every* commit since then :S
< dcousens> I'd hazard it stopped having that oppurtunity when pindexBest was refactored out
< wumpus> dcousens: then an assert makes some sense; although in a more general sense we do use assert too much for invalid input handling
< dcousens> wumpus: its probably just an indication we should use more references rather than pointers? (unless the aim is to be C compatible)
< wumpus> though an assertion crash is absolutely preferable to a SIGSEGV due to a null pointer
< dcousens> to me, the assertion just indicates, we're 99% this doesn't happen, but not 100% sure enough to dereference at the call site
< wumpus> yes
< wumpus> I didn't mean that as an argument againt your specific change
< dcousens> wumpus: oh I know, I agree with you
< wumpus> using references would indeed avoid the issue in this case
< GitHub125> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/7e9ab9555cab...4ced5de71d4d
< GitHub125> bitcoin/master bc1d1f2 instagibbs: Update p2p-segwit.py to reflect correct AskFor behavior
< GitHub125> bitcoin/master 5547aeb instagibbs: p2psegwit.py transaction is rejected due to premature witness not size
< GitHub125> bitcoin/master 4ced5de Wladimir J. van der Laan: Merge #8528: Update p2p-segwit.py to reflect correct behavior...
< GitHub57> [bitcoin] laanwj closed pull request #8528: Update p2p-segwit.py to reflect correct behavior (master...rejectsw) https://github.com/bitcoin/bitcoin/pull/8528
< GitHub197> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/4ced5de71d4d...256215244105
< GitHub197> bitcoin/master 46606af BtcDrak: Update btcdrak signing key
< GitHub197> bitcoin/master 2562152 Wladimir J. van der Laan: Merge #8662: Update btcdrak signing key...
< GitHub118> [bitcoin] laanwj closed pull request #8662: Update btcdrak signing key (master...updatekey) https://github.com/bitcoin/bitcoin/pull/8662
< wumpus> jonasschnelli: as far as I know we're using an unpatched compiler for gitian win32/64 building too; I dont think we do any specific hacks to get it to build/run
< wumpus> well there is one hack to zero the heap during compilation to work around an issue in 14.04's mingw gcc compiler where four random heap bytes leak into the executable, but that's only required for determinism
< wumpus> so yes there are some hacks for determinism, someone building from source for themselves can safely skip them, that's why they're not described in doc/build-*.md
< wumpus> disabling hardening is advice you should never give anyone in good conscience though
< GitHub142> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/256215244105...39ac1ec64264
< GitHub142> bitcoin/master 1d635ae rodasmith: fix op order to append first alert
< GitHub142> bitcoin/master 39ac1ec Wladimir J. van der Laan: Merge #8697: fix op order to append first alert...
< GitHub145> [bitcoin] laanwj closed pull request #8697: fix op order to append first alert (master...fix-op-order-1st-alert) https://github.com/bitcoin/bitcoin/pull/8697
< wumpus> so is https://github.com/bitcoin/bitcoin/pull/8699 an intermediate measure for 0.13.1 or does the `createwitnessaddress` command need to disappear forever?
< dcousens> wumpus: my understanding was it was for testing, but has no purpose in mainnet?
< wumpus> right, so it should never have made it into a release at all, and needs to be removed now?
< wumpus> do people still use it for testing or is there a better alternative?
< wumpus> apparently the RPC tests don't use it otherwise it couldn't be removed without affecting the tests
< dcousens> wumpus: not sure about current testing, my understanding is the RPC now understand witness scripts?
< dcousens> not sure about the wallet code, haven't used it in terms of segwit tbh
< GitHub171> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/39ac1ec64264...37ac67816afb
< GitHub171> bitcoin/master 152f45b Peter Todd: Add option to opt into full-RBF when sending funds
< GitHub171> bitcoin/master 05fa823 Wladimir J. van der Laan: wallet: Add BIP125 comment for MAXINT-1/-2 behavior
< GitHub171> bitcoin/master 86726d8 Wladimir J. van der Laan: Rename `-optintofullrbf` option to `-walletrbf`...
< GitHub29> [bitcoin] laanwj closed pull request #8601: Add option to opt into full-RBF when sending funds (rebase, original by petertodd) (master...2016_08_full_rbf_option) https://github.com/bitcoin/bitcoin/pull/8601
< GitHub97> [bitcoin] MarcoFalke opened pull request #8710: [0.13.1] qt Backports (0.13...Mf1609-qtBackports) https://github.com/bitcoin/bitcoin/pull/8710
< dcousens> wumpus: IIRC the txoutsbyaddress isn't utxos byaddress
< dcousens> its just txos, throughout history
< dcousens> aka, give me the balance of address X @ blockheight 233000
< dcousens> atleast, that was my impression
< dcousens> nvm
< dcousens> my impression was wrong
< wumpus> no, IIRC it ignores history and just gives you current utxo, that's what makes it interesting
< GitHub114> [bitcoin] MarcoFalke pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/37ac67816afb...7fe6c5c99370
< GitHub114> bitcoin/master 438e94d whythat: remove root test directory for RPC tests
< GitHub114> bitcoin/master c62cc4e whythat: fix path for bak file
< GitHub114> bitcoin/master 7fe6c5c MarcoFalke: Merge #8652: [qa]: remove root test directory for RPC tests...
< GitHub174> [bitcoin] MarcoFalke closed pull request #8652: [qa]: remove root test directory for RPC tests (master...cleanup) https://github.com/bitcoin/bitcoin/pull/8652
< jl2012> wumpus: I think nothing in the RPC test is using createwitnessaddress, and that could be done with 2 lines of python code
< wumpus> right, nothing is using it
< jonasschnelli> hmm... NotifyHeaderTip is firing during IBD for every block connected to the main chain... I though it should only fire when a new header gets append to the headers-chain.
< sipa> jonasschnelli: that's what it does
< jonasschnelli> For the overlay I'm working on, a notification of the header-chain tip update would be convinient..
< sipa> i'm not sure why you think that's not what happens
< jonasschnelli> sipa: right now, I get NotifyHeaderTip() during IBDs header-sync of the blocks that gets connected (example: NotifyHeaderTip fires when header-chain is at 300'000 with a block-connect at height 100).
< jonasschnelli> Listening to the signal I cannot distinct between the actual header-sync-tip-update and the block-connect
< jonasschnelli> Or it looks like the listener cannot distinct.
< jonasschnelli> Well, now I can just load pindexBestHeader->nHeight when the signal fires..
< sipa> i'm really confused
< jonasschnelli> Maybe I'm interpreting the signal wrong..
< sipa> it should fire whenever we learn about a new best header
< jonasschnelli> What i'm looking for is a way to get a tip-update of the headers-chain (in order to calculate the remaining blocks to verify)
< sipa> ok
< sipa> that's what it doez
< sipa> does
< jonasschnelli> But it does fire _again_ when a block connects
< jonasschnelli> (with the header=true)
< sipa> it should not
< sipa> can you explain the call graph that results in that spurious call?
< jonasschnelli> sipa: I'll try to give you clear steps to reproduce... give me a minute
< jonasschnelli> sipa: I think I'm misunderstanding something. Isn't pindexBestHeader the chaintip of the headers-only chain?
< jonasschnelli> During the NotifyHeaderTip signal, I get something like: initialSync: 1, height: 359 but bestheaderheight->nHeight == 48000
< jonasschnelli> I expected the signal to fire (with headers=true) when pindexBestHeader get updated with the next chunk of headers.
< sipa> oh, i see
< sipa> i does not actually notify for the best header
< sipa> it notifies for updates to the best header chain which could be verified
< sipa> so only when all blocks towards it have been downloaded
< sipa> which is sufficient for reindexing
< jonasschnelli> sipa: Do you think it would make sense to extend the signal to also fire when the best header chain updates its tip?
< jonasschnelli> Or would that break the reindexing?
< sipa> i think that would be fine
< sipa> i'll try to create a PR
< jonasschnelli> sipa: Super! +1
< GitHub50> [bitcoin] jl2012 closed pull request #8685: Discourage P2WSH with too big script or stack (master...bigp2wsh) https://github.com/bitcoin/bitcoin/pull/8685
< wumpus> I still don't get the situation around the createwitnessaddress command https://github.com/bitcoin/bitcoin/pull/8699
< wumpus> why does it exist? why was it ever added if it is never going to be useful, and even dangerous?
< wumpus> what is the point of it?
< moli> wumpus, afaik it's used to create multisig SW addresses
< sipa> creating them without adding them to the wallet
< GitHub83> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/7fe6c5c99370...c9914c209468
< GitHub83> bitcoin/master 86c3f8d Johnson Lau: Remove createwitnessaddress...
< GitHub83> bitcoin/master c9914c2 Wladimir J. van der Laan: Merge #8699: Remove createwitnessaddress RPC command...
< sipa> it's a utility function to match the wallet code
< wumpus> ah like createmultisig
< GitHub163> [bitcoin] laanwj closed pull request #8699: Remove createwitnessaddress RPC command (master...nocreatewitadd) https://github.com/bitcoin/bitcoin/pull/8699
< wumpus> well in any case it's a historical curiosity now, probably should mention it is removed in the release notes, at least if adding it was
< sipa> it was only ever usable on testnet
< wumpus> I hope it will be gone before anyone loses their coins due to it
< sipa> with 0.13.0 it can never be used on mainnet, even after segwit activates
< wumpus> yes but my question was whether it should be removed on master
< wumpus> it is a pull to master
< wumpus> the description was indeed formulated as 'this is never useful on 0.13' ,so I asked whether it was on purpose that it removes it from master too, but appearantly it is
< MarcoFalke> We should still do a backport. Just for consistency.
< wumpus> yes
< wumpus> it's marked as needs backport
< sipa> i'd rather only add something like this back of there is a demand for it
< wumpus> agreed
< sipa> and then we can assess what protections are useful
< wumpus> it's just an utility call anyhow, it doesn't need to be on RPC
< sipa> indeed
< GitHub4> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c9914c209468...fa7caf6d9116
< GitHub4> bitcoin/master 62ffbbd instagibbs: add witness address to address book
< GitHub4> bitcoin/master fa7caf6 Wladimir J. van der Laan: Merge #8693: add witness address to address book...
< GitHub8> [bitcoin] laanwj closed pull request #8693: add witness address to address book (master...addwitbook) https://github.com/bitcoin/bitcoin/pull/8693
< btcdrak> sipa: are there any more changes coming to #8393 (segwit cb)
< sipa> i haven't looked at matt's proposed changes yet
< GitHub11> [bitcoin] laanwj pushed 4 new commits to 0.13: https://github.com/bitcoin/bitcoin/compare/a9429ca26dd8...4731623777ab
< GitHub11> bitcoin/0.13 41fd852 rodasmith: fix op order to append first alert...
< GitHub11> bitcoin/0.13 d9f0d4e adlawren: Fix minimize and close bugs...
< GitHub11> bitcoin/0.13 a37cec5 Andrew Chow: Persist the datadir after option reset...
< GitHub93> [bitcoin] laanwj closed pull request #8710: [0.13.1] qt Backports (0.13...Mf1609-qtBackports) https://github.com/bitcoin/bitcoin/pull/8710
< jonasschnelli> Re: https://github.com/bitcoin/bitcoin/pull/8559 ... should we just remove the maxuploadtargets "recommended" minimum?
< jonasschnelli> Any objections?
< wumpus> no objections
< cfields> no objection here either
< btcdrak> no objections
< cfields> btw, https://github.com/bitcoin/bitcoin/pull/8708/commits/89c57428e335fefe87c6f5b4fd173c9ba9bdb486 may be worth backporting. I'm not sure what the implications are, but i assume it's possible for that to cause a stall in initial sync. Not quite sure how to test.
< cfields> basically if a user doesn't have the expected services, we start to disconnect, but we send them an initial getheaders anyway. Not sure how long it takes to recover from that, maybe a non-issue.
< cfields> s/user/peer/
< wumpus> let's backport is just in case
< wumpus> btw: which maxupload test failure? does it fail intermittently?
< cfields> wumpus: the rpc test, in -extended
< wumpus> ok
< cfields> it fails 100% without #8707. With that, I think it's intermittent (depends on how much goes through the optimistic send) without #8708, which fixes it completely.
< wumpus> thanks for the explanation
< cfields> np, sorry for the breakage.
< cfields> thanks for shoving the CConnman PR in, btw. I had lots of stuff blocked on that :)
< GitHub110> [bitcoin] laanwj closed pull request #8457: Add block height support in rpc call getblock (master...feature/add-get-block-by-number) https://github.com/bitcoin/bitcoin/pull/8457
< wumpus> well this is a pretty light breakage as these things go, it had to happen at some time
< wumpus> now it's in it can be improved
< wumpus> right :)
< wumpus> I'm happy travis is so stable again
< cfields> yea, that was getting really annoying. And as gmaxwell said, it has a quick psychological effect. Doesn't take long before you start ignoring the failures completely.
< wumpus> yes, I was already starting to rely on local testing only again
< wumpus> (which means only one platform/compiler)
< cfields> yes, same
< GitHub58> [bitcoin] MarcoFalke closed pull request #7728: Fees: Tests: Check CFeeRate internal precision in mempool_tests.cpp (master...0.12.99-feerate-precision-test) https://github.com/bitcoin/bitcoin/pull/7728
< GitHub151> [bitcoin] jonasschnelli opened pull request #8712: Remove maxuploadtargets recommended minimum (master...2016/09/rem_maxupt_min) https://github.com/bitcoin/bitcoin/pull/8712
< GitHub20> [bitcoin] jonasschnelli closed pull request #8559: Change maxuploadtarget recommended minimum calculation (master...2016/08/max_ut) https://github.com/bitcoin/bitcoin/pull/8559
< phantomcircuit> wumpus: can you take a peak at 8696 (and 8695 but less urgently as no acks yet)
< phantomcircuit> oh you did look at 8695
< GitHub170> [bitcoin] MarcoFalke opened pull request #8713: [qa] create_cache: Delete temp dir when done (master...Mf1609-qaCacheTempdir) https://github.com/bitcoin/bitcoin/pull/8713
< GitHub155> [bitcoin] MarcoFalke opened pull request #8714: [qa] gitignore, travis: Remove unused lines (master...Mf1609-qaUnused) https://github.com/bitcoin/bitcoin/pull/8714
< MarcoFalke> cfields: Do you think it is hard to get osx builds woring?
< cfields> MarcoFalke: we already do a cross osx build. You mean a native one?
< Lightsword> what’s wrong with osx builds?
< MarcoFalke> If there is no plan to add native ones, we don't need the os:linux
< MarcoFalke> strictly speaking
< cfields> MarcoFalke: sure, it's just explicit there in case it's ever required
< morcos> cfields: additional slight bug in shutdown related to ConnMan
< morcos> you call g_connman->Stop() when it might be a null pointer
< morcos> there is even a comment above warning you not to do that. :)
< cfields> heh, looking
< morcos> can be demonstrated by just running 2 bitcoind's the second one tries to exit due to data directory locked and then segfaults
< cfields> morcos: indeed. Thanks, fixing.
< GitHub32> [bitcoin] theuni opened pull request #8715: net: only delete CConnman if it's been created (master...fix-connman-shutdown) https://github.com/bitcoin/bitcoin/pull/8715
< cfields> morcos: ^^. Thanks.
< GitHub83> [bitcoin] MarcoFalke opened pull request #8716: [qa] wallet: Check legacy wallet as well (master...Mf1609-qaWalletLegacy) https://github.com/bitcoin/bitcoin/pull/8716
< GitHub132> [bitcoin] spencerlievens opened pull request #8717: [WALLET] Addition of ImmatureCreditCached to MarkDirty() (master...patch-2) https://github.com/bitcoin/bitcoin/pull/8717
< GitHub169> [bitcoin] SCDeveloper closed pull request #8703: Addition of ImmatureCreditCached to MarkDirty() (master...patch-5) https://github.com/bitcoin/bitcoin/pull/8703