< earlz> jonasschnelli: not sure how to compile bitcoin-qt to be dynamically linked without building it on device.. and building on device would take days if it's even possible. I got a compiler error that the ARM compiler wasn't supported for building Qt through gitian, so guess I'm just out of luck on it
< jonasschnelli> earlz: Indeed. ARM & QT is a large rabbit hole...
< luke-jr> why? :/
< luke-jr> should be strictly easier than Windows/Mac
< luke-jr> unless you're trying to do an Android/iOS build or something
< earlz> I'm focused on raspberry pi specifically
< earlz> but I'd like to do it all cross-compile, not on device
< earlz> But unsure how to get gitian or the depends system to cooperate with that
< bitcoin-git> [bitcoin] jonasschnelli opened pull request #11590: [Wallet] always show help-line of wallet encryption calls (master...2017/10/enc_wallet_help) https://github.com/bitcoin/bitcoin/pull/11590
< wumpus> ARM and QT5 works great, a lot of embedded devices vendors use that combination, the problem (with regard to bitcoin) is that it's not really standardized, so one mgiht be using qt w/ kms backend, the other with wayland, the other with X11, there's no one-size-fits-all static executable
< wumpus> so it would be pretty pointless to distrubute them
< wumpus> with your own cross-compile environment you can certainly build bitcoin-qt for ARM
< wumpus> but it's quite specifific to the device
< wumpus> or at least what libraries and versions and such are in its buildroot
< wumpus> you might have luck changing qt to dynamically link, but you'd have to be sure to have a compatible version of qt5 on the device - ideally you'd build against the specific qt5 library that is on the device
< wumpus> if both the host and device are ubuntu you can "cheat" by installing the qt5-dev:armhf package on the host, the cross-build chain should automatically pick that up
< luke-jr> wumpus: ideally you'd build against some minimum required Qt.. Qt is good about backward binary compat, IIRC
< wumpus> luke-jr: yeah that'd work, certainly in our case you'd lose nothing with that approach, we don't use any actual qt5 features, let alone new ones...
< jouke> win 32
< wumpus> could bulid dynamically against the first sane version of qt5. We used to do something like that for qt4 IIRC
< luke-jr> wumpus: this would fix the Ubuntu issues as well, perhaps
< luke-jr> wumpus: we don't even need the Qt5 we build against to be completely sane either since we won't distribute that.. just binary-compatible
< wumpus> I remember there was a good reason we switched to statically linking qt, there were qt incompatiblity issues on some platforms
< wumpus> I think there was at least some security feature on one distro that affected qt's binary compatiblity
< wumpus> distribution-agnostic GUI software on linux is an unsolved problem, the only things that seem to work, unfortunately (because they're big hammers) are either statically linking or shipping the whole whopping library chain (most linux games do this)
< luke-jr> the latter is better IMO
< wumpus> and it becomes even more involved when embedded development of any form is involved.
< luke-jr> at least with that approach, users can choose between using the bundled libs or using the system ones
< wumpus> linux is an operating system where things are built from source, either by the user themselves or their distro/buildroot
< wumpus> distributing binaries for linux is really a no-go, and so is trying to provide any one-size-fits-all linux cross target
< luke-jr> dunno, for Android and iOS it might be possible
< wumpus> our current hacks work pretty well given that
< wumpus> yes, I don't consider android or iOS 'linux' for this intent. THey have heavily standardized API specifications.
< luke-jr> ABCore is nice, but loses the whole GUI :P
< wumpus> for 'open' linux there's just too much drift
< wumpus> too much custom patching
< wumpus> and so on
< wumpus> which is good for freedom but not for binary software :-) (which could be considered a feature in some regards)
< wumpus> yeah, user expectation wise qt isn't that suited to building android GUIs
< wumpus> bitcoin-qt's gui would be kind of weird on a mobile device
< luke-jr> the Qt example in Play store didn't seem that bad :P
< wumpus> probably the fancy qml qt5 stuff?
< wumpus> not 'widgets' that we use
< luke-jr> dunno, does QML not use the same code as normal GUI?
< wumpus> no, it's completely different
< luke-jr> I would have expected QML was just an interface to widgets
< luke-jr> that sucks
< * luke-jr> misses TrollTech
< wumpus> qml is a graphics markup language, a kind of 2D scene graph, with lots of plugins for animation, video, audio, touch gestures, etc
< luke-jr> I thought it did widgets too
< wumpus> its' erally different from widgets which is aimed at old-style GUIs (say, desktop spreadsheets)
< wumpus> going to QML would be a complete redesign of the GUI
< luke-jr> Widgets don't look native on Android?
< wumpus> which would be ok with me, but I just don't know anything about that side of GUI design, nor do I have the time
< luke-jr> .. ironic coming from the original GUI dev :P
< luke-jr> but tbh, I probably wouldn't want to change to QML
< luke-jr> I prefer doing the entire GUI in code
< wumpus> I come from opengl driver dev, rendering backends, not frontends :p
< luke-jr> sure, I just meant that you wrote bitcoin-qt entirely yourself
< luke-jr> initially
< wumpus> oh sure I've used qt a lot in the past, mostly for tooling
< wumpus> that's old-style GUIs though, I meant I don't get QML
< luke-jr> old-style GUIs ftw
< wumpus> I agree (but we're a dying breed)
< wumpus> in any case my point was that qt-for-mobile is mostly aimed at QML, widgets are not a good fit for what peopel expect w/ touch interfaces
< luke-jr> that's disappointing to hear
< wumpus> it would certainly be possible to port the current GUI as-is to android, but I'd expect that'd mostly result in complaints
< tevrizci> hi
< wumpus> hi
< bitcoin-git> [bitcoin] fanquake closed pull request #11587: Fix warnings when building with -Wthread-safety-analysis (master...–Wthread-safety-analysis) https://github.com/bitcoin/bitcoin/pull/11587
< fanquake> wumpus are you still around?
< promag> a bit late for the qml chat
< promag> > qml is a graphics markup language, a kind of 2D scene graph, with lots of plugins for animation, video, audio, touch gestures, etc
< promag> maybe it started in that context, but qml is a declarative language with a javascript engine
< promag> it is used in other contexts, for instance, qbs which is the most recent building system
< promag> anyway, I though that if we want to split the wallet UI and add IPC then it would be a great opportunity to reimplement the UI in QtQuick
< luke-jr> promag: I don't see what reimplementing the wallet has to do with splitting it out
< promag> to change the current wallet from widgets to qtquick would take several PR's (which would sit there for a long time)
< promag> dunno if the IPC PR from ryanofsky is going forward
< promag> both require lots of changes to the wallet ui code
< promag> that's why I think adding IPC to glue a new wallet ui is probably easier
< promag> we could have both bitcoin-qt and bitcoin-quick and later drop support for the first
< promag> anyway, moving to qtquick is something IMHO we should plan
< luke-jr> why? what are the benefits?
< promag> modern ui, hw accelerated, adapts easily to non-desktop platforms, qt is pushing to that direction too, it is way easier to read and understand the code, it forces the mvc pattern..
< promag> not something we should do for the next release or 2
< fanquake> What's the minimum supported qt version?
< luke-jr> sounds mostly subjective. C++ Qt code is plenty readable.
< promag> qt5
< fanquake> 5. what?
< fanquake> 5.0
< promag> depends which components we plsn to use
< fanquake> Anything introduced earlier than 5.2 should be ok to use.
< fanquake> That is atleast the minimum for qt, because of an osx hack. Although it could be > now.
< promag> luke-jr: I've used both widgets and qtquick a lot, and from my experience with qtquick the code is more expressive and easy to read
< fanquake> promag do you have a good open source project that's using it as an example to look at?
< promag> mine no, I can point to some products that were using native code and moved to qtquick
< promag> qt has a lot of sample code
< luke-jr> I'm not sure embedding a JS engine in a wallet is a good idea :/
< promag> my point is, it's not we should do for the current ui, but we could do if something requires a great ui refactor
< luke-jr> hmm
< promag> luke-jr: why? overhead or security
< luke-jr> security mainly
< luke-jr> overhead may be a consideration for mobile too, I guess
< luke-jr> otoh, if we're refactoring, we can isolate the GUI from the wallet itself, so maybe less of an issue
< promag> right
< luke-jr> but not a non-issue either, since the GUI obviously needs to be able to make transactions
< promag> anyway, I could write a prototype if it's something we all agree on seeing
< luke-jr> promag: can C++ code contruct the GUI still with QtQuick?
< promag> you can manipulate the "DOM" in c++
< promag> but that is pretty ugly on the qml land
< luke-jr> I found it convenient to implement policy options in the GUI with format strings
< promag> usually you implement the model(s) in c++ and the view(s) in qml
< luke-jr> tr("Require transaction fees to be at least %s per kB higher than transactions they are replacing.")
< luke-jr> where %s becomes the option control
< luke-jr> not sure QML can do anything similar
< gribble> https://github.com/bitcoin/bitcoin/issues/4 | Export/Import wallet in a human readable, future-proof format · Issue #4 · bitcoin/bitcoin · GitHub
< promag> s/we/you
< luke-jr> no, that's just inserting stuff
< luke-jr> in my case, %s is turning into a spinbox, inputbox, etc
< promag> we mean the text to translate has something that is binded to a combobox selection?
< promag> err, s/we/you
< luke-jr> yes
< promag> text: qsTr("File %1 of %2").arg(combobox.selectedText).arg(total)
< luke-jr> no :p
< luke-jr> the combobox (or whatever type) would literally be inside the string
< * luke-jr> grabs a screenshot
< luke-jr> I suppose the trick to do this might be to implement a special widget *for* QML that can do it..
< promag> to do what?
< promag> everything there can be done with out-of-the-box qtquick, not sure what you mean
< promag> ah, but how do you do that with widgets? horizontal layout with label + combo + label?
< wumpus> fanquake: yes
< wumpus> promag: luke-jr: FWIW javascript of any kind in bitcoin core scares me, even in the GUI, I know this is not like dragging in a browser (it's not html based, nor is it meant as a sandbox for untrusted code) but it's still such a huge exploitable component
< wumpus> in addition to javascript just being a terrible language
< wumpus> I'd loathe to maintain that or even have to review changes to it
< wumpus> but the same argument would hold for e.g. integrating a lua or python scripting engine
< wumpus> even though I like those languages :-)
< fanquake> wumpus Cool. I think #11442 is ready to go.
< gribble> https://github.com/bitcoin/bitcoin/issues/11442 | [Docs] Update OpenBSD Build Instructions for OpenBSD 6.2 by fanquake · Pull Request #11442 · bitcoin/bitcoin · GitHub
< wumpus> fanquake: yes it is! sorry, lost track of it a bit
< fanquake> wumpus no worries. I was half waiting for someone else to test it, but I think it's better to just replace the outdated instructions.
< fanquake> Also need you to glance over #11573 given that you've pretty much maintained our changes to that code.
< gribble> https://github.com/bitcoin/bitcoin/issues/11573 | [Util] Update tinyformat.h by fanquake · Pull Request #11573 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/8335cb478183...e8f3c88133b7
< bitcoin-git> bitcoin/master 9d30f54 fanquake: [Docs] Update OpenBSD Build Instructions for OpenBSD 6.2
< bitcoin-git> bitcoin/master e8f3c88 Wladimir J. van der Laan: Merge #11442: [Docs] Update OpenBSD Build Instructions for OpenBSD 6.2...
< bitcoin-git> [bitcoin] laanwj closed pull request #11442: [Docs] Update OpenBSD Build Instructions for OpenBSD 6.2 (master...openbsd-doc-update) https://github.com/bitcoin/bitcoin/pull/11442
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.15: https://github.com/bitcoin/bitcoin/commit/cf18f4289911c657eb876d91dee055db807870ad
< bitcoin-git> bitcoin/0.15 cf18f42 fanquake: [Docs] Update OpenBSD Build Instructions for OpenBSD 6.2...
< fanquake> #11571 is also a trivial merge.
< gribble> https://github.com/bitcoin/bitcoin/issues/11571 | Fixed a couple small grammatical errors. by BitsInMyBlood · Pull Request #11571 · bitcoin/bitcoin · GitHub
< wumpus> hah the compiler checks for // Falls through comments now?
< wumpus> fanquake: thanks
< wumpus> anyhow the change looks obviously ok, just adds sanity checking
< fanquake> #11565 is also ready to go.
< gribble> https://github.com/bitcoin/bitcoin/issues/11565 | Make listsinceblock refuse unknown block hash by ryanofsky · Pull Request #11565 · bitcoin/bitcoin · GitHub
< wumpus> ah so gcc added recognition of fallthrough comments, as placeholder for in c++17 we get an explicit [[fallthrough]] marker
< wumpus> makes sense
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e8f3c88133b7...2631d55f61cd
< bitcoin-git> bitcoin/master 60b98f8 fanquake: [Util] Update tinyformat.h...
< bitcoin-git> bitcoin/master 2631d55 Wladimir J. van der Laan: Merge #11573: [Util] Update tinyformat.h...
< bitcoin-git> [bitcoin] laanwj closed pull request #11573: [Util] Update tinyformat.h (master...pull-upstream-tinyformat) https://github.com/bitcoin/bitcoin/pull/11573
< bitcoin-git> [bitcoin] laanwj closed pull request #11565: Make listsinceblock refuse unknown block hash (master...pr/since) https://github.com/bitcoin/bitcoin/pull/11565
< fanquake> #11550 Is also mergeable, unless there are some more backports to be added. A new PR can always be opened later anyways.
< gribble> https://github.com/bitcoin/bitcoin/issues/11550 | [0.15.1] qa: Backports by MarcoFalke · Pull Request #11550 · bitcoin/bitcoin · GitHub
< wumpus> I was confused about 0.15.1 versus there
< fanquake> Should it make a difference if it's ending up in the 0.15 branch?
< wumpus> so if they are 0.15.1 backports they shouldn't go in yet
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.15: https://github.com/bitcoin/bitcoin/commit/7d4546f17dca84928bd2b6d1e2588b673c237321
< bitcoin-git> bitcoin/0.15 7d4546f Russell Yanofsky: Make listsinceblock refuse unknown block hash...
< wumpus> I don't know - usually not, I guess
< wumpus> unless it's segwit wallet functionality then it shoudl be held up
< fanquake> wumpus yea ok, it can wait then.
< wumpus> it's just testing stuff so as it passes it can get in already
< fanquake> Don't think it will create any rebase issues either
< wumpus> if anything it makes rebasing easier by making 0.15 more like master :)
< bitcoin-git> [bitcoin] laanwj closed pull request #11550: [0.15.1] qa: Backports (0.15...Mf1710-0151qaBackports) https://github.com/bitcoin/bitcoin/pull/11550
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e1f6a2a801a0...c95832da87ac
< bitcoin-git> bitcoin/master f927ee1 Christian Gentry: Fixed a couple small grammatical errors....
< bitcoin-git> bitcoin/master c95832d Wladimir J. van der Laan: Merge #11571: Fixed a couple small grammatical errors....
< bitcoin-git> [bitcoin] laanwj closed pull request #11571: Fixed a couple small grammatical errors. (master...patch-1) https://github.com/bitcoin/bitcoin/pull/11571
< fanquake> Just checking for anything else that is ready to go.
< fanquake> Maybe #11511 , I feel like we are going in circles a bit with the EXIT_CODE stuff though.
< gribble> https://github.com/bitcoin/bitcoin/issues/11511 | [Init] Remove redundant exit(EXIT_FAILURE) instances and replace with return false by donaloconnor · Pull Request #11511 · bitcoin/bitcoin · GitHub
< fanquake> They seem to get added, then removed, then added again.
< wumpus> fanquake: yeah same feeling here, it didn't occur to me as terribly relevant
< wumpus> but as it has ACKs, I"m fine with merging it
< luke-jr> ‎[12:09:14] ‎<‎promag‎>‎ ah, but how do you do that with widgets? horizontal layout with label + combo + label? <-- yes, autogenerated from the (post-translated) strign
< bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/c95832da87ac...db2f83ed463b
< bitcoin-git> bitcoin/master b296bf1 donaloconnor: Init: Remove redundant exit(EXIT_FAILURE) instances and replace with return false
< bitcoin-git> bitcoin/master db2f83e Wladimir J. van der Laan: Merge #11511: [Init] Remove redundant exit(EXIT_FAILURE) instances and replace with return false...
< bitcoin-git> [bitcoin] practicalswift opened pull request #11591: wallet: Add missing cs_wallet locks in GetAvailableCredit/GetAvailableWatchOnlyCredit/CreateWalletFromFile (master...cs_wallet) https://github.com/bitcoin/bitcoin/pull/11591
< bitcoin-git> [bitcoin] laanwj closed pull request #11511: [Init] Remove redundant exit(EXIT_FAILURE) instances and replace with return false (master...161017_refactor_AppInit) https://github.com/bitcoin/bitcoin/pull/11511
< fanquake> Seems like the windows instructions need updating again already. Should we just be providing instructions for the latest release?
< fanquake> Having multiple different sets of instructions just for enabling the same feature seems un-ideal
< fanquake> I'm also not a great fan of 11438, seems like it just opens up lots of opportunities for miscompilation
< fanquake> Although it has been simplified quite a bit now.
< bitcoin-git> [bitcoin] laanwj pushed 7 new commits to master: https://github.com/bitcoin/bitcoin/compare/db2f83ed463b...cffa5ee132f3
< bitcoin-git> bitcoin/master 3b4ac43 Matt Corallo: Rewrite p2p-acceptblock in preparation for slight behavior changes...
< bitcoin-git> bitcoin/master 3d9c70c Matt Corallo: Stop always storing blocks from whitelisted peers...
< bitcoin-git> bitcoin/master 932f118 Matt Corallo: Accept unrequested blocks with work equal to our tip...
< bitcoin-git> [bitcoin] laanwj closed pull request #11531: Check that new headers are not a descendant of an invalid block (more effeciently) (master...2017-10-cache-invalid-indexes) https://github.com/bitcoin/bitcoin/pull/11531
< BlueMatt> yeesh, can I get some postumous acks on that one ^
< LumberCartel> That looks like a good idea.
< wumpus> fanquake: yeah... tend to agree one maintained way that works is better than 10 different ways different people prefer that work a bit
< wumpus> I've long since given up getting people to agree, the only request I make is that cross-building for windows on 16.04 should be heavily discouraged because of the stack protector issue
< wumpus> it's potentially very dangerous
< wumpus> but in any case it results in broken executables so itt's a waste of time
< LumberCartel> I should clarify -- I was commenting that 11531 looks like a good idea. I think that improving ways to reduce or eliminate the effective of DoS attacks always a good idea.
< wumpus> yes, it is (if it works, please help review the code)
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #11592: [qa] Fix silent merge conflict (assert_raises_rpc_error) (0.15...Mf1711-qa01502) https://github.com/bitcoin/bitcoin/pull/11592
< xinxi> Will the US regulate Bitcoin development while it’s going to regulate Bitcoin derivatives?
< xinxi> What do you guys think?
< wumpus> no political discussion here please, use #bitcoin instead
< karelb> is there ETA for
< karelb> not exact date, just if it will be this week or before 2x at all etc (yeah I know ETA questions are annoying :))
< sipa> karelb: "soon"
< sdaftuar> cfields: i just commented in #11560 -- i'd like to undo the change to looping on mapNodeState vs ForEachNode
< gribble> https://github.com/bitcoin/bitcoin/issues/11560 | Connect to a new outbound peer if our tip is stale by sdaftuar · Pull Request #11560 · bitcoin/bitcoin · GitHub
< sdaftuar> matt pointed out to me that ForNode has to walk vNodes -- so looping on mapNodeState makes that loop n^2, instead of n log n
< cfields> sdaftuar: well you'll have to hold cs_vNodes for that to be safe...
< sdaftuar> ForEachNode already holds cs_vNodes right?
< cfields> or you mean doing the whole thing in ForEachNode ?
< sdaftuar> yeah i wanted to undo the change from ForEachNode -> loop on mapNodeState
< cfields> do you have the old commit handy?
< sdaftuar> yep
< sdaftuar> unsquashed branch is here: https://github.com/sdaftuar/bitcoin/commits/11560.1
< sdaftuar> it occurred to me that maybe i should check that CNodeState is not null in there
< sdaftuar> if i'm using ForEachNode i mean
< cfields> oh, i see what you mean. I thought you were talking about the _other_ lookup in ForNode below
< sdaftuar> ah, no i'm not worried about that, that's just O(n)
< sdaftuar> i'm just concerned about the n^2
< cfields> yea, ok
< sdaftuar> great thanks
< cfields> sdaftuar: fwiw, I was hoping to do something like this: https://github.com/theuni/bitcoin/commit/0693ffb8ed386095a27a8ca62a7eacada2e9db04
< cfields> (after the upcoming libevent changes, the fDisconnect status doesn't really matter since nodes are disconnected almost instantly. So we will want to drop those checks anyway.
< cfields> so that just leaves the CNodeState checks in that first test
< sdaftuar> ah, interesting
< sdaftuar> so we'd drop having to look into the cnode at all?
< * sdaftuar> -> lunch
< cfields> yea, we'd just connman->Disconnect(id)
< cfields> well, we'd also have to check for connection time > 30 sec. That's why I was suggesting that we make the timeout equal to the handshake, then we wouldn't have to do that either.
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/cffa5ee132f3...1b8c88451b05
< bitcoin-git> bitcoin/master 5d465e3 Tomas van der Wansem: Ensure backupwallet fails when attempting to backup to source file...
< bitcoin-git> bitcoin/master 1b8c884 MarcoFalke: Merge #11376: Ensure backupwallet fails when attempting to backup to source file...
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #11376: Ensure backupwallet fails when attempting to backup to source file (master...core) https://github.com/bitcoin/bitcoin/pull/11376
< sipa> sdaftuar: were you going to adapt the approach to use an incrementable semaphore instead of a separate bool?
< sipa> or rather do that in a post- improvement
< sdaftuar> sipa: i'm not that familiar with the semaphore implementation so i was going to hold off on that
< sipa> okay
< sdaftuar> also i think there was some preference (at least from matt, not sure about others) for not having both a feeler and a 9th connection out at the same time
< sdaftuar> i don't feel strongly about that, but that would be a reason in favor of a bool vs incrementable semaphore i think?
< sdaftuar> cfields: i'm not following how you could drop the connection time > 30 second check?
< sdaftuar> cfields: specifically i had greg's same question about how you avoid disconnecting a peer that you just connected to
< cfields> sdaftuar: you'd still check, but it could be done in CConnman. Changing to 60sec would just mean that CConnman's notion of earliest-possible-timeout (these change to libevent timers) wouldn't change
< cfields> I was getting ahead of myself, I guess that comment makes no sense without the future changes in mind
< sipa> BlueMatt: how did the invalidateblock code result in failure for pruned nodes?
< sipa> (just trying to understand)
< sdaftuar> sipa: i think the issue is if you call invalidateblock on a blockhash that you've pruned
< BlueMatt> yea, it'll walk backwards, and set the invalid flag BEFORE disconnecting it
< BlueMatt> then when you go to disconnect, you will fail to read from disk and shut down
< BlueMatt> (AbortNode, I believe) but now when you start up no block meets the conditions for inclusion in setBlockIndexCandidates
< BlueMatt> because there are no blocks that are either a) our tip and valid or b) at least as much work as our tip and have data
< BlueMatt> so you hit the !setBlockIndexCandidates.empty() assert
< sipa> BlueMatt: i see!
< sipa> would there be a problem with marking the blocks invalid after disconnecting them, but one by one?
< sipa> as opposed to after the whole invalidated
< sipa> brb, meeting
< BlueMatt> sipa: not 100% sure. you'd definitely fail CheckBlockIndex, but it may actually be fine...you'd end up with things in your mapBlockIndex upon restart that have BLOCK_FAILED_CHILD but who's parents are IsValid, which is nonsense
< BlueMatt> it might, however, very likely not actually break anything
< BlueMatt> still, I *really* didnt want to try to think through that in the context of the g_failed_blocks stuff
< sipa> anyone, no objection to the change that was merged
< sipa> *anyway
< BlueMatt> I take that back, CheckBlockIndex does *not* verify this, but, again, its nonsense
< sipa> BlueMatt: i've for a while regretted the choice of having validity flags inside the block index
< BlueMatt> I mean they are a pretty separate concept
< sipa> your PR makes me wonder if we couldn't get rid of them entirely
< sipa> valid = chainActive
< BlueMatt> I mean we have to have them somewhere
< sipa> invalid = in g_failed_blocks
< BlueMatt> no, we need a concept of "descends from invalid"
< BlueMatt> I mean that kinda sucks cause it grows unbounded
< BlueMatt> though not the end of the world, I suppose
< sipa> it's at most a constant factor larger than mapBlockIndex itself
< sipa> a very small constant factor
< gmaxwell> how would thing like invalidateblock / reconsider block work? or pipelined validation with multiple stages of verification?
< sdaftuar> yes but it slows down acceptblockheader
< sdaftuar> you wouldn't want to iterate mapBlockindex just to accept a new header
< sipa> right, you want some form of 'caching' of invalid descendants
< sipa> but perhaps that doesn't need to be mapBlockIndex
< sipa> anyway, just a wild idea
< gmaxwell> it is kind of silly to have an extra field for something that is almost always just in a single state.
< BlueMatt> yea, I mean it is true that we barely use nStatus for anything except for HAVE_DATA
< sipa> and it doesn't even cover the whole thing
< BlueMatt> (which I guess is implied by the diskpos thing)
< sipa> we use nChainTx as indicator for "all parent blocks downloaded"
< BlueMatt> yea, I mean we do need VALID_SCRIPTS for net_processing :/
< BlueMatt> cause we're not allowed to send stuff unless its VALID_SCRIPTS
< bitcoin-git> [bitcoin] theuni opened pull request #11593: rpc: work-around an upstream libevent bug (master...fix-libevent-cb) https://github.com/bitcoin/bitcoin/pull/11593
< BlueMatt> cfields: travis failed on ^ (libevent in the travis os is 2.0, looks like, which does not have the getcb function)
< BlueMatt> only 2.1 has it
< cfields> ugh
< cfields> ok, working on it
< MarcoFalke> wumpus: cfields: I was trying to prepare the release but I am stuck at deciding whether to backport #10756.
< gribble> https://github.com/bitcoin/bitcoin/issues/10756 | net processing: swap out signals for an interface class by theuni · Pull Request #10756 · bitcoin/bitcoin · GitHub
< MarcoFalke> some of the commits depend on it
< MarcoFalke> It might be shorter (with regard to LOC) to solve the merge conflicts manually
< MarcoFalke> But I'd wanted to see what you prefer
< cfields> MarcoFalke: i don't think that should be needed. What are the conflicts?
< MarcoFalke> In function ‘bool SendMessages(CNode*, CConnman&, const std::atomic<bool>&)’: error: ‘ConsiderEviction’ was not declared in this scope
< cfields> ah right, new members
< cfields> sec
< cfields> MarcoFalke: blah, backporting 10756 may be easier, as more and more commits build on it
< cfields> MarcoFalke: it's either that, or break the new functions (like ConsiderEviction) global/static, and pass in the CConnman pointer
< MarcoFalke> Jup, figured that
< MarcoFalke> Since the not-yet-merged #11560 also needs it, I will try to cherry-pick your refactoring commits
< gribble> https://github.com/bitcoin/bitcoin/issues/11560 | Connect to a new outbound peer if our tip is stale by sdaftuar · Pull Request #11560 · bitcoin/bitcoin · GitHub
< ghost43> I would be interested in the exact semantics of nLockTime and chainActive.Height() here https://github.com/bitcoin/bitcoin/blob/5b059a833eb57a4dd8458f42aedba85265b2bbf3/src/wallet/wallet.cpp#L2661 -- does this line ensure that only the next block contain the tx? should it be height+1? at the same time I just tested and bitcoind rejects transactions with nLockTime for the next block. so does nLockTime mean that the block to contain a tx
< ghost43> has to have a height of nLockTime+1?
< cfields> BlueMatt: pushed a new version that mimics the upstream fix
< BlueMatt> thanks! will review in a few minutes