< luke-jr>
wouldn't the implicit in-memory redeemscript adding for old keys be sufficient if only applied to the keypool/new keys?
< sipa>
read the design doc, it goes through all the scenarios it is intended to work in, and how they necessitate the implemented behaviour
< luke-jr>
sipa: I read it, but it's not clear to me why 1a requires it effect ALL keys, rather than just keypool (ie, unused at the time of the backup) and future
< luke-jr>
labelled keys already have a non-segwit address assigned, and should never be seen in segwit form
< sipa>
you can't know that you aren't using a restored backup which had a lost future in which the same key was used in a different address type
< luke-jr>
hm, so 1) user backs up, 2) user makes a new address, 3) user restores backup, 4) user upgrades, 5) user makes a new segwit address overlapping the one in 2, 6) user downgrades, 7) user restores backup again
< luke-jr>
?
< sipa>
the only way we can actually remove the ability to accept payment to a different address type than the one we know about is afaik by having separate keychains for each address type
< sipa>
luke-jr: i don't think 6 and 7 are needed in that scenario
< sipa>
and swap the segwit and legacy
< meshcollider>
sipa: they are needed because the first address was legacy and the second was segwit i believe
< meshcollider>
yeah
< luke-jr>
sipa: I don't understand.. step 2 can't make a segwit address since it's pre-upgrade
< sipa>
someone first creates a backup, then creates a segwit address, then restores the backup, creates a legacy address with the same key, restarts
< sipa>
no old versions involved even
< luke-jr>
upon restoring the backup, that key would be in the keypool, so it would get the implied script
< sipa>
the result is a file with no explicit segwit script, and a label for a legacy address for a certain key
< luke-jr>
oh, add a restart and the implicit script disappears!
< meshcollider>
yeah the legacy address has to be created on the old version so it doesnt hit the implicit addition of segwit scripts
< meshcollider>
oh
< sipa>
no it doesn't
< meshcollider>
yeah I got it now, right
< luke-jr>
I thought I got it until "no it doesn't" :x
< luke-jr>
unless that was a reply to meshcollider
< sipa>
the "no it doesn't" is a response to meshcollider saying it needs an old version
< meshcollider>
yeah that was a reply to me
< luke-jr>
ah ok
< sipa>
i'm not sure this scenario is actually covered in thendocument though
< meshcollider>
sipa: it is, just not so explicitly written out
< sipa>
may be worth spelling kt out
< sipa>
*it
< meshcollider>
it just stops at restoring the backup, it doesn't mention creating a new legacy address and restarting
< meshcollider>
yeah
< provoostenator>
The step 1-7 scenario luke-jr describes above is somethign I'd like to try on top of #12134
< meshcollider>
provoostenator: I've only looked briefly at that but won't adding cross version testing to travis be too slow
< provoostenator>
meshcollider: the releases are cached, so it's a one-off 30 minute thing (and only for one machine).
< meshcollider>
provoostenator: oh ok, that's all good then :)
< luke-jr>
sipa: PM
< meshcollider>
Does collaborators only mean members of the organization with write access to the repo? I'd previously assumed we'd be able to comment on PRs/issues even if they were locked because we were members but that doesn't appear to be the case
< provoostenator>
Travis is in a bad mood today?
< zelest>
must be Ultron
< meshcollider>
provoostenator: are you referring to 11991? I restarted it for you
< meshcollider>
sipa: if you're online maybe you could add Sjors to the org so he can restart travis too
< sipa>
i'm just on my phone now, will do later
< provoostenator>
meshcollider: 11991 and 12119
< meshcollider>
provoostenator: 12119 failed 4 checks, are you sure that its travis fault not something wrong with your PR?
< provoostenator>
I'm not ruling that out, but the errors seemed to be timeouts.
< meshcollider>
ok we'll try it again and see :)
< provoostenator>
I'll also run the full suite locally
< bitcoin-git>
[bitcoin] luke-jr opened pull request #12146: Wallet: Support disabling implicit Segwit operation (master...opt_wallet_segwit) https://github.com/bitcoin/bitcoin/pull/12146
< meshcollider>
provoostenator: seems they're all failing again, so yeah must be something to do with the PR not travis
< meshcollider>
what do these people hope to achieve with PRs like that, do they honestly ever think it will get merged?
< sipa>
meshcollider: i believe they don't understand what 'pull' means in this context
< sipa>
i believe many of them are just trying to make a clone or try making a simple change in their own cooy
< provoostenator>
meshcollider: probably, but they both pass on my machine. 11991 fails with "/wallet/db.h:21:20: fatal error: db_cxx.h: No such file or directory" on one machine. I can try on a linux VM...
< bitcoin-git>
[bitcoin] jsarenik opened pull request #12168: Trivial: Fix #include sys/fcntl.h to just fcntl.h (without sys/) (master...jasan/fcntl.h) https://github.com/bitcoin/bitcoin/pull/12168
< bitcoin-git>
[bitcoin] kekimusmaximus opened pull request #12169: Avoid temporary copies in C++11 ranged-based for loops. (master...remove_loop_implicit_casts) https://github.com/bitcoin/bitcoin/pull/12169
< morcos>
maaku: It is possible I have been out of the loop. As you may know a lot has been happening in the last several months.
< morcos>
I though there were several MAST proposals in the works
< morcos>
I thought based on the amount of traffic that they weren't currently getting anywhere near the developer or community mindshare to be getting near adoption
< morcos>
I'd seen know discussion on whetther there was any consensus to the protocol changes or not
< morcos>
To be honest, I don't know if I have any concerns or objections becuase I haven't looked at it yet. I'm in favor of the general concept, but I'm not convinced it should be a priority right now
< morcos>
It would be one thing if it wasn't a consensus change, but it is, so I think before discussing PR's being ready for Core, it makes sense to discuss whether the community wants this.
< morcos>
That said, having Code is always helpful, and its good for me and others to get the feedback that this is further along than I thought if thats the case
< wumpus>
congrats on merging segwit wallet support jonasschnelli
< wumpus>
and sipa and everyone that reviewed ofcourse
< provoostenator>
luke-jr from the title and description of 12146 it's not immediatley clear what you're tying to do, but I'll study it later. If you mention the "-walletimplicitsegwit" flag explictly in the description, it's probably a bit more clear.
< provoostenator>
It would also help to clarify how it's different from -addresstype=legacy, other than foot-shooting protection.
< provoostenator>
I assume we'll chat during todays meeting about what the earliest possible release date is, so people know how much time is left to get more stuff in?
< provoostenator>
(particularly stuff related to SegWit)
< provoostenator>
I'd say at least two weeks given some of the current discussion.
< wumpus>
yes we should give some time for fixes after this merge and before the release
< promag>
there are some things to do
< provoostenator>
Is there something we changed since v0.15.1 that causes newly created wallets to be incompatible with v0.15.1? If so, what compile / launch flag do I need to use for v0.15.1?
< promag>
maybe sipa is working on those follow ups?
< provoostenator>
(if not, then I'm during something wrong in my regtest setup, quite possible)
< wumpus>
probably the segwit change caused newly created wallets to be incompatible with 0.15.x?
< provoostenator>
sipa: I'm a bit confused as to how to interpret scenario 1 in your gist. At least in the test I wrote, if you restore from a backup then any addresses that were generated after that backup would only reappear if you remember the correct sequence of legacy/p2sh-segwit/bech32 choices: https://github.com/bitcoin/bitcoin/pull/12152
< provoostenator>
And funds don't show up for me unless I get that sequence right.
< sipa>
provoostenator: that's a bug!
< gmaxwell>
?!? what do you mean choices?!
< provoostenator>
sipa: ok, either that or my test is wrong.
< gmaxwell>
they should all appear with you doing nothing but starting the software and waiting for the rescan to complete.
< sipa>
yup
< sipa>
amd it is totally plausible that it is broken
< sipa>
*and
< sipa>
in which case i'd very much like to know
< provoostenator>
Right, but that's what the "look for related keys" logic is for, right?
< gmaxwell>
I did test this at one point.
< gmaxwell>
though only with native segwit outputs.
< provoostenator>
Maybe it's because my tests use accounts, I don't know how that changes things.
< sipa>
provoostenator: what fails? it should work fine as long as you don't create bech32 addresses
< sipa>
provoostenator: oh, or the default key thing?
< provoostenator>
After I copy the wallet over to a 0.15 node and restart, it throws "Error loading wallet.dat: Wallet requires newer version of Bitcoin Core". I only added a legacy address.
< provoostenator>
default key thing?
< sipa>
provoostenator: the account obviously won't be credited - it can't predict which label you'll give future addresses
< sipa>
but the wallet balance should get uodated
< sipa>
yes, i don't think you can use 0.16 wallets in 0.15
< sipa>
but you can create a 0.15 wallet, use it 0.16, and then downgrade
< provoostenator>
Ok, I'll use that approach in my backwards compatilibity test. It's more future proof anyway.
< provoostenator>
I didn't check the total wallet balance, so that might be it then. So after restoring from a backup a user would see random new addresses have funds?
< wumpus>
yes, that's the way to go
< sipa>
yes
< provoostenator>
I suppose the only way to prevent that is to disallow address creation before rescan / sync is done.
< sipa>
?
< provoostenator>
Anything around --with-incompatible-bdb that's worth testing?
< provoostenator>
The wallet would check to see if a key already has funds and then skip it in the new address UI.
< sipa>
i'm very confused :)
< provoostenator>
Right now if you click on new address it will just pick the next key, even the corresponding address has coins on it.
< sipa>
no
< sipa>
oh... if an address is seen used on the network it is removed from the keypool
< provoostenator>
Which is weird for the user, because they thought they created a fresh address.
< sipa>
so it won't be given out agaon
< provoostenator>
"is seen used" is the operating word then, because in my test it doesn't see it in time.
< sipa>
okay
< sipa>
well there may be a bug!
< sipa>
or maybe you didn't wait for rescan etc
< provoostenator>
I did the rescan after generating the address, yes.
< sipa>
s/rescan/sync/
< provoostenator>
So you're saying if I do a rescan before generating an new address this won't happen?
< sipa>
there shouldn't be a need for rescan
< provoostenator>
What happens when you load an outdated wallet backup where new addresses have been added and funded by the user? Does it rescan it?
< sipa>
at startup, if your wallet is older than the chain, the missing part is automatically rescanned
< provoostenator>
In. my test (line 206) I had to do a rescan in order for the balances to show up on generated addresses.
< provoostenator>
Maybe that's because regtest behaves different in this regard and doens't do rescans automatically.
< provoostenator>
The test stops the node, replaces the wallet with an older version, starts the node, generates the same addresses and then calls rescan. That's when the balance shows up for these addresses.
< provoostenator>
Without rescan the address balances remain zero, though I didn't check the wallet balance.
< provoostenator>
I'll try some variations once I understand the intented behavior better.
< sipa>
don't look at address balance
< sipa>
look at wallet balance
< sipa>
listtransactions maybreport garbage
< sipa>
listunspent should probably work
< provoostenator>
I'm using getbalance("account name")
< sipa>
... how could that possibly work?
< sipa>
the backup doesn't know what name you're going to give a future address
< provoostenator>
Which I'm assuming is what QT relies on to show the balance in the receive tab, so it could be pretty confusing (but haven't tested)
< sipa>
no
< sipa>
that's even a deprecated RPC
< sipa>
the GUI doesn't support accounts, and never has
< blueneon>
Hi there, I've made a fork/clone of the bitcoin coin in order for me to play around and get familiar. All went well after updating chainparams.cpp etc and I created new genesis and merkle etc -the code compiles and runs. However it crashes and debug.log says: ERROR: ReadBlockFromDisk: Errors in block header at CBlockDiskPos(nFile=0, nPos=8)
< provoostenator>
Ah, that points to my confusion between accouts and labels then..
< blueneon>
Any advise would be much appreciated.
< sipa>
blueneon: ##altcoin-dev
< blueneon>
Ok thanks
< sipa>
provoostenator: the only thing you should be looking at is getbalance (without argument), listtransactions, listunspent
< provoostenator>
How is the label that I use in QT when I create a payment reqest represented in the RPC?
< blueneon>
seems ##altcoin-dev is a dead chan :/
< blueneon>
Any chance someone here can shed light on my issue?
< sipa>
blueneon: not here, sorry
< provoostenator>
blueneon: that's not a good way to get familiar with the code in my experience. I somewhat selfishly suggest looking at random bitcoin core github issues and trying to help out in little bits.
< sipa>
provoostenator: labels amd accounts are the same up to that functionality... it's a name associated with an address
< sipa>
provoostenator: but labels don't have a balance
< sipa>
listreceivedbyaddress would also work
< provoostenator>
Ah, so a key has multiple addresses, each address can have a label. Not in the other direction?
< sipa>
but inherently, when recovering fromna backup, the labels won't be there - they can't be
< gmaxwell>
one of the many limitations of labels.
< sipa>
provoostenator: addresses have a label; multiple addresses can have the same label
< provoostenator>
I know the labels wouldn't come back. But when the user creates a payment request that address shouldn't already have money on it. But it seems I didn't actually test that, becaus eI was using accounts.
< provoostenator>
Multiple address (from different keys?) can have identical label strings or do you mean they point to the same label object?
< sipa>
provoostenator: what's the difference?
< provoostenator>
The difference is what happens to the label of addres A if I change the label of address B.
< provoostenator>
If they were identical before.
< sipa>
oh, i see
< sipa>
nothing changes to A
< provoostenator>
In other words, what's the SQL schema :-)
< sipa>
addresses have a string lab
< sipa>
el
< gmaxwell>
same label can be on many addresses (and often is)
< sipa>
pro when the user requests a payment it should never give out an address associated with a key that has already been used
< sipa>
*provoostenator
< provoostenator>
Alright, I'll take another look with that background.
< provoostenator>
How are we doing on actually deprecating accounts?
< sipa>
basically rename the string account label everywhere in the source code
< sipa>
and then removing getbalance(account)
< sipa>
and removing the ability to specify a "from" account
< sipa>
when creating a tx
< sipa>
provoostenator: actually, i am pretty interested in knowing what listtransactions shows after recovery
< provoostenator>
sipa: I can take a look later (you should be able to run the backwards compatibilty tests locally in the meantime).
< bitcoin-git>
[bitcoin] ryanofsky opened pull request #12150: Fix ListCoins test failure due to unset g_address_type, g_change_type (master...pr/listg) https://github.com/bitcoin/bitcoin/pull/12150
< bitcoin-git>
bitcoin/master 2be2b5d Jacky C: Remove the ending slashes from RPC URI format.
< bitcoin-git>
bitcoin/master 3c62868 Wladimir J. van der Laan: Merge #12112: Docs: Remove the ending slashes from RPC URI format....
< bitcoin-git>
[bitcoin] laanwj closed pull request #12112: Docs: Remove the ending slashes from RPC URI format. (master...docs/multi-wallet_RPC_interface_correction) https://github.com/bitcoin/bitcoin/pull/12112
< provoostenator>
sipa: wallet balance immedidately after backup restore: 3 (missing funds and transactions from the post-backup keys)
< bitcoin-git>
[bitcoin] promag opened pull request #12151: Remove cs_main lock from blockToJSON and blockheaderToJSON (master...2018-01-blocktojson) https://github.com/bitcoin/bitcoin/pull/12151
< provoostenator>
If I add a rescan to the test, 2 coins and their transactions reappear (not associated with an account).
< provoostenator>
There's a third coin that won't appear until you've called getnewaddress 3 times.
< sipa>
provoostenator: after a confirmation?
< provoostenator>
Yes, these were all confirmed and synced before deleting the wallet and replacing it with a backup.
< provoostenator>
But I'm a bit confused myself now as to what's so special about htat 3rd ps2sh-segwit address.
< provoostenator>
It appears that a rescan revealed the wallet balance for 2 out of 3 keys (p2sh-segwit, bech32, p2sh-segwit), but it took 3 calls to getnewaddress to reveal the third one. Either a bug or more likely something really weird about my test.
< provoostenator>
Or maybe these transactions are chained in a way that matters.
< provoostenator>
I have a node0 which mines the blocks and sprinkles coins to the test nodes.
< provoostenator>
I should probably run these with deterministic random seed for easier comparison.
< sdaftuar>
gmaxwell: re #11739 -- thoughts on next steps there? i was thinking i need to document the change somehow, perhaps an update to the relevant bips and an email to the -dev list?
< gribble>
https://github.com/bitcoin/bitcoin/issues/11739 | RFC: Enforce SCRIPT_VERIFY_P2SH and SCRIPT_VERIFY_WITNESS from genesis by sdaftuar · Pull Request #11739 · bitcoin/bitcoin · GitHub
< provoostenator>
sipa: what's the expected failure mode of downgrading a wallet with a bech32 address? Is it supposed to be fully recoverable when you upgrade?
< sipa>
provoostenator: the failure mode is that RPCs may report weird addresses
< provoostenator>
Ok, that's what I'm seeing...
< sipa>
in particular one very short address that starts with a 3
< provoostenator>
I'll just bake that into the test, although a more severe warning would be nice, at least after upgrade.
< sipa>
it should be fixed by uograding again
< provoostenator>
Nope, weird address sticks for me. But I'm trying that again now.
< provoostenator>
It does recover. Any other scenarios I should add ^ ?
< sipa>
provoostenator: have you seen my gist on segwit wallet?
< sipa>
it explicitly lists a number of scenarios that are intended to work
< provoostenator>
I read it in early December. It is still up to date? Back then it had several atlernative plans.
< provoostenator>
So it's 1a and 3a now?
< sipa>
yes, it is up to date
< sipa>
and 5
< sipa>
but the solutions don't matter for you, just the supported scenarios :)
< provoostenator>
Ah yes, alright I'll see if can turn those into tests.
< bitcoin-git>
[bitcoin] promag opened pull request #12153: Avoid permanent cs_main lock in getblockheader (master...2018-01-getblockheader) https://github.com/bitcoin/bitcoin/pull/12153
< provoostenator>
sipa: might be a good idea to move that gist to the docs repo or some other easier to find spot? At least the current-facts part of it.
< sipa>
provoostenator: perhaps, though such documents may get outfated soon
< sipa>
*outdated
< provoostenator>
In case of the wallet format, any proposed change should reference that document and we could consider a PR unfinished until a corresponding doc change PR is there. Hopefully eventually the wallet will be intuitive enough that the doc can be removed.
< promag>
GH down?
< instagibbs>
promag, yes
< larafale>
hello guys, any brave soul to help me on this. I'm learning and testing address decoding. I've got a public key (from an electrum wallet) and I'm trying to ripemd160(sha256(pubkey)), but the result I get is not the result I'm expecting. I thought the result was going to be the pubkeyhash present in the scriptpubkey in one of the output where that address was used. Am I missing something ?
< bitcoin-git>
bitcoin/master 18be3ab Chris Stewart: Adding test case for SINGLE|ANYONECANPAY hash type in tx_valid.json
< bitcoin-git>
bitcoin/master 0910cbe MarcoFalke: Merge #12082: Adding test case for SINGLE|ANYONECANPAY hash type in tx_valid.json...
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #12082: Adding test case for SINGLE|ANYONECANPAY hash type in tx_valid.json (master...add_tx_valid_singleanyonecanpay) https://github.com/bitcoin/bitcoin/pull/12082
< achow101>
what's wrong with travis today?
< wumpus>
what is it doing?
< achow101>
nvm. I was just confused as to how 8 of the most recent PRs were failing travis. thought it was a problem travis but its actually a problem with those 8 PRs :/
< wumpus>
indeed, master is passing according to my mails
< provoostenator>
Call me superstitious, but Travis seems in a much better mood today.
< BlueMatt>
aws disk io more available cause everyone's jobs got slowed down at the cpu level for meltdown fixes? :p
< wumpus>
oops, accidentelly tagged #12101 0.17 while it should be 0.16
< provoostenator>
As discussed earlier today: what's the minimum time before we branch of 0.16? So there's some time to test stuff and fix PR's.
< gribble>
https://github.com/bitcoin/bitcoin/issues/12101 | Clamp walletpassphrase timeout to 2^31 seconds and check its bounds by achow101 · Pull Request #12101 · bitcoin/bitcoin · GitHub
< cfields>
tl;dr: there was an issue where fd's could be maxed out causing crashes and weird behavior. Let's assume that we get that issue under control. Should we still attempt to bump up our fd limit?
< wumpus>
cfields: I'm not sure that should be default behavior, how much is it frowned upon if daemons do that, don't know how other software handles it?
< wumpus>
cfields: I think the usual way is to leave ulimit setting to the user/sysadmin
< ryanofsky>
provoostenator, i think normally there is a feature freeze on master before creating the release branch
< wumpus>
yes, normally there's a feature freeze
< wumpus>
I also need to open translations for 0.16
< cfields>
wumpus: I tend to agree with that. IMO if we're bumping into fd issues, we want to see them rather than mask them
< cfields>
but iirc gmaxwell was generally in favor of a general bump
< wumpus>
cfields: indeed, we should try to be robust against them, or at least exit with a clear error if that it's not possible
< cfields>
wumpus: ok, mind commenting there? I'll poke gmaxwell again about it too
< cfields>
</topic>
< wumpus>
cfields: sure
< cfields>
thanks :)
< wumpus>
other topics?
< jonasschnelli>
I have a short disussion request for 11281
< provoostenator>
bech32 change thing needs a small refactor which I'll try to do tomorrow. But also has some discussion about which circumstances should trigger bech32 use. Please weigh in...
< wumpus>
both 11991 and 12119 are already in the 0.16 milestone
< gmaxwell>
The whole reason we didn't _always_ use native change was privacy, but if the outputs are native, then that same argument says we should use native.
< provoostenator>
What about inputs?
< sipa>
arguably you could just uniformly ranomly pick the address type of 1 of the inputs, and make the change that
< gmaxwell>
provoostenator: I think they're irrelevant.
< wumpus>
sipa: sounds like a plan
< gmaxwell>
sipa: I don't think that makes any sense.
< provoostenator>
As in: if any input or any output is bech32, use bech32, unless changeaddress param says otherwise. .... ok, why intput irrelevant?
< luke-jr>
wumpus: 12146 isn't yet.
< * sipa>
meh 12146
< wumpus>
luke-jr: seems like that needs some more discussion
< gmaxwell>
Why would it possibly be relevant in any way?
< instagibbs>
we want to frustrate chain analysis, mimicking the payment as much as possible is ++
< promag>
gmaxwell: what if someone wants 0.16 to behave like 0.15?
< luke-jr>
wumpus: releasing Segwit wallet without it is a problem for people who don't want to use Segwit.
< provoostenator>
We also want to reduce fees.
< gmaxwell>
The only reason to not always use native, assuming you are using segwit to begin with in your wallet, is that it would make it easier to tell which of the outputs are change.
< jonasschnelli>
luke-jr: define don't use?
< wumpus>
luke-jr: just don't use the segwit address types then?
< gmaxwell>
But that doesn't apply if the outputs (any of them, arguably) are native.
< luke-jr>
jonasschnelli: never have a Segwit UTXO despite what anyone else may do
< provoostenator>
So in other words, if you're spending from a bech32 address, there's no reason _not_ to use a native address as change?
< luke-jr>
wumpus: without 12146, people can malleate your address and you'll never know
< gmaxwell>
provoostenator: no!
< instagibbs>
provoostenator, ??
< wumpus>
is address malleation a thing now?
< provoostenator>
It saves fees and there's no privacy downside, what am I missing?
< sipa>
luke-jr: they can already
< gmaxwell>
Again, inputs are irrelevant-- I can't figure out why you think inputs are at all relevant.
< jonasschnelli>
provoostenator: he's worried about block size increase
< sipa>
and have forever
< jonasschnelli>
as sipa said,.. it was also possible by p2pk -> p2pkh
< luke-jr>
sipa: with 12146, you would at least notice / not accept it
< sipa>
luke-jr: why not? it's cheaper!
< jonasschnelli>
s/was/is
< luke-jr>
sipa: what?
< wumpus>
would anyone use that at all, why would it be worth maintaining?
< sipa>
you're lucky if someone sends you a witness output instead anyone of p2pkh
< jonasschnelli>
I think luke-jr argument are more political/philosophical then technical/economical
< wumpus>
it just complicates all kinds of logic
< gmaxwell>
provoostenator: the privacy downside arises when your pay to a non-segwit destination. If you pay a non-segwit destination but make a native output as change, then which of the outputs is change is far more easily identified.
< luke-jr>
wumpus: because using Segwit enables miners to increase block size, and has no benefits for the current wallet.
< instagibbs>
i think this is going to be another "no one else agrees" type argument, sorry luke-jr
< sipa>
(i think it's really bad that we accept outputs to addresses we didn'√ give out)
< BlueMatt>
provoostenator: its still a privacy leak, you're (probably) much, much more likely to use non-bech32 outputs as non-change, so irrespective of the inputs you'd be pretty clearly tagging your change output
< sipa>
but it's not specific to segwit, and needs fixing independently
< instagibbs>
simply not handing out those addresses should be enough... sender doesn't care if you want to spend more later
< BlueMatt>
provoostenator: in fact, even worse, you're tagging it *because* your inputs are bech32 you're making clear that you support segwit in your wallet, so the bech32 output is *most likely* the change
< luke-jr>
instagibbs: the sender can trick you into accepting Segwit without 12146
< wumpus>
instagibbs: indeed
< instagibbs>
luke-jr, worse, the sender *wont know* you don't implictly convert, leading to a orphaned utxo!
< wumpus>
'trick you into accepting segwit'
< instagibbs>
whatever jerk is doing that
< gmaxwell>
But if the other outputs are native, then this issue doesn't exist.
< luke-jr>
instagibbs: orphaned UTXO is exactly the expected outcome
< luke-jr>
instagibbs: and that's his fault
< Chris_St1>
so the debate is whether we are creating the change output as the same script type as the payment output in a two output scenario?
< instagibbs>
.....
< wumpus>
man, that sounds like it should be prosecuted as a war crime
< provoostenator>
Alright, so rule should be: ignore inputs, if any output is bech32, use bech32 for change, unless setting overrides?
< luke-jr>
Chris_St1: there's two parallel conversations going on here now :/
< provoostenator>
luke-jr: yes, IRC needs threads
< wumpus>
#topic when to use bech32 for change
< wumpus>
^^
< meshcollider>
provoostenator: yes that's most sensible
< BlueMatt>
provoostenator: thats not a bad idea, imo
< Chris_St1>
what about when there are more than two outputs? Or are we just worried about GUI?
< * jtimon>
thinks about what "trick you into accepting segwit" may mean...
< instagibbs>
Chris_St1, privacy mostly
< gmaxwell>
provoostenator: thats my belief.
< meshcollider>
Was sipa suggestion of random choice serious or a joke
< provoostenator>
Chris_St1: _any_ output, not _all_
< instagibbs>
meshcollider, serious to me, makes sense
< luke-jr>
IMO any output makes sense.
< gmaxwell>
provoostenator: I think someone could reasonable argue that it should be "if a majority" rather than any.
< cfields>
don't you lose the change privacy if there's a mix of segwit output types, though?
< meshcollider>
Yeah was unsure if there's any downside
< wumpus>
meshcollider: I think he was serious, it makes sense, less deterministic the change choice the better
< Chris_St1>
instagibbs: yes, but what if you a 3 segwit payment outputs and 2 p2pkh payment outputs, what is the change type?
< gmaxwell>
I think "all" is obviously too strong.
< cfields>
if there's a p2wsh output and change goes to p2wpkh, that's obvious
< instagibbs>
Chris_St1, arguments to be had. Could flip weighted coin, could just pick the cheapest to mimic
< instagibbs>
(native)
< gmaxwell>
cfields: "segwit" here means native.
< morcos>
I don't think its worth going overboard with complication. If you are super privacy conscious you can manually do what you want, otherwise we should make the default what makes sense for the network.
< morcos>
So I like provoostenator's idea
< provoostenator>
Chris_St1: bech32 in my proposal s well as the majority proposol (which I think it's a bit complicated...)
< Chris_St1>
morcos: But that erodes the privacy of *everyone*. see zcash?
< morcos>
Chris_St1: i don't think that analogy applies
< gmaxwell>
Any is what I proposed on the PR, though I noted that "majority" would also be defensible. Requiring all, or worse, not using native even if all are native is bad for privacy.
< BlueMatt>
yea, majority-are-native or signle-is-native are both fine with me
< luke-jr>
weighed random?
< gmaxwell>
I think it's okay to bias some here towards the more cost effective form.
< morcos>
well maybe, but thats why we're preservign some level of privacy... in that we only use bech32 for change if we're paying at least one bech32 address
< wumpus>
yes
< instagibbs>
hiding with other segwit wallets imo is your best bet anyways...
< gmaxwell>
also the difference between these things only matters for sendmany...
< provoostenator>
luke-jr: I that would be a great way for me to demonstrate my C++ incompetence :-)
< * sipa>
is fine with (if not otherwise configured) using any bech32 -> change is bech32
< jnewbery>
any-output-is-native seems reasonable to me
< achow101>
same
< gmaxwell>
sipa: I think my suggestion was "unless configured that change should be legacy, change is native if any output is native".
< morcos>
i think non-determinism in what kind of change address you have is just ick.. especially since the p2sh-wrapped kind is temporary
< provoostenator>
As for luke-jr's point: having some flag to completely opt-out of SegWit seems reasonable to me.
< gmaxwell>
(or if anyone felt that was too strong, change is native unless the majority is non-native; but it seems no one does)
< sipa>
gmaxwell: and p2sh-p2wsh otherwise?
< gmaxwell>
sipa: yes.
< sipa>
makes snese
< Chris_Stewart_5>
gmaxwell: by native you mean p2wpkh?
< gmaxwell>
sipa: so if you set yourself to be legacy, you'll stay legacy regardless, in order to respect any weird requirement to avoid creating segwit outputs.
< instagibbs>
morcos, if it has native change is native is entirely determinstic, luckily
< instagibbs>
sorry, has native, comma, change is native
< gmaxwell>
morcos: I think you were responding to things like "weighed random change types"
< morcos>
correct
< BlueMatt>
more topics?
< luke-jr>
back to 12146?
< gmaxwell>
yea, I could go for weighed-random change types if the choice were otherwise neutral, but it's not.
< * BlueMatt>
notes that it really would be nice to see #12118 in 0.16, just because its really a free 5% speedup in removeForBlock which is a big chunk of block validation latency....
< instagibbs>
luke-jr, Creating utxo bloat to *slightly* reduce blocksize is weird weird feature to add imo
< instagibbs>
(if we've transitioned)
< luke-jr>
instagibbs: it's not UTXO bloat.
< BlueMatt>
luke-jr: nack, just use 0.15.1 and focus on the real fix of splitting hd chain
< instagibbs>
the scenario here is that some joker pays you, and you don't want to spend it, right?
< morcos>
+1 BlueMatt
< BlueMatt>
alternatively, this could be accomplished by using labels/accounts
< BlueMatt>
label addresses and then payments will show up without the labels
< luke-jr>
instagibbs: more or less
< luke-jr>
BlueMatt: NACK, to myself and many others, mandatory Segwit wallet is a regression
< BlueMatt>
then dont use 0.16
< luke-jr>
so 0.15 is supported forever?
< gmaxwell>
luke's argument is lame but there are other arguments for the position... But I think it's ultimately futile.
< luke-jr>
including new features?
< morcos>
luke-jr: right, but thats basically the only feature, so just dont use it until we have the version that doesn't accept all variations of key encoding
< BlueMatt>
if you want the option, do the real fix, not the dirty hack of working like 0.15.1
< gmaxwell>
luke-jr: by anyone who cares to support it
< instagibbs>
This feature doesn't make any sense to me though.
< luke-jr>
BlueMatt: there isn't a better fix
< BlueMatt>
(the real fix being splitting the hd chain so that we never identify as ours addresses other than the ones we gave out)
< morcos>
^ it's in sipas gist
< achow101>
luke-jr: the real fix is using different hd paths
< sipa>
indeed.
< BlueMatt>
which sipa proposed (I believe to much agreement) as the long-term route for the wallet
< luke-jr>
BlueMatt: that's more than a fix. :p
< gmaxwell>
instagibbs: it's very bad that we support showing funds paid to addresses we never issued. It invites really bad behavior that can cause irrecoverable funds loss. But the ship has already sailed on that.
< instagibbs>
gmaxwell, no I get the issue
< instagibbs>
I don't get his fix, it's one-sided
< gmaxwell>
ah.
< morcos>
we all agree to fixing that, what we don't care about is that we are temporarily makign it worse for the case of giving out legacy and receiving segwit
< instagibbs>
sender has no clue you won't accept it!(outside of us wagging fingers to not try it)
< gmaxwell>
Well thats because his motivation isn't one of the sensible ones, it's because he's discouraging people from using segwit to keep the blocksize down.
< BlueMatt>
luke-jr: it is more than a fix for your specific issue, but its much nicer and isnt ridiculous from a ux/maintainability perspective
< luke-jr>
that is a sensible one
< BlueMatt>
we dont need more options in the wallet...
< instagibbs>
anyways, all i have to say. Let's fix the wallet.
< gmaxwell>
I don't share his concern, but even if I did-- his fix isn't needed for it: anyone autoconverting an address has a serious risk of funds loss already.
< provoostenator>
luke-jr: what's wrong with launching with -addresstype=legacy?
< luke-jr>
BlueMatt: 12146 is trivial
< morcos>
luke-jr: would it help if we we very publicly communicated that our philosophy is to not accept payments except for addresses which have been given out?
< wumpus>
certainly not options that only exist for some political goal and no one will actually use nor test
< morcos>
we discussed doing that when we were making this change, b/c we knew we'd be making things worse on that front, but we'd already crossed the bridge with addwitnessaddress
< luke-jr>
provoostenator: the current code will still accept Segwit payments to malleated addresses
< morcos>
and other variants
< BlueMatt>
anyway, one more review on #12118 and slipping in past feature freeze would make me very happy...good to keep the march of performance improvements in block validation latency moving :)
< gmaxwell>
luke-jr: your change isn't needed to prevent autoconverting. Anyone who 'autoconverts' will have casual funds loss due to converted outputs being unreconized by older software, and _unrecoverable loss_ due to uncompressed pubkeys and potentially HSMs.
< cfields>
instagibbs: at best, if you wag your finger and I'm forced to re-send a non-witness tx, that's 2x the tx's for that interaction. Not doing much to reduce block size...
< provoostenator>
luke-jr: oh, you mean someone sees the public key after you spend it and then figures out how to send to its SegWit equivalent? I'm not sure if that actually works, see my backwards compatibility test PR.
< gmaxwell>
luke-jr: that should be ample discouragement there.
< wumpus>
morcos: would make sense to document that, though I wouldn't know where!
< jnewbery>
Doesn't seem to me like this needs to be discussed for v0.16 release. Luke can maintain his patch in knots and everyone is happy. And then we can reconsider 12146 for 0.16.1
< instagibbs>
cfields, what we need is a way of bloating the weight without taking up serialization space lol
< luke-jr>
gmaxwell: all the more reason to allow people to avoid accepting the attempts
< provoostenator>
But I don't see the point in ignoring such a transaction. There's certainly no way to stop it.
< achow101>
provoostenator: given a p2pkh address, you can easily make the p2wpkh output without needing to see a transaction
< morcos>
We can use the @bitcoin twitter handle maybe
< meshcollider>
provoostenator: you don't need the public key, segwit addresses use PKH too
< wumpus>
but a warning against 'malleating' addresses would make sense, I mean it's bad form and just because bitcoin core happens to accept it doesn't mean other wallets do
< luke-jr>
provoostenator: it's an invalid payment
< gmaxwell>
And yes, as morocos said, we knew that our handling of segwit would increase the risk that someone falsely believed they could autoconvert. It was a tradeoff we made, otherwise segwit support would be delayed behind a massive redesign.
< luke-jr>
morcos: It would probably be good to do so, but I'm not sure it accomplishes the same thing
< luke-jr>
also, you *can't* malleate right now
< morcos>
yes, so lets spend our time doing the redesign and communicating our intentions
< cfields>
instagibbs: heh
< morcos>
you can do other forms of malleation
< BlueMatt>
</topic>? It seems like everyone agrees outside of luke, and there is a *real* solution...also easy to not use 0.16 for now
< sipa>
luke-jr: you can use p2pk instead of p2pkh
< wumpus>
BlueMatt: yes, any other topics?
< Chris_Stewart_5>
PSA: if anyone is interested in talking more about #12131 or #12132 there is a development channel that some work has been occurring in: #bitcoin-mast
< luke-jr>
sipa: only if you know the key, which only the recipient does
< morcos>
aren't those a bit premature for PR's?
< arubi>
sipa, then I'll trick you into accepting some big bare n-of-m with a bunch of pubkeys
< achow101>
luke-jr: you can malleate it to a p2sh nested p2pkh
< gmaxwell>
luke-jr: I think it's adequate enough to let people know that if they do that, they _will_ lose funds, its effectively a guarentee. some switch to willfully ignore the outputs in a recoverable way wouldn't help that message at all.
< arubi>
(all of yours)
< gmaxwell>
and basically no one but you would set it.
< morcos>
is there any consensus at all around wanting those features?
< luke-jr>
achow101: nope, pretty sure we won't accept that without adding it to the wallet explicitly
< sipa>
achow101: as luke-jr says
< luke-jr>
gmaxwell: I don't agree only I would set it.
< gmaxwell>
luke-jr: you can malleat addresses now fwiw, this is a long term design flaw in the wallet.
< gmaxwell>
oh sipa pointed that out too
< luke-jr>
gmaxwell: but you can't, as I pointed out
< wumpus>
there's not really incentive for anyone to set a 'ignore some payments to me' setting, real user complaints are about not receiving payments...
< Chris_Stewart_5>
morcos: I would say they are premature -- but I think they are definitely worht exploring more
< instagibbs>
you can send people naked multisig...
< instagibbs>
lol
< Chris_Stewart_5>
if that was directed at me
< sipa>
instagibbs: indeed
< luke-jr>
instagibbs: !
< gmaxwell>
luke-jr: something like 80% of addresses are reused...
< arubi>
and with a bunch of pubkeys
< morcos>
Chris_Stewart_5: yes... i just wasnt sure if i was behind the times...
< luke-jr>
gmaxwell: unsupported use case, not relevant to the discussion, but instagibbs found an example case
< gmaxwell>
as wumpus says, the incentives are against setting that knob.
< instagibbs>
I guess that's another argument, there are tons of ways of malleating it, let's just fix the wallet instead.
< Chris_Stewart_5>
morcos: Also why we should discuss the development in a separate channel IMO as it is a little speculative still
< wumpus>
gmaxwell: also thanks to some exchanges that have a limit on the number of deposit addresses
< sipa>
instagibbs: indeed
< sipa>
DING
< wumpus>
#endmeeting
< lightningbot>
Meeting ended Thu Jan 11 20:00:10 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< * luke-jr>
glares at a certain Amazon wrapper that won't let him generate even a second address at all
< gmaxwell>
Yea, thats my point: regardless of any willfully-ignore-naughty-payments-knob other vectors exist... and if someone does segwit convert random stuff, they're going to lose funds in an unrecoverable way.
< meshcollider>
Since when do all PR mentions get added to the minutes
< achow101>
I don't think any wallet that people use actually does that sort of conversion. It would have to be someone either being malicious or using their own faulty software
< gmaxwell>
achow101: no of course not, it will immediately cause unrecoverable funds loss.
< luke-jr>
so outcome of conversation: I consider it reasonable to not have 12146 (so I won't be too upset if it doesn't get in), but not quite so reasonable to reject it for no reason (so I will be disappointed).
< gmaxwell>
achow101: one of the concerns we had with the design we used here is we chose a design where it will sometimes work... and this may encourage someone to try it with the belief that it always works.
< gmaxwell>
achow101: which is unfortunate, but no option was great.
< luke-jr>
btw, if someone can quickly unlock #11403 for conversation, I'd like to Post-merge utACK it
< meshcollider>
Yeah why can't all members comment on locked PRs/issues
< achow101>
meshcollider: you need to have write access to the repo
< achow101>
which means you could commit
< gmaxwell>
achow101: and an opt out knob really doesn't educate people more: The issue with it working at all is people incorrectly thinking that they can do it and people can collect their funds simply by upgrading. Adding a knob would make it so that they falsely believed people could collect their funds simply by flipping the knob back.
< gmaxwell>
achow101: when in reality some people are still using uncompressed keys...
< BlueMatt>
luke-jr: lol no? outcome is consider it as-is unreasonable, but if you want to fix the broader issue, sounds great!
< gmaxwell>
meshcollider: github sucks.
< achow101>
gmaxwell: oh yeah, uncompressed keys. that would be a problem
< gmaxwell>
achow101: or HSMs.
< achow101>
lol, armory has that problem...
< gmaxwell>
Someone could have their key potted in a hardware device where it can never be exported.. and which can't sign segwit style.
< gmaxwell>
when we talked about our style of segwit support, the concern was raised that because conversion would sometimes work it might cause fools and madmen to think it always works... Unfortunately, we didn't really have any good alternative for segwit support. Among other reasons backwards supporting the addwitnessaddress stuff more or less required us to go this route for now.
< gmaxwell>
also geesh, please don't call someone manipulating your address malleation.
< wumpus>
bitcoiners like to overload words with as many as possible different meanings
< gmaxwell>
We already have far too many morons out there confused due to existing reuse of the word.
< luke-jr>
gmaxwell: what do we call it then?
< luke-jr>
mutilation? :p
< gmaxwell>
Conversion? I dunno. though the surrounding issues could also arise without any conversion.
< gmaxwell>
For example, a wallet could plausably not scan for any change outputs because it knows all that it creates. If I go find one of your prior change addresses and pay it there is no reason to think the wallet would ever see it. ... and in existing software (e.g. bitcoin core) the payment wouldn't show up even if it did see the funds.
< meshcollider>
Hmm achow101 all branches are protected so then write access still does not allow members to push to them without explicit permission right?
< wumpus>
meshcollider: correct
< achow101>
meshcollider: oh!
< gmaxwell>
luke-jr: AFAIK there is no common term for "someone attempted to 'pay me' by digging a hole in by back yard and leaving an envelope of money in it"... because outside of bitcoin no one is that @#$@# stupid. :)
< luke-jr>
gmaxwell: conversion is too innocent-sounding IMO
< achow101>
write access means you can also tag things
< instagibbs>
gmaxwell, IsChange logic is also kind loopy...
< luke-jr>
anyway, gotta run
< cfields>
out of curiosity, how to handle this with segwit v1? enforce incompatibility with all previous witness versions?
< sipa>
cfields: have split chains first
< gmaxwell>
every address type should be its own chain.
< sipa>
that ^
< gmaxwell>
so you just wouldn't see such a payment.
< cfields>
right, ok
< meshcollider>
achow101: that could be a benefit :) but no I guess write access is too potent still
< gmaxwell>
that would have been the prefered path for segwit wallet support, but it requires much more major changes _AND_ isn't compatible with the addwitness stuff people were already doing.
< gmaxwell>
meshcollider: part of the problem with that too is that github might randomly stir what 'write access' could do at any time.
< meshcollider>
gmaxwell: mhm true, the permission system doesn't seem very flexible
< cfields>
petertod1: ping. Is there any canonical format for an opentimestamps proof?
< gmaxwell>
github in general has a lot of anti features.
< gmaxwell>
there was some thread on reddit where one group of ignorant people was screaming at another group of ignorant people with at claim that 0.16 wouldn't be out for a year because some random and usless github milestone "percentage" page said such and such.
< meshcollider>
Heh
< provoostenator>
gmaxwell: I really like Github's code review UI, except that it doesn't email the full thread if someone says "fixed" in response to inline comment.
< gmaxwell>
the code review has gotten better at least.
< provoostenator>
Github also completely breaks down if a PR or issue becomes political.
< gmaxwell>
not just 'political' but linked to on any part of the internet mostly populated by idiots.
< provoostenator>
It was almost impossible to have a sane discussion about replay protection on the btc1 repo; you'd have 3 people making serious arguments and 100 ranting comments from others, including from people who do write code.
< gmaxwell>
god help anyone who gets their repository linked in a youtube comment.
< provoostenator>
I suppose "political" is a euphemism for "linked to on any part of the internet mostly populated by idiots"
< Chris_Stewart_5>
lol
< gmaxwell>
They're highly correlated at least. :)
< provoostenator>
I sometimes tweet out PR's; haven't seen too negative results for the Core ones, but I would think twice doing that for anything contentious.
< gmaxwell>
I'm not sure about any of your tweets, but sometimes it does.
< gmaxwell>
e.g. the announcements re the merge of the segwit PR inspired a half dozen really nasty comments on that PR (which people just deleted)
< provoostenator>
I only have 1.5K followers and anyone who SegWit has probably unfollowed me by now.
< bitcoin-git>
[bitcoin] kekimusmaximus opened pull request #12159: Use the character based overload for std::string::find. (master...use_char_overload_find) https://github.com/bitcoin/bitcoin/pull/12159
< gmaxwell>
I probably should have brought up #11739 during the meetings.
< gribble>
https://github.com/bitcoin/bitcoin/issues/11739 | RFC: Enforce SCRIPT_VERIFY_P2SH and SCRIPT_VERIFY_WITNESS from genesis by sdaftuar · Pull Request #11739 · bitcoin/bitcoin · GitHub
< promag>
late suggestion, but how about -changetype=auto? (being that the default)
< bitcoin-git>
[bitcoin] jtimon opened pull request #12172: Bugfix: RPC: savemempool: Don't save until LoadMempool() is finished (master...b16-bugfix-savemempool) https://github.com/bitcoin/bitcoin/pull/12172
< jtimon>
being a bugfix, could #12172 get in for 0.16 ?
< luke-jr>
jtimon: IMO savemempool shouldn't succeed until it is actually saved, so if you make it delay the save, the RPC probably needs to block, which is complex
< jtimon>
luke-jr: yeah, that's an interesting point but kind of orthogonal, no? (although definitely related)
< * BlueMatt>
is of the opinion #12118 can be merged
< jtimon>
luke-jr: perhaps I can add something to the documentation while at it?
< bitcoin-git>
[bitcoin] jonasschnelli opened pull request #12173: [Qt] Use flexible font size for QRCode image address (master...2018/01/fix_qr_font) https://github.com/bitcoin/bitcoin/pull/12173
< instagibbs>
provoostenator, I think we have different definitions of actionable, haha
< jonasschnelli>
sipa: mind doing a short review on #11281 (since you have raised some issues there)?
< sipa>
instagibbs: is "cheaper" actionable? (just trying to understand your definition)
< instagibbs>
sipa, of course it is, but the comment was about privacy(which we agree users won't be able to act on)
< sipa>
instagibbs: i'm confused
< sipa>
instagibbs: how is "cheaper" actionable but "better privacy" isn't?
< instagibbs>
the language says "may" or somesuch, and if the user googles, or whatever, they'll find nothing. I *guess* the could just get scared and never set it.
< instagibbs>
so I take it back, whatever, it's just a super verbose message imo
< sipa>
instagibbs: oh no disagreement that it's too verbose
< instagibbs>
also if uptake is quick(doubt, but possible) that users will be confronted with the message and go the wrong direction
< instagibbs>
i think release notes might be the best place
< promag>
and in validation.cpp just use mapBlockIndex directly
< BlueMatt>
unless you want to make things more scripted-diff...
< BlueMatt>
no, not validation.cpp, CChainState
< BlueMatt>
outside of CChainState should all be const CBlockIndex, even in validation.cpp
< BlueMatt>
but that may not be trivially possible quite yet
< BlueMatt>
promag: you saw my outstanding pr to constify them with scritped-diffs, right? You're welcome to replace it if you want, but no use duplicating effort wholesale...
< BlueMatt>
promag: also, generally, that pr is already one giant commit that does like 3 things....please try to script what you can and cut it down
< promag>
+1
< promag>
curiosity, an off chain block could have 0 confirmations, why -1?
< bitcoin-git>
[bitcoin] MarcoFalke reopened pull request #12089: qa: Make TestNodeCLI command optional in send_cli (master...Mf1801-qaCliOptions) https://github.com/bitcoin/bitcoin/pull/12089
< promag>
gmaxwell: "it's a mistake to say "bech32" -- there is no address involved with change"
< promag>
listunspent includes changes right? what will be "address" for change utxo?
< sipa>
yes, bech32
< gmaxwell>
promag: you're missing my point. One can use scripts which no address can be defined.
< gmaxwell>
er s/can be/is/
< sipa>
but if bech32 didn't exist, there would just be no way to show an address for such outputs, but they would have been totally possible to use
< gmaxwell>
And if there were no bech32 defined we still could be using p2wpkh as change.
< gmaxwell>
The fact that there is an address is incidental.
< gmaxwell>
There are also other address encodings for that same script, e.g. BIP142 (though hopefully no one uses them)
< sipa>
for certain types of addresses there even exist no address that can be derived from the output (e.g. ECDH based constructions like stealth addresses or CT)
< sipa>
listunspent would just be unable to show an address for such outputs