< sipa> d/query gwillen
< sipa> oops
< bitcoin-git> [bitcoin] fanquake opened pull request #20336: build: automatically determine macOS base translations (master...auto_find_translations) https://github.com/bitcoin/bitcoin/pull/20336
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20339: ci: Run macos ci config on cirrus (master...2011-moreCirrus) https://github.com/bitcoin/bitcoin/pull/20339
< bitcoin-git> [bitcoin] fanliao1989 opened pull request #20343: Update README.md (master...master) https://github.com/bitcoin/bitcoin/pull/20343
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #20343: Update README.md (master...master) https://github.com/bitcoin/bitcoin/pull/20343
< bitcoin-git> [bitcoin] theStack opened pull request #20344: wallet: fix scanning progress calculation for single block range (master...20201108-wallet-avoid_div_by_zero_on_single_block_rescan) https://github.com/bitcoin/bitcoin/pull/20344
< bitcoin-git> [bitcoin] SatoshiBitcoin opened pull request #20345: Update README.md (master...Version1) https://github.com/bitcoin/bitcoin/pull/20345
< bitcoin-git> [bitcoin] SatoshiBitcoin closed pull request #20345: Update README.md (master...Version1) https://github.com/bitcoin/bitcoin/pull/20345
< andytoshi> achow101: i think there is a bug in CreateTransaction related to bnb .. can you sanity check me before i file a bug
< achow101> andytoshi: what's the bug?
< andytoshi> one sec, typing
< andytoshi> the problem is that when we create txNew for the purpose of getting a tx size to estimate the fee required, in this line https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2940 we selectively add a change output based on whether bnb was used by SelectCoins
< andytoshi> so far so good
< andytoshi> but .. if we use bnb, but we don't get enough fees, and we have subtractfeefromeamount on, we hit this line https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L3038
< andytoshi> which says "SelectCoins succeeded, we didn't hit the fee we needed, but that's fine, we can increase the fee by subtracting from outputs. turn off pick_new_inputs and loop again"
< andytoshi> but then, when we do the next loop iteration, we skip SelectCoins and just set bnb_used to false https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2928
< andytoshi> which means that when we recalculate nFeeRequired, we'll get a higher number than we did before. we set nFeeRet to nFeeRequired at the end of the previous iteration. so now this check will fail https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2984
< andytoshi> ..and the whole loop will fail, because we say "well, we didn't hit our fee target, and pick_new_inputs was off, so we must be screwed"
< andytoshi> but actually the problem is just that we didn't compute nFeeRequired consistently from one iteration to the next
< andytoshi> (end of description)
< achow101> how is bnb used but not enough fees?
< luke-jr> ARGH, google has decided "dnsseed.bitcoin.dashjr.org" has deceptive content is now blacklisting all of dashjr.org -.-
< achow101> with subtractfeefromamount, unless the amount is less than the fees?
< andytoshi> lemme take a look
< luke-jr> sipa: bluematt: cdecker: jonasschnelli: petertodd: sjors: emzy: this will probably hit you soon too
< jonatack> luke-jr: confirmed
< jonatack> "Deceptive site ahead. Firefox blocked this page because it may trick you into doing something dangerous like installing software or revealing personal information like passwords or credit cards. Advisory provided by Google Safe Browsing."
< emzy> luke-jr: benause of the dnsseed. It has nothing to do with http...
< emzy> https://dashjr.org/ works vor me in Firefox.
< jonatack> dnsseed.bitcoin.dashjr.org has been reported as a deceptive site. You can report a detection problem -> I filed an report
< achow101> andytoshi: regardless, i'm trying to get at least the effective value pr for the next release so there won't be any such bug as the loop is removed
< emzy> luke-jr: you are sure that they will blacklist all of dashjr.org? I think the http(s) blacklist of dnsseed.bitcoin.dashjr.org is right.
< achow101> luke-jr: is the warning part of chrome or is it some dns thing?
< achow101> or something else?
< jonasschnelli> My DNS seed runs no http site (I guess).
< andytoshi> achow101: cool, thanks
< jonasschnelli> Or does it even matter what runs on the seed machine?
< andytoshi> though there's maybe an 80% chance this is actually some elements weirdness related to how we account for change in coin selection
< emzy> jonasschnelli: the A records you deliver are not your IP addresses anyways.
< emzy> As long as only the whole domain is blacklisted for browsing it is totaly fine.
< emzy> I would argue it is right to do it. Because it could lead to a malicious site.
< andytoshi> achow101: oh i think my issue may have been resolved in https://github.com/bitcoin/bitcoin/pull/17458
< andytoshi> certainly the logic before that PR is way harder to understand
< luke-jr> jonatack: well, the dnsseed domain probably should get flagged - the problem is they're extending it to my entire domain :/
< luke-jr> achow101: I assume it's part of Google's existing censorship framework
< luke-jr> jonasschnelli: it points at random IPs, which may be running malicious webservers..
< jonasschnelli> Yes. That. But it has always been that way.
< andytoshi> achow101: i think i'm right, this is a core thing not an elements thing ... https://github.com/bitcoin/bitcoin/blob/7e373294a5ae819099c39d9d03d1f5a311d63cfc/src/wallet/wallet.cpp#L3016-L3019 this comment says "nFeeNeeded should not have changed"
< andytoshi> 20:26 < achow101> how is bnb used but not enough fees?
< andytoshi> what's happening here is that bnb is used and it gives us -some- fees, but if we're subtracting fees from outputs it doesn't bother trying to give us enough
< achow101> I suppose bnb assumes that the output is enough to cover the fees
< achow101> but that may not be the case
< andytoshi> the logic is that we'll fail the nFeesRet >= nFeesNeeded check, but then we'll go around the loop again
< andytoshi> it is the case
< andytoshi> but the code is still wrong
< andytoshi> in my code the output is more than enough to cover the fees, the problem is that we don't subtract enough (or rather, we subtract enough but then incorrectly think we should've subtracted more)
< achow101> do you have a particular test case I can try?
< andytoshi> sorta, i can give you an elements functional test from before #17458
< gribble> https://github.com/bitcoin/bitcoin/issues/17458 | Refactor OutputGroup effective value calculations and filtering to occur within the struct by achow101 · Pull Request #17458 · bitcoin/bitcoin · GitHub
< andytoshi> let me try to extract a bitcoin example
< andytoshi> because i'm pretty confident that the issue has nothingh to do with elements
< achow101> Are you hitting the "Transaction fee and change calculation failed" error?
< andytoshi> yep
< bitcoin-git> [bitcoin] tylerchambers opened pull request #20346: script: modify security-check.py to use "==" instead of "is" for literal comparison (master...literal-comparison-update) https://github.com/bitcoin/bitcoin/pull/20346
< andytoshi> i think, if you have a wallet where bnb can solve exactly (say you have two 10btc inputs and want a 20btc output) and you put subtractfeesfromoutput on, you'll trigger this
< achow101> andytoshi: just tried, and nope
< andytoshi> hmmmm, i don't understand. i'll have to run it myself with a pile of logging added
< andytoshi> can you push a diff somewhere to add a test case?
< andytoshi> since you have one
< andytoshi> can i get log output from unit tests?
< achow101> unit tests make a dir in /tmp that should have logs
< andytoshi> thanks!
< andytoshi> i almost have a unit test, i am just trying to set a non-zero feerate without segfaulting..