< kallewoof>
How does bitcoin core track bip9 activation states? I have odd cases where a copied chain state will result in all bip9 soft forks turning up as "failed" rather than "activated". If I disable the timeout, they show up as 'started', but with 'possible: false'.
< sipa>
kallewoof: versionbits.cpp
< kallewoof>
sipa: Yeah, I've been staring at that file for awhile now. Will stare some more.
< sipa>
kallewoof: it should be implementing bip9 exactly
< kallewoof>
Yeah, I think I'm confused over what I'm confused over tbh. Will do some debugging.
< achow101>
kallewoof: those errors you mention sound like mtp isn't being computed correctly
< kallewoof>
achow101: Huh.. I haven't really touched that part of the code.
< sipa>
but everything should be recomputed at runtime
< achow101>
kallewoof: sipa: is it possible that pruned chainstates cannot compute bip9 status?
< kallewoof>
They only rely on block headers so I think they should be able to
< sipa>
indeed
< bitcoin-git>
[bitcoin] MeshCollider opened pull request #14491: Allow descriptor imports with importmulti (master...201810_importmulti_desc_2) https://github.com/bitcoin/bitcoin/pull/14491
< bitcoin-git>
[bitcoin] kallewoof opened pull request #14492: autoconf: add 'test' alias for 'tests' to configure (master...ac-test-arg-alias) https://github.com/bitcoin/bitcoin/pull/14492
< sipa>
meshcollider: that's growing into a painful amount of code :)
< wumpus>
meshcollider: thanks, will have a look
< sipa>
will review in detail soon
< meshcollider>
sipa: the segwit one?
< sipa>
and thanks for quickly addressing my comments so far
< meshcollider>
sipa: it still removes more than it adds ;)
< meshcollider>
thanks for giving comments so quickly
< meshcollider>
and 14303 removes all the duplicated pwallet->MarkDirty() calls which will tidy it up a bit more too
< wumpus>
habasto: would rather close 14370, don't think it is a necessary change
< hebasto>
wumpus: thanks for your quick review; I'll think about it.
< wumpus>
if something can be solved by better documentation, please work on documentation!
< wumpus>
don't change the code instead
< bitcoin-git>
[bitcoin] MeshCollider opened pull request #14494: Error if # is used in rpcpassword in conf (master...201810_hash_in_rpcpassword_error) https://github.com/bitcoin/bitcoin/pull/14494
< wumpus>
habasto: huh so does #13143 provide an actual use-case for quoting functionality?
< meshcollider>
wumpus: I would say it makes it even more confusing lol
< wumpus>
hehe
< wumpus>
well I remember because windows .ini format has no way to insert #'s, that's also why I know they have no quoting...
< wumpus>
all this is kind of terrible edge-case stuff
< meshcollider>
I agree
< meshcollider>
we should just disallow same-line comments and only exclude lines from the conf if they *start* with a #
< wumpus>
yes, I think that's a fair option, I remember seeing that in other software (but don't remember which one, right now); though,also a breaking change
< wumpus>
people might already be using # to comment their .ini files lines... so would create the *opposite* issue :(
< meshcollider>
yeah I think its too hard to change now
< wumpus>
anyhow I think 14494 is good, just don't have "illegal hash character" in the error message that's terrible :-)
< meshcollider>
haha ok what should I change it to
< wumpus>
well what I said in the PR, it's at most ambigious, it's rejected because it's probably a mistake
< meshcollider>
" do not use hash characters in rpcpassword" ?
< wumpus>
if it's an actual comment it is actually okay, in principle
< wumpus>
well a friendlier message, not something about 'do not use' but just explain to the user why this is a problem
< meshcollider>
ok
< wumpus>
illegal hash characer sounds like the cops will show up any time :)
< kallewoof>
wumpus: I think iptables will happily try to parse # chars unless they are the first on the line
< meshcollider>
"using hash in rpcpassword can be ambiguous and should be avoided"
< wumpus>
yes, that would be better
< wumpus>
or maybe don't call it hash but simply mention '#'
< wumpus>
that's more direct for non-english speaking people
< hebasto>
^
< meshcollider>
true, and even english speakers sometimes call it pound or octothorpe
< sipa>
meshcollider: probably only antipodians
< meshcollider>
sipa: lol
< meshcollider>
sipa: achow used "pound", so it must be you all who are upside down
< luke-jr>
meshcollider: but if we can't hash over RPC, how will people mine? /s
< sipa>
clearly we need to rename it 'pounders'
< luke-jr>
sipa: also, what are these 'podes' that people are protesting? :p
< karelb>
I am trying to run my fork of bitcoind on travis.... and travis stops with this
< karelb>
failure generated from test/lint/lint-spelling.sh
< karelb>
.....ummmmm, ok?
< karelb>
I did no change in threadinterrupt, and that "mut" is just some mutex
< karelb>
well whatever, one commit that renamed "mut" to "mutex" fixed that, I just wonder why I needed to do that...
< gwillen>
karelb: your travis instance is stupid
< gwillen>
if you make it less stupid you will have fewer problems
< gwillen>
it is trying to spell-correct your code, and doing it moronically
< karelb>
:D
< gwillen>
and seems to be set to warnings-as-errors.
< karelb>
I use travis-ci.org
< gwillen>
well, apparently they are stupid
< gwillen>
but I imagine there is an option not to make codespell warnings errors
< karelb>
the same config as bitcoin core
< gwillen>
which they absolutely should not be
< gwillen>
oh, hm
< gwillen>
I mean, seemingly not
< karelb>
well let's see what happens when I make a PR, if that `mut` stuff is still there
< meshcollider>
sipa: re private keys, if youve imported the private key then the danger isnt as prominent is it
< meshcollider>
because even if they send to the key directly, you have the privkey
< wumpus>
karelb: lol that's stupid
< wumpus>
please don't tell me that stupid spelling check is mandatory in travis now
< karelb>
wumpus: maybe it's some misconfiguration... I don't really want to deal with it so I made a commit that I will remove before doing PR
< karelb>
that commit just renamed mut to mutex
< wumpus>
AHH apparently it was changed to fail in e413c2ddd1240d7bacd1837fa49d25781fe6e5fa
< wumpus>
yes I can imagine...
< wumpus>
i've tried, in vain, to prevent all kinds of silly lints from being merged
< karelb>
:D
< karelb>
I wonder why is my fork failing but not bitcoin master
< karelb>
but I don't wonder *that much*
< wumpus>
maybe... it only runs on PRs?
< wumpus>
well you could always rename it 'mutt' or 'moot', i'm sure that's an improvement...
< karelb>
'mutex' seems to work, although 'mutt' has a ring to it
< wumpus>
i'm not sure whether this constitutes a 'variable names need to be valid english words' policy
< promag>
wumpus: from the PR discussion looks like that change slipped?
< promag>
maybe the reason is because nobody checks the warnings
< luke-jr>
'variable names need to be valid english words' sounds like a stupid policy..
< wumpus>
luke-jr: no disagreement from me, i'm sure i've protested in the PR that added this linter, but meh i don't have the energy to fight this
< * wumpus>
wants to add a linter that deletes all linters
< luke-jr>
:P
< wumpus>
soo enough about that shit, are there any serious PRs I should pay attention to?
< luke-jr>
dunno, barely have time to rebase my own stuff :/
< luke-jr>
the downgrading warning in 0.17 relnotes is confusing; why is it talking about 0.15?
< meshcollider>
luke-jr: because that's the version at which it changed isn't it
< meshcollider>
but yeah its not worded well
< luke-jr>
dunno, 0.16 didn't mention it
< luke-jr>
I thought we had a change in 0.17 too?
< wumpus>
I *guess* it's when you want to downgrade from 0.17 to 0.15?
< meshcollider>
"Wallets created in 0.16 and later are not compatible with versions prior to 0.16 and will not work if you try to use newly created wallets in older versions. Existing wallets that were created with older versions are not affected by this."
< wumpus>
it's a bit late to discuss 0.17 release notes, now
< meshcollider>
it looks like its just been copy+pasted from 0.15, oh well
< luke-jr>
wumpus: that's not what it says though :/
< luke-jr>
yes, just noticed
< wumpus>
it probably has
< wumpus>
the upgrading/downgrading part tends to be copy-pasted between releases because it tends to stay relevant
< wumpus>
at some point it can be removed because no one is going to downgrade to 3 major versions back, but still
< wumpus>
would be good to have close scrutiny of the release notes before a release
< promag>
wumpus: I'm depending on #14291 for the multiwallet support in the UI
< wumpus>
promag: yeah have that one merged locally for testing
< promag>
wumpus: nice
< meshcollider>
sipa: for the descriptors PR, what if we don't even import the public keys? Why not just import the private keys, otherwise just import the scriptPubKey only as watch only
< promag>
sipa: meshcollider: please also see #14303
< meshcollider>
promag: Already concept ACK'ed it ;) mentioned it above, its a nice cleanup
< promag>
ken2812221: how do you build and run tests on windows? do you use wsl and depends?
< ken2812221__>
promag: I use both MSVC and virtual machine to build it. The IO on WSL is extremely slow.
< promag>
ken2812221__: right, it is
< promag>
ken2812221__: is there a guide or something so I can easily setup the same here?
< ken2812221__>
Copy bitcoin-cli.exe and bitcoind.exe into src folder. Modify and copy test\config.ini.in to test\config.ini. Then you can run python test\functional\test_runner.py --force.
< wumpus>
promag: i'm personally scared of that behavior, but if others think it's ok, i'll just go along...
< wumpus>
promag: you're right it only scans the files at recursion depth=0 for the btree signature, though it still *nests* into all the directories nevertheless
< promag>
wumpus: do you think it should do that only if walletdir != datadir?
< wumpus>
that would, to me, seem like the most straightforward way to prevent this; then give the user a clear explanation *why* it won't work and what they can do to make it work
< wumpus>
one specific concern: opening and closing the .lock file will nix the datadir lock, at least on some OSes
< promag>
hmm, skip that filename then?
< promag>
ryanofsky: ^
< wumpus>
that seems *extremely* fragile
< wumpus>
maybe this brings a valid problem to light: there's just too many ways to organize the data directory with regard to wallets, all need to be handled, and tested
< wumpus>
in any case, I'd prefer if the RPC simply failed in that case
< promag>
but do you agree on the concept?
< wumpus>
in the case where there is a separate wallet directory, certainly!
< bitcoin-git>
[bitcoin] practicalswift opened pull request #14495: build: Warn (don't fail!) on spelling errors (master...revert-codespell) https://github.com/bitcoin/bitcoin/pull/14495
< promag>
wumpus: I'll push a commit with that change to see what others say
< bitcoin-git>
[bitcoin] practicalswift opened pull request #14496: build: Pin to specific versions of Python packages we install from PyPI in Travis (master...pin-pip-installed-packages-in-travis) https://github.com/bitcoin/bitcoin/pull/14496
< promag>
ken2812221: just say build_msvc/README.md
< Jmabsd>
sorry for disturbing - can you give me some reference where the hash in a P2WSH pubkey script is validated or/and generated? i like to understand if it's single or dual iteration SHA256 and how the code looks.
< karelb>
ad the codespell issue - it seems I have branched master *before* the codespell version was fixed, so that's why it got updated now and shouted at me
< bitcoin-git>
[bitcoin] DesWurstes closed pull request #14486: Add explicit cast to base58 and bech32 string constants in order to silence GCC warning (master...patch-4) https://github.com/bitcoin/bitcoin/pull/14486
< meshcollider>
hebasto: I mentioned that idea above briefly, we can't just stop ignoring comments in-line, because many people's conf files may already use inline comments
< meshcollider>
So suddenly they would all stop being ignored
< gmaxwell>
oh come on, getblockstats run with no special aguments either works or doesn't work without txindex enabled, based on what block you call it on.
< echeveria>
achow101: they kind of need to go away
< echeveria>
achow101: there's fucking like, 3000+ open RPC ports on IPv4.
< echeveria>
achow101: assclowns like Samurai Wallet are telling people to bind RPC to 0.0.0.0.
< gmaxwell>
echeveria: what?!
< gmaxwell>
why?
< echeveria>
gmaxwell: connect your wallet to your own node, using RPC! or something. I don't know much about it other than reading some people trying to do it in #bitcoin.
< gmaxwell>
actually they tell you to VPN, no rpc bind it seems?
< gmaxwell>
what does this feature do?
< luke-jr>
they also tell you to rpcallowip your LAN (or VPN) only
< luke-jr>
but I guess that may bind 0.0.0.0 :/
< gmaxwell>
why are they not just using p2p?
< gmaxwell>
"Use your personal node to broadcast transactions to the bitcoin network"
< gmaxwell>
thats what it claims it does
< luke-jr>
wtf, that's not even particularly useful
< echeveria>
from the description I doubt it uses it as a data source.
< echeveria>
this is the company that lied in their original software release and claimed to be decentralized, while using blockchain.info in their closed source release.
< gmaxwell>
I know previously it used bc.i as its data source. I don't see how it would use your local node as a data source.
< gmaxwell>
"We build the software that Bitcoin deserves" ... just so.
< echeveria>
all of their "privacy" tools are snake oil, to boot. it's unfortunate that they're impacting bitcoin node users as well as people fooled into using their incompetent software.
< achow101>
echeveria: removing rpcuser and rpcpassword wouldn't change that though
< achow101>
you can still set rpcauth
< achow101>
(albeit harder to do)
< echeveria>
achow101: it ends up being an automatically generated token, doesn't it?
< gmaxwell>
echeveria: no thats cookie auth.
< achow101>
echeveria: no, that's only the cookie auth stuff. that token goes into the .cookie file
< achow101>
rpcauth is like rpcuser and rpcpassword except as one config option and the password is hashed
< echeveria>
oh, I thought they were the same thing.
< echeveria>
guess that doesn't implicitly solve any issue like I thought.
< achow101>
i mean it's harder for people to figure out how to hash their password to use rpcauth
< gmaxwell>
cookie auth is a replacement for things that read passwords out of the conf file, rpcauth is the replacement for things that have a persistant key left in another app.
< echeveria>
yep, understood, I just bundled them together in my head. it sort of surprised my awfully to realise that so many RPC ports were open in public IPv4 scans. I'm not sure there's a resolution to that, it's already something that's non-trivial to do.
< gmaxwell>
I hope that many of them are honeypots, etc.
< echeveria>
unless the RPC bind option is changed to IM_AN_IDIOT=0.0.0. or something, which I can understand why nobody would want to do that.
< echeveria>
hopefully.
< meshcollider>
sipa: how about checking the flatsigningprovider only added a single pubkey for each descriptor, and import it if it did
< gmaxwell>
just change the string to a new random 256 bit hex value in each release. :P
< meshcollider>
Would that work?
< echeveria>
gmaxwell: see I'd say remove the ability to bind anything but localhost, but I know that would mean people would refuse to upgrade for compatibility reasons.
< sipa>
meshcollider: i have a bit of another strategy in mind
< achow101>
or remove rpcbind and make people figure out iptables/firewall crap to get it accessible from the outside world
< sipa>
meshcollider: let's focus on making importmulti work first for all supported things, and ignore descriptors
< sipa>
meshcollider: i think the code can be massively simplified (but let's do that after merging your PR)
< gmaxwell>
echeveria: it's not unreasonable to bind to a private lan, for example. just security wise fragile.
< meshcollider>
sipa: Alright, sounds good
< sipa>
and after simplification, it can share a lot of logic with the descriptor approach
< gmaxwell>
echeveria: maybe {stop} should work without authentication. :P
< meshcollider>
sipa: does this include change to the wallet or just the RPC code
< sipa>
meshcollider: just the scope of your current PRs
< gmaxwell>
echeveria: oh there you go, auto stop after 1000 invalid auth attempts. :P
< echeveria>
yuck.
< sipa>
echeveria: wumpus had some unix socket binding RPC stuff earlier, not sure what the state is
< sipa>
perhaps that can be made default the only open thing, and you need a separate tool to proxy it to TCP/IP
< sipa>
... at some point
< gmaxwell>
sipa: it was held up because the client side support needed upstream improvements in libevent. :(
< sipa>
gmaxwell: right
< echeveria>
sipa: I think that results in people using obsolete versions sadly.
< sipa>
echeveria: quite possibly
< echeveria>
that's what happened with BitPay's Insight. they stopped maintaining it so everybody who built around it is still running 0.12, as far as I can tell not behind guard nodes.
< gmaxwell>
I do think that making it harder to make rpc remotely accessible might be a reasonable move, esp post domain socket support. (not that they're really related)
< jarthur>
sipa gmaxwell do you remember why we needed to re-use a socket object in libevent? Is it because we wanted the unix socket file managed by Core instead of libevent?
< gmaxwell>
echeveria: disabling it would do that, but requiring an insecure_nonlocal_binding=1
< echeveria>
yep.
< gmaxwell>
jarthur: should be in wumpus' issue in the libevent repo.
< jarthur>
sipa: alright. I think that's what wumpus' upstream PR was all about. I think we can do it in libevent today if we have libevent opening the socket in the first place.
< jarthur>
Why is this remote RPC stuff coming up? IMO, it's already hard to expose an RPC port on anything but the loopback.
< echeveria>
jarthur: I noticed that there's a large number of RPC sockets open.
< jarthur>
echeveria: ah, when using Samurai? Or just listening sockets on a clean Core run?
< gmaxwell>
jarthur: wumpus' major motiviation behind working on domain sockets was getting bitcoind to run in a very restrictive sandbox.
< gmaxwell>
samurai is just one of many potential causes for the large number of rpc exposes hosts.
< echeveria>
jarthur: just in general, many IPv4 that have 8333 open also have 8332 open. there's various tools like shodan that present this information in an easy to consume way. samourai was just one theory of mine as to why people might be doing it, as they do suggest exposing your RPC port in some way, for a very stupid reason.
< gmaxwell>
(and a kind of absurd one, since it looks like they have the user open up the RPC to remote hosts just to sendrawtransaction)
< achow101>
even with rpcbind=0.0.0.0, you still need to be whitelisted in rpcallowip to connect though
< achow101>
unless people also did rpcallowip=0.0.0.0
< jarthur>
echeveria: alright. There's also electrumx, electrum-server, electrum-personal-server, electrs, getwork proxy, stratum mining proxies, stratum mining pools. A whole lot of RPC uses out there.
< gmaxwell>
none of those things should be exposing the bitcoind rpc to the wide internet.
< echeveria>
jarthur: none of what you've described need a world listening RPC port.
< jarthur>
Yea, not justifying the open port, just why people end up opening those ports in the first place.
< gmaxwell>
achow101: perhaps we should require "massively_increased_security_vulnerablity_exposure=1" for any rpcallowip statement wider than /8. :P
< jarthur>
I sometimes load up an EC2 Core server, open up RPC to the net and use Amazon Security group to only let myself have access. I got no shame in it :)
< echeveria>
jarthur: huh? it needs extra effort to make it bind to 0.0.0.0, which you don't need for nay of the services you talk about.
< echeveria>
jarthur: you're a bad sysadmin. you have SSH, forward the port locally.
< gmaxwell>
jarthur: the fact that its possible to 0/0 safely (although fragile) is why its possible at all.
< jarthur>
echeveria: yea, that's certainly better. Have done that too when less lazy.