< bitcoin-git>
bitcoin/master d9180c5 Wladimir J. van der Laan: Merge #20145: contrib: add getcoins.py script to get coins from (signet) f...
< bitcoin-git>
[bitcoin] laanwj merged pull request #20145: contrib: add getcoins.py script to get coins from (signet) faucet (master...202010-signet-getcoins) https://github.com/bitcoin/bitcoin/pull/20145
< Kiminuo>
MarcoFalke, Hi Marco, could you say whether it's worth working on std::fs at this time? You have, I think, mentioned that `std::fs` is not supported by some OSs. So I just want to know whether it makes sense to work on this or let it wait a few months or a few years for next upgrade (https://github.com/bitcoin/bitcoin/pull/19245#issuecomment-730291399)
< MarcoFalke>
It would mean that compilation on Ubuntu 18.04 no longer works, I think
< MarcoFalke>
I might have to check again
< Kiminuo>
Thanks for info
< wumpus>
no opinion on timespan but one thing to be really careful with when transitioning to std::fs is that the unicode character subtleties still work, across OSes
< wumpus>
including interaction between Qt and it, especially on windows there has been a history of issues there
< wumpus>
unfortunately, locale stuff tends to be really awful in C standard libraries
< Kiminuo>
wumpus, Yes, I have tried to collect PRs for "fs" to see what the issues were. I can only say that it *tentatively seems* that std::fs behaves nicer wrt unicode than Boost. But it will be a long ride and maybe I'm wrong. It's hard to guess at this point.
< bitcoin-git>
bitcoin/master ea93bbe practicalswift: init: Fix incorrect warning "Reducing -maxconnections from N to N-1, becau...
< bitcoin-git>
bitcoin/master 7aa9456 Wladimir J. van der Laan: Merge #20024: init: Fix incorrect warning "Reducing -maxconnections from N...
< bitcoin-git>
[bitcoin] laanwj merged pull request #20024: init: Fix incorrect warning "Reducing -maxconnections from N to N-1, because of system limitations" (master...fix-incorrect-warning) https://github.com/bitcoin/bitcoin/pull/20024
< wumpus>
I guess if these are really urgent, please help reviewing them
< MarcoFalke>
wumpus: Don't worry abou those. They are minor RPC inconsistencies
< hebasto>
the tagged for backporting, no?
< wumpus>
if not, let's move them to the 0.21.1 milestone instead
< luke-jr>
wumpus: feature_segwit test is failing too
< MarcoFalke>
I am sure we will find more issus for rc2
< sipa>
inconsistencies in newly introduced stuff?
< jnewbery>
hi
< MarcoFalke>
sipa: The feerate things ...
< luke-jr>
sipa: what we discussed the other night
< sipa>
sounds like it - if so i think we'll want those in 0.21 anyway i guess
< murch>
Sending feerates were restricted to >0 instead of >=0
< murch>
mea culpa, I missed it in the review
< MarcoFalke>
Also, "" == null 0==null
< MarcoFalke>
and the third one a minor issue with upgradewallet
< sipa>
no objection to doing rc1 without them in that case, but if it means there'll be an rc2 anyway.... up to wumpus i guess
< wumpus>
anyhow right now they're holding up the release
< luke-jr>
more of a beta1 than a rc1 in that case :P
< MarcoFalke>
I'd be highly surprised if this was the first major release with just one rc
< sipa>
ha, fair
< MarcoFalke>
we'll find more bugs, don't worry
< wumpus>
anyhow merging the feerate last minute was probaly ill advised so I understand if last minute issues came up
< sipa>
luke-jr: i wonder why you're the only one hitting this
< MarcoFalke>
wumpus: It had to be merged (or everything reverted)
< sipa>
luke-jr: is signing or hashing particularly slow on your system?
< luke-jr>
wumpus: well, it was really a bugfix for the explicit fee rate mess merged months ago
< wumpus>
MarcoFalke: yes, but it should have been merged sooner in the release cycle
< luke-jr>
sipa: I saw it on CI too?
< sipa>
oh, ok
< MarcoFalke>
wumpus: Agree
< jonatack>
I think they've been addressed and those PRs are close to RFM. There is a lot of test coverage in place now, which helps.
< MarcoFalke>
luke-jr: feature_taproot?
< luke-jr>
sipa: no reason it would be - I have 16 SMT4 cores
< sipa>
MarcoFalke: there is a simple fix; aj posted one the other day, i think an even simpler one is possible
< sipa>
i'll PR soon
< MarcoFalke>
wumpus: This release is the heavies ever in features merged per time, I think
< MarcoFalke>
sipa: Thx, I'll ack
< luke-jr>
MarcoFalke: yes, sorry
< MarcoFalke>
so the delay was expected
< wumpus>
MarcoFalke: yes, seems like it :)
< luke-jr>
MarcoFalke: looks like it simply builds a huge wallet and then times out in coin selection
< sipa>
yeah, or quadratic hashing of legacy spends
< MarcoFalke>
luke-jr: No coin selection. It is signrawtxwithwallet
< MarcoFalke>
(or a bug in the wallet)
< luke-jr>
I thoguht it was the create
< wumpus>
it fails on 0.21 branch but not on master?
< aj>
sipa, luke-jr: want me to PR that patch?
< luke-jr>
aj: sgtm
< MarcoFalke>
wumpus: It fails on slow CPUs ;)
< luke-jr>
MarcoFalke: my CPU is nothing close to slow..
< MarcoFalke>
CI should be unaffected, because it has the timeouts cranked up to max
< wumpus>
I haven't noticed any tests failing on master but haven't locally run the tests for the 0.21 branch yet
< wumpus>
(because there is no divergence yet)
< achow101>
I've had feature_taproot fail occasionally, but only with the test runner
< MarcoFalke>
luke-jr: Its not intel, so it doesn't have the speedup-backdoors
< sipa>
that could be it
< MarcoFalke>
And running test_runner with high --jobs might also cause it
< sipa>
SHA-NI hashing is a lot faster
< achow101>
MarcoFalke: that's what I ran into
< jonatack>
achow101: same here
< wumpus>
it would definitely be good to not have the tests dependent on specific intel optimizations :)
< sipa>
it's just silly
< luke-jr>
aj's optimisation seems like a good idea even with longer timeouts
< sipa>
we could also split the test in two, and run it once for post-activation and once for pre-activation
< michaelfolkson>
sipa said he runs -j60. I'm assuming that is too high. Something like -j5 better?
< luke-jr>
lol?
< wumpus>
disable-all-timeouts mode would be useful in that case xD
< michaelfolkson>
Or that's what I remembered. Am I remembering incorrectly...?
< sipa>
-j60 works fine here
< MarcoFalke>
wumpus: The ci runs it, but we can't enable that locally
< sipa>
and seems around the fastest in overall clock time
< luke-jr>
michaelfolkson: as long as -j is set sensibly, the number itself shouldn't make problems
< MarcoFalke>
I guess we can scale the timeout with --jobs
< jonatack>
michaelfolkson: i've adopted -j60 since sipa mentioned it, and added the suggestion as an option to try in the compile guide
< luke-jr>
I was getting it 100% of the time, with just the one test
< wumpus>
I'm not sure that needs to be done automatically, but instructions on how to do it for people running into timeouts would be useful
< sipa>
luke-jr: i think it's fair to say the test is just broken, and it's masked by the fact that almost everyone runs it on hardware where SHA256 is sufficiently optimized to mask the issue
< jonatack>
michaelfolkson: (not for make, just for running the functional suite)
< wumpus>
hehe
< MarcoFalke>
Jup, each raise of a timeout error could mention "You can use --timeout_factor to scale the timeouts"
< luke-jr>
MarcoFalke: but it shouldn't be necessary
< wumpus>
MarcoFalke: good idea!
< aj>
sipa: seems like there's something wrong with signrawtx in that it's taking 20s (for me) or 30s+ (for luke) as well though
< luke-jr>
if a test hits it normally, that means the timeout is too low
< michaelfolkson>
jonatack: Let me try make with -j60... :)
< miketwenty1>
does -j1 or no -j flag fail?
< MarcoFalke>
with enough ram you can do -j 999
< sipa>
miketwenty1: -j1 is just painfully slow
< luke-jr>
miketwenty1: I was running the test by itself, nothing else
< sipa>
aj: quadratic hashing of enormous (non-standard!) transactions is a known issue
< miketwenty1>
i know it's slow i've compiled bitcoin like 5 times today with no -j flag
< wumpus>
the thing with test timeouts (especially for CPU intensive ones) is that it's kind of a random barrel shoot, you can always have a slower machine where it fails
< MarcoFalke>
sipa: That patch would still fail intermittently
< sipa>
MarcoFalke: how so?
< luke-jr>
miketwenty1: alias make='make -j60'
< MarcoFalke>
sec ...
< jonatack>
miketwenty1: use ccache? see doc/productivity.md
< jonatack>
miketwenty1: also build only what you need
< miketwenty1>
the hardware that's compiling is purposely small
< wumpus>
alias invokeoomkiller='make -j60'
< luke-jr>
wumpus: unless the timeout is set high enough it only fails when there's hanging
< luke-jr>
wumpus: lol
< luke-jr>
ok, I really do make -j32
< MarcoFalke>
sipa: Maybe coin selection picking only the small coins?
< luke-jr>
(but having it aliased really helps me use it)
< sipa>
MarcoFalke: how would it fail?
< miketwenty1>
what is the logical way to calculate how many threads or the -j<n> count you can use for a machine?
< MarcoFalke>
sipa: It picks a lot of small coins, which takes long to sign?
< MarcoFalke>
coin selection isn't deterministic and the amounts in the test aren't either
< sipa>
MarcoFalke: with just 70% of the balance there should be no need for many tiny coins
< luke-jr>
miketwenty1: start with number of cores + 1
< luke-jr>
miketwenty1: possibly including SMT in core count
< sipa>
miketwenty1: for building, not more than your number of threads + 1, and assume around 1 GB of memory per -j
< wumpus>
miketwenty1: in my experience it's about 1.5GB memory per -j, for the C++ build, for running the functional tests it's different though
< sipa>
miketwenty1: for functional tests, you can go way higher than what your thread count permits
< wumpus>
I remember it was quite a difficult merge in the first place, maybe we shouldn't have done it, but I remember some were saying it was really important to get it in at the time
< achow101>
the wallet fix just does what we do in master. it's not quite a backport because we kind of got to this result in master by accident through 3 or so prs
< aj>
wumpus: i think there's reasonable confidence, but there's no need for it anymore either
< MarcoFalke>
achow101: That might make it harder to review
< aj>
wumpus: (19680 and 19681 were the backports)
< wumpus>
sipa: thanks
< jnewbery>
wumpus: should be trivial to back it out. It wasn't actually too difficult to backapply, and as Marco says, if the diff after the revert is empty then it's a trivial ACK
< wumpus>
jnewbery: yes, it's easy to revert (not much happend on the 0.20 branch since), I was just confused about the change in priorities suddenly
< jnewbery>
wumpus: me too :)
< MarcoFalke>
wumpus: I think there was confusion generally, and probably an assumption that no 0.20 release was planned any time soon
< wumpus>
MarcoFalke: yes, I do remember that we wanted to do the next 0.20 release after 0.21.0
< luke-jr>
not so sure about the latter
< MarcoFalke>
wumpus: Jup, that was under the assumption that wtxid relay was going to be backported
< wumpus>
in any case please review the revert and other PRs for 0.20.2
< wumpus>
when they're merged we can tag 0.20.2rc1 as well
< wumpus>
any other topics?
< wumpus>
I guess we can pick up high priority for review again
< wumpus>
#topic High priority for review
< core-meetingbot>
topic: High priority for review
< aj>
sipa: your fundrawtx patch seems to take 0.2s rather than 19.8s for me
< luke-jr>
btw, 0bin is a nuisance for patches… :p
< sipa>
luke-jr: use "copy to clipboard"; that avoids some formatting/indenting
< sipa>
i want to try the patch on my odroid (as it also has unoptimized SHA256)... but it's been over a year since i logged into it and i have no idea what its password is
< sipa>
i'm even more surprised it's still running
< luke-jr>
sipa: yes, and then move the clipboard to my dev side, and can no longer see the patch URL in my BASH history. :/
< aj>
sipa: if i change it from 70% to 99% it takes 1.5s instead
< aj>
sipa: but hmm, if i set it to 100% with subtraceFeeFromOutputs I get a "Transaction too large" error, so doesn't this just give the same behaviour as when sendtoaddress was used?
< sipa>
aj: oh, hmm.
< sipa>
your approach may be better then
< bitcoin-git>
[bitcoin] luke-jr closed pull request #20121: configure: Allow users to explicitly enable libsecp256k1's GMP bignum support (master...secp256k1_allow_bignum) https://github.com/bitcoin/bitcoin/pull/20121
< bitcoin-git>
[bitcoin] ajtowns opened pull request #20428: tests: split feature_taproot transfer of funds into smaller txs (master...202011-test-taproot-signmany) https://github.com/bitcoin/bitcoin/pull/20428
< luke-jr>
sipa: aj: so what do I test now? or all done?
< aj>
luke-jr: my patch is up at 20428 (above), re-test if you'd like but should be fine
< luke-jr>
tACK'd
< luke-jr>
aj: it'd be a clean merge to both branches if you rebase it onto fac865b72d5c0e01fce74b84ab21e5ebbf069327
< aj>
sipa: hmmmm.... if i sort the unspends by amount first, i only need the top 500 of them to hit 70% (with ~4200 utxos remaining)
< sipa>
aj: heh
< aj>
sipa: top 500 seems to be about 90% of the value
< sipa>
i think your first "top 500" is a typo
< aj>
sipa: i was doing them in batches of 500, expecting i'd need more than one batch
< aj>
luke-jr: rebased on top of the merge commit
< sipa>
aj: you stated that the top 500 is both 70% and 90%... so i assume one of the two at least is wrong
< aj>
sipa: it's just under 90%, but the first statement was nevertheless not a typo :)
< sipa>
ok, not a typo, but still wrong?
< * sipa>
very confused
< aj>
sipa: i was rounding up to the nearest 500
< aj>
sipa: (i'd tell you how many outputs were need for 70%, but i haven't worked it out yet. presumably it's 390ish?)
< sipa>
<ultranitmode> rounding up, or rounding the nearest? </ultranitmode>
< aj>
up, but i wanted to be clear i'm not skipping the next 500!
< aj>
in case you thought the real answer was -300 coins were needed, i guess
< aj>
sipa: 270 coins got me to 70%
< aj>
sipa: so, sort lisunspent, take the 500 with highest amount, and be done with it maybe? or have a check that the top 500 are at least 70% of the total as well?
< sipa>
aj: pretty sure you can take the top 10 outputs too, and be done with it :)
< sipa>
only 10 transactions are performed with that amount
< miketwenty1>
would this typically be the time people built, verified v0.21.0rc1 ? I would like to be signer going forward.
< sipa>
miketwenty1: if you want to be a gitian signer for rc1, now is the time
< miketwenty1>
sipa: hold on.. let me put my cape on..
< miketwenty1>
k ready
< achow101>
miketwenty1: yes
< miketwenty1>
so.. i have gitian builds with 1 click.. only thing you need to specify is the tag.. and it builds with terraform module
< miketwenty1>
im guessing this would belong in bitcoin-core or maybe personal repo
< luke-jr>
aj: does your simplification break if unrelated parts of the test change in the futrue?
< sipa>
luke-jr: very unlikely
< emzy>
where do I get the Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers.tar.gz for the gitian build?
< miketwenty1>
this is my first time doing a complete terraform from scratch build of a bitcoin with gitian process.. can someone tell me if they also got 073aaacb86e8898c4806ed10cd14d54869f31e353f86b60bb83f93ae89a3591d for bitcoin-0.21.0rc1-x86_64-linux-gnu.tar.gz