< bitcoin-git> [bitcoin] fanquake opened pull request #17169: doc: correct function name in ReportHardwareRand() (master...random_doc_inithardwarerand) https://github.com/bitcoin/bitcoin/pull/17169
< meshcollider> Ok so regarding #16341, who is currently reviewing or intending to review? promag mentioned that he was going to yesterday. Just to get a rough idea. jnewbery / sipa are either of you planning to?
< gribble> https://github.com/bitcoin/bitcoin/issues/16341 | Introduce ScriptPubKeyMan interface and use it for key and script management (aka wallet boxes) by achow101 . Pull Request #16341 . bitcoin/bitcoin . GitHub
< sipa> definitely plannimg to
< jeremyrubin> There aren't RPCs which support raw scripts are there?
< sipa> what does that mean?
< jeremyrubin> e.g., in sendmany you pass a associative list of addresses and values
< sipa> decodescript certainly does, in a way
< jeremyrubin> but I guess there's not a decent way to make a transaction to a raw piece of ASM
< sipa> no, asm is ambiguous even
< sipa> you can't convert it back to script
< jeremyrubin> err hexstr
< sipa> with a lot of imagination, sendrawtransaction/fundrawtransacrion/... support that
< sipa> but that's it
< jeremyrubin> hence 'decent'
< promag> meshcollider: yes, I'm reviewing that one.
< meshcollider> ?
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/46d6930f8c7b...4f42284fc0c5
< bitcoin-git> bitcoin/master f59bbb6 Jim Posen: test: Fix bug in blockfilter_index_tests.
< bitcoin-git> bitcoin/master 4f42284 MarcoFalke: Merge #17140: test: Fix bug in blockfilter_index_tests.
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #17140: test: Fix bug in blockfilter_index_tests. (master...fix-index-test) https://github.com/bitcoin/bitcoin/pull/17140
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/4f42284fc0c5...fcf1ebde3db9
< bitcoin-git> bitcoin/master 5013171 fanquake: doc: correct function name in ReportHardwareRand()
< bitcoin-git> bitcoin/master fcf1ebd MarcoFalke: Merge #17169: doc: correct function name in ReportHardwareRand()
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #17169: doc: correct function name in ReportHardwareRand() (master...random_doc_inithardwarerand) https://github.com/bitcoin/bitcoin/pull/17169
< bitcoin-git> [bitcoin] ch4ot1c opened pull request #17172: doc: Update list of valid PR areas (master...doc/pr-areas) https://github.com/bitcoin/bitcoin/pull/17172
< bitcoin-git> [bitcoin] laanwj pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/fcf1ebde3db9...048e456fc408
< bitcoin-git> bitcoin/master 85016e5 Andrew Toth: [rpc] Fix broken bitcoin-cli examples
< bitcoin-git> bitcoin/master 048e456 Wladimir J. van der Laan: Merge #17119: doc: Fix broken bitcoin-cli examples
< bitcoin-git> [bitcoin] laanwj merged pull request #17119: doc: Fix broken bitcoin-cli examples (master...example-fixes) https://github.com/bitcoin/bitcoin/pull/17119
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/048e456fc408...ec3ed5a44878
< bitcoin-git> bitcoin/master 1f6c650 Sjors Provoost: travis: run tests on macOS native
< bitcoin-git> bitcoin/master ec3ed5a MarcoFalke: Merge #16597: Travis: run full test suite on native macOS
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #16597: Travis: run full test suite on native macOS (master...2019/08/travis-macos) https://github.com/bitcoin/bitcoin/pull/16597
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #17176: ci: Cleanup macOS runs (master...1910-ciMac) https://github.com/bitcoin/bitcoin/pull/17176
< bitcoin-git> [bitcoin] fjahr opened pull request #17177: doc: Describe log files + consistent paths in test READMEs (master...pr15830) https://github.com/bitcoin/bitcoin/pull/17177
< sipa> jonasschnelli: where is your latest v2 p2p protocol draft?
< sipa> gleb: re #17163... are you sure that no lightweight clients do their own IP address management? maybe they don't currently, but that sounds like a bug
< gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs . Pull Request #17163 . bitcoin/bitcoin . GitHub
< wumpus> sipa: I also wasn't sure the tight coupling between lightweight validation and no IP address management, but apparently it's an accepted thing
< wumpus> tbh, I think everyone that cares about decent address management gave up on SPV nodes long ago
< wumpus> so it's not wrong, but it wouldn't have to be a necessary correlation
< gleb> sipa: This was my intuition. Maybe, we should allow them to ask (we can discuss that), but we certainly don't want to rely on them when we choose 1/2 peers to forward a <10 addr message.
< sipa> gleb: agree there; i commented on your PR
< gleb> Let's ask others at the meeting in an hour :) Should be pretty quick.
< gleb> Another idea would be to add a service flag? Sounds reasonable to me protocol-wise, but I'm not sure I have the right intuition.
< wumpus> you want a way to be able to signify that nodes don't want to receive addr messages at all?
< bitcoin-git> [bitcoin] fanquake closed pull request #17172: doc: Update list of valid PR areas (master...doc/pr-areas) https://github.com/bitcoin/bitcoin/pull/17172
< wumpus> might be able to smuggle that into addrv2 somehow; it has a proposed message for a peer to signify support for v2 addrsses, maybe it could signify 'no addr support' as well
< luke-jr> maybe there should be a node mode that *only* relays addr messages, and sipa could only return those with his DNS seed :P
< fanquake> rex4539 This might be the error message you were talking about the other day #17179
< gribble> https://github.com/bitcoin/bitcoin/issues/17179 | macos: shutdown on first run due to -psn_ parameter . Issue #17179 . bitcoin/bitcoin . GitHub
< rex4539> Yes, this was it :)
< fanquake> Good to know at least 1 other person has been (clean) testing rc1 on macOS
< wumpus> why does this happen with rc1 though?
< wumpus> and not with other versions
< luke-jr> another Qt thing?
< luke-jr> like the duplicate URIs
< fanquake> It might be because we are doing more strict argument passing now? It's also only happens when you run Bitcoin Core for the very first time
< wumpus> oh, maybe Qt filtered it out before
< wumpus> right
< wumpus> Qt's argument handling probebly kind of shielded us from platform specific craziness like this
< fanquake> heh, so qt does filter it out yes
< fanquake> from qtbase: "// eat "-psn_xxxx" on Mac, which is passed when starting an app from Finder."
< wumpus> like the files handling
< luke-jr> fanquake: is it near the duplicate URI filtering code? maybe someone should go through all that
< fanquake> Will link to the tree in a sec
< wumpus> everyone on every OS now has to suffer basically because of a buggy browser issue on windows :-)
< luke-jr> so we should chdir too..?
< luke-jr> or do we even care
< fanquake> I don't think we care. I'm not sure why they are changing directory code was added. The commit is from 7 years ago, and doesn't have much detail. However they were doing the psn filtering before that was added.
< gleb> luke-jr: jokes aside, I was considering separating addr-relay from tx-relay and block-relay. Because I think topology leakage is cumulative, haha. But that requires a lot of analysis.
< bitcoin-git> [bitcoin] JeremyCrookshank opened pull request #17180: gui: Improved wording/explanation of Bitcoin sends amount box (master...sendamounttooltip) https://github.com/bitcoin/bitcoin/pull/17180
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/ec3ed5a44878...88eff969c201
< bitcoin-git> bitcoin/master 9576614 Martin Erlandsson: doc: Describe log files + consistent paths in test READMEs
< bitcoin-git> bitcoin/master 88eff96 MarcoFalke: Merge #17177: doc: Describe log files + consistent paths in test READMEs
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #17177: doc: Describe log files + consistent paths in test READMEs (master...pr15830) https://github.com/bitcoin/bitcoin/pull/17177
< warren> I'm here for the weekly meeting, that's in 20 minutes?
< wumpus> yes
< dongcarl> gleb: Not 100% sure what you mean in your latest comment on #17163
< gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs . Pull Request #17163 . bitcoin/bitcoin . GitHub
< dongcarl> "Older nodes are very likely to forward their <10 addr messages in the blackhole" because they might forward to newer nodes that will ignore addr messages from incoming connections like sipa described?
< gleb> Yes
< bitcoin-git> [bitcoin] fanquake pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/88eff969c201...4daadce36cfe
< bitcoin-git> bitcoin/master fa04673 MarcoFalke: chain: Set all CBlockIndex members to null, remove SetNull helper
< bitcoin-git> bitcoin/master 4daadce fanquake: Merge #17162: chain: Remove CBlockIndex::SetNull helper
< bitcoin-git> [bitcoin] fanquake merged pull request #17162: chain: Remove CBlockIndex::SetNull helper (master...1910-docChainLocks) https://github.com/bitcoin/bitcoin/pull/17162
< dongcarl> gleb: "it's very likely that this "stem" will end up at a private node very fast (just graph-wise), and it will drop it on the floor" private node being a node with only outgoing connections?
< gleb> dongcarl: also yes.
< dongcarl> gleb: if this private node only has outgoing connections, why would it drop anything on the floor?
< gleb> Because I assume that if we enforce a rule "don't accept addr from inbounds", then this private node won't even try to relay it further (as the receiver will discard it)
< gleb> .
< dongcarl> gleb: oh I see...
< wumpus> #startmeeting
< lightningbot> Meeting started Thu Oct 17 19:00:31 2019 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< jnewbery> hi
< warren> hi
< kanzure> hi
< gleb> hi
< moneyball> hi
< wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb moneyball kvaciral
< jonasschnelli> hi
< fanquake> hi
< dongcarl> hi
< jeremyrubin> hi
< wumpus> one proposed topic for today: taproot proposal next steps; possible review sessions (moneyball)
< sipa> hi
< moneyball> wumpus: i assume we'll do high priority list first?
< aj> hola
< promag> hi
< wumpus> moneyball: yes, let's start with that (I wait a bit in case people have additional last minute topics to propose)
< wumpus> #topic High priority for review
< achow101> hi
< promag> please add 17135
< wumpus> #17135
< gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag . Pull Request #17135 . bitcoin/bitcoin . GitHub
< fanquake> promag: can you fill out the description in that PR? Why is it high-prio?
< wumpus> promag: ok done
< promag> fanquake: IMO they are necessary for 0.19, also #16963 #17120
< promag> ty
< gribble> https://github.com/bitcoin/bitcoin/issues/16963 | wallet: Fix unique_ptr usage in boost::signals2 by promag . Pull Request #16963 . bitcoin/bitcoin . GitHub
< wumpus> yes, would be nice if you also say what it is blocking
< gribble> https://github.com/bitcoin/bitcoin/issues/17120 | gui: Fix start timer from non QThread by promag . Pull Request #17120 . bitcoin/bitcoin . GitHub
< wumpus> seems to big a change for 0.19.0 at least
< wumpus> 0.19.1 maybe
< promag> I'll update description sure
< fanquake> I'll re-review 16963
< fanquake> I'm not sure there's too much else to discuss high-prio wise other than hopefully people are doing some rc1 testing
< promag> but it's meant to fix #17112
< gribble> https://github.com/bitcoin/bitcoin/issues/17112 | v0.19.0rc1 GUI repeatedly not responding . Issue #17112 . bitcoin/bitcoin . GitHub
< fanquake> Maybe jump into the taproot discussion?
< wumpus> promag: that's just too deep a rabbit hole to fix before the release, is it really a reversion in 0.19?
< promag> yes
< wumpus> where did it happen then?
< wumpus> ok
< wumpus> I still think adding the GUI async stuff last minute is risky
< sipa> is there a simpler way to fix the issue itself?
< promag> well it's still a HP decision
< sipa> harry potter? health points? hewlett-packard?
< wumpus> I mean it's the obvious thing to do long term but between rcs?
< promag> heh, high prio
< sipa> oh lol, sorry, that should have been obvious :)
< promag> wumpus: it's fine if we postpone the that change, but let's decide that then :p
< aj> gui hangs would cause a lot of user complaints though?
< wumpus> yes, but we've always have lots of GUI hangs during initial sync
< jonasschnelli> Yes. This has been there for a long time
< wumpus> this lock just makes it slightly worse I guess
< jonasschnelli> No need for risky fixes in rc-timeframe
< sipa> if it's a visible regression it seems reasonable to fix it, but is it possible to just say undo the change that caused it rather than fixing the root issue?
< jonasschnelli> Better do proper fixes in 0.20
< wumpus> but this is always been a problem and I don't think we're going to solve it for 0.19.0
< wumpus> can we fix it by reverting something instead?
< promag> Jackielove4u: tested that PR in win/linux/macos and compared to 0.18
< wumpus> (on the 0.19 branch I mean)
< promag> 0.19rc is really bad in that regards
< wumpus> instead of adding more code
< jonasschnelli> Its not an easy fix
< jonasschnelli> It requires conceptual changes
< wumpus> no, it's not an easy fix, it's changing things in a very differnt place than where the problem was introduced
< sipa> jonasschnelli: well there was a specific PR that caused it, or at least worsened it
< promag> yeah revert is another option I guess, MarcoFalke_?
< sipa> i'd like to know if we can revert that PR instead
< wumpus> so this basically means we're delaying 0.19.0 indefnintely
< jonasschnelli> sipa: maybe a part of it. But GUI unresponsivenes was always an issue
< wumpus> until the GUI is async
< jonasschnelli> The GUI was not created to be async
< sipa> jonasschnelli: i'm very much aware; but i'm not talking about fixing the root issue, only the regression
< wumpus> which is not soemthing I can get behind, sorry
< promag> BTW, the same approach is already used in WalletController
< jonasschnelli> sipa: sure. If we can fix the regression in a sane way we may want it in 0.19. But unclear of 17120 fixes that
< jonasschnelli> s/of/if
< sipa> jonasschnelli: i'm literally suggesting reverting the PR that caused it, nothing more
< fanquake> so revert #14193 ?
< wumpus> that would be ok with me
< gribble> https://github.com/bitcoin/bitcoin/issues/14193 | validation: Add missing mempool locks by MarcoFalke . Pull Request #14193 . bitcoin/bitcoin . GitHub
< valwal> hi
< promag> ok, I'll revert it in 0.19 and see how it goes
< fanquake> Does that not mean we are back with whatever problems that was meant to fix?
< wumpus> what we're arguing against is making changes to GUI asynchronicity before the 0.19.0 release, I think if it's possible to revert the locking change that caused it in the first place that's a more direct and safer option
< jonasschnelli> Yes
< wumpus> but #17135 adds an extra thread, that's not a trivial fix
< gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag . Pull Request #17135 . bitcoin/bitcoin . GitHub
< wumpus> all we know it might make locking issues worse somewhere
< jonasschnelli> Reverting #14193 (merged in july, invasive) may be not super easy
< gribble> https://github.com/bitcoin/bitcoin/issues/14193 | validation: Add missing mempool locks by MarcoFalke . Pull Request #14193 . bitcoin/bitcoin . GitHub
< fanquake> MarcoFalke: Had you seen inconsistent mempool reads prior to 14193?
< wumpus> which is okay for a release schedule for 0.20.0 where there's a long time to go before the actual release, but not something to do last minute
< promag> I'll just submit the revert commit to 0.19, ask for gitian build and let's see how testing goes
< wumpus> promag: thanks!
< jonasschnelli> promag: nice!
< achow101> could just add a known issue to the release notes and tell people to not mess around in the gui during IBD?
< wumpus> achow101: that would be the fallback then, yes
< promag> achow101: I think it's not just ibd - not sure
< achow101> i would be somewhat surprised if people were actually doing things in the gui during ibd
< jonasschnelli> achow101: I guess the UX stopper is, if someone wants to get an address or so while he is syncing the last 2-3 days
< wumpus> not only IBD but also when you catch up after having not run it for a while
< jonasschnelli> (laptop users, etc.)
< wumpus> that's usually the most annoying time for it to happen
< jonasschnelli> indeed
< wumpus> espeiclaly when you want to send a transaction quickly
< promag> the problem is more evident in windows because windows show that app is not responding that can cause some frustration
< sipa> #17135 does not look crazy, and may be worth considered... but my preference is reverting
< gribble> https://github.com/bitcoin/bitcoin/issues/17135 | gui: Make polling in ClientModel asynchronous by promag . Pull Request #17135 . bitcoin/bitcoin . GitHub
< wumpus> but anyhow this is not new, and can't be solved before 0.19.0
< wumpus> sipa: it's not crazy but it does add a new thread
< jonasschnelli> I tested 17135 a bit and I still had freezes. It's better but not solved.
< wumpus> I think that's a big change between rcs
< sipa> wumpus: agree it's a big change to do at this stage
< jonasschnelli> Lets aim for the mempool locks revert and add more fixes in 0.20
< promag> jonasschnelli: I'd love to know more about that :D
< wumpus> and... it's annoying but not a crash issue
< achow101> would reverting the locks cause other problems?
< promag> wumpus: yeah but people can force kill upon these hangs
< jonasschnelli> promag: yeah. Currently setting up a test env to better reproduce those locks
< achow101> I guess it wouldn't really be a regression..
< * jonasschnelli> wishes the GUI would just work via RPC
< warren> deadlock can happen in headless bitcoind?
< sipa> warren: this discussion is about Qt GUI only
< warren> ok thx
< promag> next topic?
< fanquake> cc moneyball
< moneyball> Hi
< jnewbery> MarcoFalke has been responding but his messages aren't getting through
< wumpus> #topic Taproot proposal (moneyball)
< moneyball> aj, harding, and i have been discussing organizing a review of the schnorr/taproot/tapscript proposals
< wumpus> jnewbery: oh no :( is he logged in to nickserv
< moneyball> it would be a 7 week series ending by end of year
< promag> jnewbery: MarcoFalke_ been there too :(
< wumpus> it looks like he isn't logged in-- you can't post here in that case, and worse, you don't really get feedback--I had the same problem some meetings ago
< moneyball> i raise it here because (a) create awareness (b) solicit feedback on the idea (c) solicit ideas for how to invite high quality broad reviewers
< moneyball> i'm thinking we will communicate this on bitcoin-dev, lightning-dev, optech newsletters + slack to member companies
< sipa> sounds like a great idea to get feedback
< moneyball> the goal of this is to generate high quality feedback, give us further confidence in the proposal, give the Core project confidence to proceed with code review and QA of the existing PR, and to improve the decentralization / broader participation of review of the proposal
< sipa> there is no PR
< moneyball> sorry, branch
< aj> to proceed with getting an implementation that we can PR :)
< sipa> yeah
< MarcoFalke> test
< jnewbery> (b) I think it's a good idea
< sipa> MarcoFalke: reading you loud and clear
< aj> MarcoFalke: success
< MarcoFalke> I was wondering why no one responeded to me for days
< jonasschnelli> heh
< sipa> oww
< fanquake> moneyball sounds good. I see some australian friendly time slots as well
< jnewbery> welcome back, MarcoFalke
< wumpus> oh crap
< promag> lol
< achow101> moneyball: is the idea to kind of do it in the style of the pr review club?
< wumpus> kafkaIRC
< sipa> wumpus: IRC where only some people see what certain others are saying... sounds like twitter to me
< moneyball> achow101: aj has proposed a small group study structure then come together as a larger group once a week via IRC for Q&A (see the doc for details)
< MarcoFalke> I think the issue was the znc put an underscore behind my name and bitcoin-core-dev only allows logged in users to post?
< MarcoFalke> (sorry ot)
< moneyball> it will definitely require diligence and commitment, but i think it is necessary for a high quality review of the proposal
< wumpus> sipa: yes it seems the same kind of shadowban idea
< wumpus> MarcoFalke: yes, it allows only logged in users to post, though you can log in while having an alternative nick (by using /msg nickserv login <registered_user> <pass> AFAIK instead of just /... login <PASS>)
< moneyball> "once a week" = twice a week at different times to provide good global coverage
< wumpus> MarcoFalke: the crazy thing is that freenode doesn't seem to send an error message anymore in that case
< sipa> 7 weeks seems like a lot, but i also agree it's not something that can be done in 1-2 hours
< jeremyrubin> i think it would make sense to talk about deployment/acceptance criteria
< wumpus> MarcoFalke: the reason for "only registered users can post" is for some spam avoidance, it used to be really bad at some point, maybe it's no longer necessary I don't know
< sipa> jeremyrubin: imho that's an entirely different discussion; we first need to know if the ecosystem is on board including all the details it entails (to the extent they care), then we can talk about activation
< warren> MarcoFalke: strongly recommend figuring out the nickserv SASL or TLS cert authentication, then you'll never connect without login
< wumpus> warren: somehow it did happen to me once
< achow101> moneyball: who are the participants? open to public?
< warren> wumpus: yeah server splits or other weird conditions
< jeremyrubin> sipa: thats what i said kinda
< jeremyrubin> "we first need to know if the ecosystem is on board" == acceptance criteria
< jeremyrubin> knowing what that means, i agree, is a separate discussion
< moneyball> achow101: open to public. some level of moderation may be needed just to keep folks on track and avoid trolls. i mention above the proposed method of outreach. if others have ideas let me know.
< jeremyrubin> but an impt one for taproot and other stuff too, separate from the actual instance
< aj> sipa: i don't think shrinking it below 5 "sessions" works, and multiple sessions a week seems a bit intense, 7 weeks seems the max before hitting xmas/new year, and 5 + 2 weeks padding seems okayish
< sipa> jeremyrubin: i have no idea what you're trying to say
< sipa> aj: ok
< jeremyrubin> ok
< jeremyrubin> can chat out of band later
< sipa> ok
< moneyball> also feel free to comment directly in the doc if you have ideas or concerns
< moneyball> thanks everyone! hope to see many of you participate. it is a fairly large commitment but as aj points out in the doc:
< wumpus> thanks!
< sipa> sgtm
< MarcoFalke> #proposedmeetingtopic address relay and spv clients
< gleb> I wanted to mention that in #17163 we're discussion whether we should stop relaying addresses to light clients or just limit relay to the cases where it does not hurt addr propagation.
< gleb> It's not clear for us whether SPV should sync their addr database with random peers from the network. Input welcome!
< gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs . Pull Request #17163 . bitcoin/bitcoin . GitHub
< gleb> I guess we tend to "allowing SPV to ask for addresses does not hurt", so the latter, but I'd like to hear more opinions.
< MarcoFalke> I guess we switched topics?
< MarcoFalke> #topic address relay and spv clients
< gleb> To be clear: currently when we get a short ADDR message (less than 10), we choose 1 or 2 peers to relay forward. That's the primary way to announce new nodes in the network. Currently it's possible that we choose SPV which will throw it on the floor, which is unfortunate.
< aj> how about just biassing against picking peers for the 1 or 2 to relay for if those peers haven't sent us ADDR messages already, if that makes sense?
< achow101> I think it would hurt SPV clients to not receive addr messages
< achow101> *for them to not receive addr messages
< gleb> I also mentioned above that an explicit service flag is maybe a good idea, to decouple addr-relay from SPV/non-SPV reasoning.
< gleb> Or whatever other explicit way you can imagine. aj: Biasing is sort of that, but implicit.
< gleb> Anyway, just wanted to mention that this discussion is taking place in #17163. Let's continue there. Thanks.
< gribble> https://github.com/bitcoin/bitcoin/issues/17163 | p2p: Avoid relaying ADDR messages to SPV clients by naumenkogs . Pull Request #17163 . bitcoin/bitcoin . GitHub
< achow101> gleb: are you certain that spv clients discard addr messages?
< MarcoFalke> as mentioned on the pull request, SPV clients should not deal with addr messages. I can only see ways in which they shoot themselves in the foot
< wumpus> maybe most current ones do, but it's not a necessary coupling
< sipa> MarcoFalke: i don't understand that comment
< sipa> and i don't understand what SPV has to do with it
< MarcoFalke> Though, there might be a node without NODE_NETWORK set that wants addr messages
< wumpus> I don't get why SPV clients couldn't implement full address management instead of blindly trusting DNS seeders
< MarcoFalke> So that coupling doesn't make sense
< achow101> i don't understand what about spv makes it such that they don't need addrs
< wumpus> right
< MarcoFalke> wumpus: I doubt implementing address management is trivial
< MarcoFalke> See for example feeler connections
< wumpus> no one is talking about 'trivial'
< sipa> MarcoFalke: i'm sure it's nontrivial, but it's nontrivial for everyone
< wumpus> but why couldn't they if they wanted?
< wumpus> so the reasoning is 'SPV implementations tend to be trivial, so they won't implement something as complex as addr handling'
< sipa> i agree partially with luke-jr that it's reasonable for lightweight clients to instead rely on a trusted server... but not all light clients do that; and within those that do, i think doing actual ip address management is far more reasonable that blindly using dns seeds
< wumpus> which more or less makes sense but it's very indirect
< warren> The "throwing to the floor" part is concerning but I'm thinking address relay is best effort. It is reasonable to need to connect to multiple/many peers before you get any quality addresses. There is no guaratee that a peer you try first is honest.
< BlueMatt> what does wasting a service bit cost? we've got a bunch of 'em :p
< achow101> wumpus: but at the same time, they are probably using some library like bitcoinj that already handles it for them (at least I think bitcoinj uses addrs)
< MarcoFalke> would the service bit be for "i want addr messages" or "i send addr messages"
< wumpus> achow101: that's another assumption though, based on current software
< wumpus> achow101: I"m not sure that should determine the protocol
< sipa> i think a service bit makes sense (e.g. explicitly opting out of address relay), but i think that's an independent question of whether we want to bias our own relay away from nodes we assume won't participate in relaying further
< gleb> MarcoFalke: Your peer shouldn't care whether you promise to send them something or not
< MarcoFalke> I also think it makes sense to make this more explicit with service bits or a new message type
< MarcoFalke> Would addrv2 help with that?
< wumpus> it could be added to that
< dongcarl> addrv2 has a message to indicate that I want addrv2 messages
< dongcarl> I'm hearing we want some kind of more complex negotiation?
< wumpus> the same mechanism (say, a new message) for requesting addrv2 messages could be used to request *NO* addr messages
< MarcoFalke> gleb: It is useful to know whether a peer might not relay addr messages, but still wants them
< wumpus> dongcarl: I don't think it would be more complex
< wumpus> just a third option (besides addr and addrv2)
< gleb> MarcoFalke: yeah, if they want, they should signalize. But whether they send us or not -- we don't care.
< gleb> Okay, it seems like I change a PR to avoid forwarding ADDR to SPV, but still allow SPV to ask for addresses.
< warren> +1
< gleb> And then we should expand addrv2 spec for this further change separately.
< sipa> that sounds reasonable
< achow101> how are you determining a node is spv? no node_network?
< gleb> and no node_network_limited.
< aj> (we also drop ADDRs on the floor if they're from a node we've set as block-relay-only per #15759)
< gribble> https://github.com/bitcoin/bitcoin/issues/15759 | p2p: Add 2 outbound block-relay-only connections by sdaftuar . Pull Request #15759 . bitcoin/bitcoin . GitHub
< achow101> that'd affect old pruned nodes tho
< wumpus> aj: oh? block relay only also means no addr relay?
< gleb> Yeah, but we already don't forward short addr to "block_relay_only" :)
< wumpus> aj: I kind of wondered that
< BlueMatt> time
< aj> wumpus: only for the 2 of 10 outbounds when we do tx's but have a couple of block-relay-only nodes
< wumpus> #endmeeting
< lightningbot> Meeting ended Thu Oct 17 20:01:13 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< aj> gleb: but we might pick a node who's chosen us as block_relay_only to forward addresses to
< MarcoFalke> gleb: this ^ for example
< achow101> filter by fRelay
< gleb> aj: In current implementation we won't pick a node which is set to be block_relay_only.
< MarcoFalke> I think we should guess whether a node relays (sends) addr messages based on other (unrelated) service bits or version flags
< aj> gleb: block_relay_only reflects our choice, not our peer's choice
< aj> or fRelay reflects that, i think; bit long since i've looked at it now
< MarcoFalke> gleb: That doesn't mean that in the future there might be a blocks-only-relay-and-addr-relay node
< gleb> It should be per-link, non per-direction, otherwise doesn't make sense.
< MarcoFalke> s/might/won't/
< gleb> Because then transactions will flow through that link, just in one direction :)
< aj> gleb: right, but for -blocksonly you want ADDR but not txs to flow, and you don't know if the other guy's selected you as block_relay_only or is doing -blocksonly in general
< aj> (i think)
< achow101> imo we should do the service bit thing to explicitly ask for or opt out of addr messages. all of the things mentioned don't necessarily exclude needing addrs
< gleb> achow101: Before service bit thing happens, I am suggesting to still allow everybody to learn, but just not forward them <10 addr messages.
< MarcoFalke> Sure, old Bitcoin Core nodes won't set that service bit, so they should be accomodated for and provided with addr messages
< luke-jr> [19:55:54] <MarcoFalke> would the service bit be for "i want addr messages" or "i send addr messages" <-- it doesn't make sense to use service bits for "i want"
< luke-jr> that can very easily be done at connection negotiation
< meshcollider> Oops forgot the meeting
< meshcollider> The taproot thing sounds good to me too
< luke-jr> moneyball: ACK/HOLD isn't clear. If there's minor changes desired, does that need a HOLD?? :/
< aj> luke-jr: some minor changes shouldn't need a HOLD (like "maybe add a reference to https://..." or "might be clearer if X was described before Y") ; and still want to go through github issues/PRs to actually get changes made. i'm thinking of ACK/HOLD as more like the summaries on the Segwit_support wiki page
< aj> luke-jr: (also, a double question-mark character? fancy)
< luke-jr> I haven't checked if my review comments were addressed yet; I wonder if I should do that in advance, or wait for the meetings
< sipa> luke-jr: i never understood your comment about avoiding space savings
< sipa> do you just mean to clarify whether it's about bandwidth or other kinds of savings?
< luke-jr> sipa: I mean weight shouldn't be gamed.
< luke-jr> if a different weight is desirable, the algorithm for weight can be proposed adjusted, but just omitting bytes to manipulate weight isn't a good way
< aj> s/omitting/adding/ ?
< luke-jr> aj: IIRC it was omissions in the earlier draft
< sipa> ?
< sipa> weight = base_size + 3 * witness_size; nothing in taproot ever did anything else
< luke-jr> I'll need to go re-read, sec
< aj> there's "Instead, the taproot annex can be used to add weight to the witness ..." in bip-tapscript; otherwise i've no idea
< sipa> one idea for the "annex" (but not currently proposed) is that it could be used to add a "extra weight" marker, which would then translate to an additional allowance for example for hypothetical future opcodes that have a much higher cpu cost per byte than existing things
< sipa> fwiw, it seems that right now, blocks full of sha256, blocks full of stack operations, or blocks full of signature checks remarkably are all very similar in their cpu cost per byte for verifiers
< luke-jr> sipa: basically, my point is that p2pkh, p2wpkh, and the new taproot equivalent with the same CPU/IO/etc costs should have the same weight, not be tweaked in special-casing ways to reduce it
< sipa> i don't understand
< sipa> we're trying to use block space as efficiently as possible
< sipa> without increasing cpu cost per byte
< sipa> (i'd argue that if it becomes possible to do the same thing with less bytes, but significantly increase how burdensome it is to validation, that would be a bad thing, but i don't think that's the case)
< luke-jr> I don't see how that isn't self-contradicting
< aj> iirc it costs slightly more to send to a taproot address than p2wpkh (because it's a 32B point not a 20B hash) and corresponding less to spend via taproot key path (64B sig, versus 33B key reveal and 72B DER signature), but they average out almost the same in weight
< luke-jr> what does it mean to "increase efficiency" of block space use, without increasing CPU cost per byte?
< sipa> luke-jr: if i can perform a payment on chain with fewer bytes, but also reduce the CPU cost needed to verify it, is it justified that the weight also goes down?
< aj> batch verification decreases the cpu cost per signature, so if that were the only factor more sigs per byte could keep cpu cost stable
< luke-jr> I seem to recall special-casing where something is omitted if it's a particular value.. but I'm having trouble finding it now
< luke-jr> sipa: so Taproot in fact uses less CPU time?
< luke-jr> I would think it uses more
< sipa> luke-jr: if batch validation is implemented, significantly less
< luke-jr> isn't that just per transaction, though? so 1 input wouldn't benefit?
< sipa> you can batch all signatures in a block
< sipa> or even more
< luke-jr> hmm
< sipa> note that batch validation is not the same as aggregation; there is still a signature on chain per logical signature to check; you just have a faster algorithm that can tell you whether all of them are valid or not (which won't tell you which ones are invalid if it fails, but in block validation that's not relevant)
< aj> luke-jr: SIGHASH_ALL signatures are 64 bytes, versus others being 65 bytes; that's the only case like that that comes to mind
< luke-jr> can we fix it so segwit/taproot are on the same weight scale as pre-segwit? or would that need to be another BIP? because right now, the weights are too low
< luke-jr> (this is admittedly a different issue beyond merely not making it worse)
< luke-jr> aj: that sounds like what I remember; why shouldn't they all be 65?
< sipa> i don't believe that belongs in the same bip
< luke-jr> the code would likely be simpler to have 65 bytes for everything
< luke-jr> and unless I'm mistaken, SIGHASH_ALL vs other SIGHASH don't change actual resource costs
< aj> sighash_all is slightly easier to calculate (hash is the same for all sighash_all sigs in a tx)
< aj> err, in an input, except for codesep use
< sipa> i think it would save maybe 2 lines of code here: https://github.com/sipa/bitcoin/blob/taproot/src/script/interpreter.cpp#L1527L1533
< sipa> i think there is a fungibility benefit to encouraging a default sighash
< luke-jr> 4 lines
< luke-jr> if (sig.size() != 65) return false;
< sipa> ok!
< luke-jr> uint8_t hashtype = sig.back();
< luke-jr> sig.pop_back();
< luke-jr> I think the fungibility thing is stretching it, but aj may have a point
< luke-jr> sipa: did you understand/address my other comments?
< sipa> the non-overridable branches thing was answered i think in the thread, and is also in the document; you can use a point without known DL as internal key, and the result is something that can provably only be spent using scripts
< sipa> salting branches shouldn't be needed if you don't reuse pubkeys
< luke-jr> what if a branch doesn't have any keys?
< aj> you can pair it with a branch that's "OP_RETURN <salt>"
< luke-jr> anyone-can-spend is apparently useful to troll the IRS :P
< luke-jr> aj: ah, nice idea
< bitcoin-git> [bitcoin] ch4ot1c closed pull request #16797: scripts: Add convenience script for committing scripted-diffs from a file (master...scripts/commit-script) https://github.com/bitcoin/bitcoin/pull/16797
< bitcoin-git> [bitcoin] promag opened pull request #17182: 0.19: Revert 14193 to fix 17112 (0.19...2019-10-revert-14193-fix-17112) https://github.com/bitcoin/bitcoin/pull/17182
< promag> ^ 14963, #16443 and probably others are making the revert hard
< gribble> https://github.com/bitcoin/bitcoin/issues/16443 | refactor: have CCoins* data managed under CChainState by jamesob . Pull Request #16443 . bitcoin/bitcoin . GitHub
< promag> lots of annotations were merged