< wumpus>
andytoshi: it doesn't really know, there's a manual fetch of all the repositories in there IIRC< it spooked me too once
< wumpus>
or no there isn't, I don't really know
< wumpus>
but I ran into some issues when I had to use a non-standard subtree for some temporary patches / experimentation in a PR (for a leveldb update)
< wumpus>
on one hand I'm happy no longer being the only person running into this, on the other it would be good if it had better documentation
< wumpus>
err wait the 'error' exit has a positive exit status, riight
< wumpus>
I'll add an argument to the script instead
< wumpus>
and help
< sipa>
great
< bitcoin-git>
[bitcoin] laanwj opened pull request #20567: test: Add option to git-subtree-check to do full check, add help (master...2020_12_subtree_check) https://github.com/bitcoin/bitcoin/pull/20567
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20568: rpc: Use FeeModes doc helper in estimatesmartfee (master...2012-rpcDocFee) https://github.com/bitcoin/bitcoin/pull/20568
< promag>
MarcoFalke: why just those 2 cases in #20566?
< wumpus>
why is all the addrv2 discussion happening last minute between rcs
< jnewbery>
there were lots of very large features merged just before feature freeze, so it's maybe not surprising that issues are being found now
< wumpus>
my first public draft was apparently june 1, 2018, I posted it to the mailing list feb 18, 2019 (there we no replies at all), I created the PR for the BIPs repository on feb 27, 2019, it was merged jul 23, 2019. Vasild picked up the implementation april this year, and quite quickly had a working implementation (though some other detailed were still ironed out). It was discussed in
< wumpus>
meetings multiple times.
< wumpus>
does any of this feel rushed to you
< wumpus>
I am still struggling to find out what I did wrong for even such a relatively straightforward proposal
< wumpus>
I don't think people are exagerrating when they claim it has become almost imposible to make changes to bitcoin, even non-consensus, maybe this is a good thing! I don't know
< wumpus>
but it's too frustrating for me personally and I don't think I'll be working on BIPs again
< wumpus>
it wasn't even controversial in any way
< vasild>
indeed, why was this brought up now? the btcd issue for disconnecting was opened in btcd's repo 20+ days ago
< wumpus>
nor even a new idea! people had been talking about a new address mechanism for years before that, I mainly collected some ideas people already had
< wumpus>
and wrote them down
< vasild>
what about releasing 0.21 as is and _considering_ #20564 for 0.21.1 based on reception of 0.21?
< wumpus>
vasild: well, it being a temporary workaround it should probably be merged quickly
< wumpus>
sipa's change is very low risk in itself
< vasild>
I agree it is low risk and can be merged quickly, but I dont see it being temporary.
< wumpus>
what people are (rightly) being worried about is the implications of this behavior
< wumpus>
yes
< vasild>
so we have team1 insisting that new messages should come with a protocol bump and impl1 should disconnect when it sees unknown message, team2 thinks it is ok to just ignore unknown messages.
< vasild>
if team1 still insists and team2 follows suit, I don't see team1 changing their mind...
< wumpus>
I don't agree at all that ignoring unknown messages is a DoS vector, if you want to DoS it's better to use a known message type that consumes resources
< vasild>
right
< wumpus>
or that has a large response for a small message
< wumpus>
just flooding a host is uninteresting and possible in so many ways
< vasild>
That is kind of obvious, but apparently not for everybody. My worry with 20564 is that it is going to reaffirm "new messages come with protocol bump"
< wumpus>
and while I do agree the P2P code could use some more anti-DoS measures this is not one
< wumpus>
right
< wumpus>
vasild: maybe, though at least everyone in the PR agrees that it's a one time thing -- this says nothign about signaling to other implementations of course
< vasild>
:)
< jonasschnelli>
<wumpus> my first public draft was apparently june 1, 2018, I posted it to the mailing list feb 18, 2019 (there we no replies at all), I created the PR for the BIPs repository on feb 27, 2019, it was merged jul 23, 2019. Vasild picked up the implementation april this year, and quite quickly had a working implementation (though some other detailed were still ironed out). [...]
< jonasschnelli>
This is my main concern... fixing this quietly.
< jonasschnelli>
It might lead to more reluctant testing and lesser interest to read BIPs more carefully
< wumpus>
... even less interest and we might as well puglish BIPs directly into the arctic code vault
< wumpus>
but I agree, the whole BIP process is there to standardize the bitcoin protocols, to coordinate between implementations, that's the point
< wumpus>
well, it also helps coordinate between bitcoin core developers on a more conceptual level than implementation details, but that discussion happened on the BIP repo PRs, not on the mailing list
< wumpus>
maybe it should have been on the mailing list but I'm not sure most of the people commenting there are even subscribed there, not least because a lot of people consider mailing lists an awful communication medium
< wumpus>
it's also perfectly normal to find and fix issues in rcs at the implementation level, what is weird here is that BIP-level conceptual issues are raised now
< harding>
FWIW, addrv2 was covered by the Optech newsletter six times total, three times this year. https://bitcoinops.org/en/topics/addr-v2/ We also directly covered sdaftuar's March 2020 mailing list post about using new messages for feature negotiation with the call to action, "Daftuar is seeking feedback from maintainers of full nodes about whether the insertion of negotiation messages would cause any problems. If you’re aware of any
< harding>
problems, please reply to the thread." http://bitcoinops.org/en/newsletters/2020/03/04/#improving-feature-negotiation-between-full-nodes-at-startup Later (August), about the new wtxidrelay message, we wrote, "This behavior is already implemented in an unreleased development version of Bitcoin Core. If the maintainers of other implementations, or anyone else, opposes this change, please respond to the mailing list thread."
< wumpus>
harding: oh wow thanks for weighing in, I even forgot about that
< jonatack>
Yes, it's not like we weren't publishing repeatedly about BIP 155 addrv2 arriving and testing, tweeting about it numerous times, people responding enthusiastically, etc.
< jonatack>
also 2 review club sessions about it, one of which was covered with a monthly Optech feature writeup and quiz
< jonatack>
I guess it's human nature to wait until things are really impending though
< jonasschnelli>
I don't get this... configure.ac sets AC_SUBST(HAVE_WEAK_GETAUXVAL), this means it will not get evaluated in #if defined(HAVE_STRONG_GETAUXVAL) || defined(HAVE_WEAK_GETAUXVAL), right?
< jonasschnelli>
(without setting CPPFLAGS to -DHAVE_WEAK_GETAUXVAL=@HAVE_WEAK_GETAUXVAL@)
< jonasschnelli>
There is no AC_DEFINE for HAVE_WEAK_GETAUXVAL but randomenv.cpp is evaluating it. My guess is that getauxval will never be used in randomenv.cpp.
< sdaftuar>
wumpus: just to share my opinion -- the issue in my mind is that this is a surprise to anyone that we implemented a feature that will cause existing software to disconnect us. i think it shouldn't have been a surprise to us, because every time this topic comes up on the mailing list, there is opposition to the idea of sending unknown messages without a protocol bump (as i rediscovered in august)
< sdaftuar>
i know this BIP was advertised in several places, but the updated draft of it was never sent to the -dev list after the negotiation changed; so i think that's something that would have brought this to light to everyone sooner
< sdaftuar>
at any rate, i'm not trying to advocate that we necessarily change our feature negotiation. i know lots of other people ahve strong views on it, and i don't myself
< sdaftuar>
i just think that we should all be aware that this is an area where people disagree, and our communication should reflect that by making it clear what our software will do
< sdaftuar>
(anyway, i also did basically zero work to help addrv2 make it this far, so thank you to everyone who has worked on it! i hope my words don't come across as criticism of the overall effort)
< wumpus>
I didn't bother with them ailing list anymore because the first time it got exactly zero engagement
< sdaftuar>
wumpus: yeah i have encountered that myself in the past too :(
< wumpus>
I don't personally expecti t would have been different when sending it a second time but feel free to disagree
< wumpus>
okay :/ yes I can only imagine how difficult it is to make say, consensus changes
< wumpus>
if even seomthing like this...
< sdaftuar>
my thought is that even if people don't engage, by sending it to the mailing list (which has historically been our place of publication of protocol changes) it shifts the burden to others to read, respond, or object
< MarcoFalke>
it is impossible to know how much reading engagement a post has received if the majority of subscribers are readers
< MarcoFalke>
s/if/because/
< wumpus>
sure...
< wumpus>
it was kind of demotivating for a while in itself though
< kanzure>
what is the mechanism by which ignoring an unknown message causes a DoS problem?
< sdaftuar>
the great question :)
< wumpus>
disconnecting literally generates more traffic than ignoring the message
< kanzure>
MarcoFalke: readers also can't be determined on a github pull request or issue comment.
< MarcoFalke>
wumpus: I think it was an oversight of the btcd devs
< MarcoFalke>
The first comment says "Thanks for reporting this! The easiest way to resolve this is to permit unknown messages rather than disconnecting as we do now"
< MarcoFalke>
Maybe the btcd devs wouldn't have noticed even if the bip was sent to the mailing list
< MarcoFalke>
it is easy to forget details of how the p2p protocol is implemented (or even how it changed in the past)
< MarcoFalke>
code/bip review can never catch all issues upfront. They come up when implementing or running tests on real networks
< MarcoFalke>
we are the first ones to implement the bip, so no one else had the chance to spot bugs while implementing
< bitcoin-git>
bitcoin/master 773c42b Andrew Chow: tests: Test that a fully signed tx given to signrawtx is unchanged
< bitcoin-git>
bitcoin/master 751ffaa MarcoFalke: Merge #20562: tests: Test that a fully signed tx given to signrawtx is unc...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20562: tests: Test that a fully signed tx given to signrawtx is unchanged (master...test-signraw-fullysigned) https://github.com/bitcoin/bitcoin/pull/20562
< Provostro>
Hi I asked about LedgerX and they told me to come here and speak with Kanzure
< Provostro>
I was checking it out, but it seems like a scam.
< Provostro>
the order books are scammy
< Provostro>
kanzure, are you running a scam?
< Provostro>
!tlast
< gribble>
18856.24
< luke-jr>
Provostro: this channel is for Bitcoin Core development, not for speaking with specific people about other topics
< Provostro>
I'd like to settle matter amicably, thats why i came to you to ask.
< Provostro>
TO please stop running your scam, or pay me 100 btc.
< luke-jr>
Provostro: /query kanzure
< Provostro>
luke-jr, ok i typed that, now what?
< luke-jr>
use that tab to talk privately with him
< Provostro>
i like to do it publicly so the whole world knows who's a scammer
< luke-jr>
well it's not welcome here
< Provostro>
do you hagve an issue with that?
< Provostro>
luke-jr, perhaps you too are a scammer in on the scam?
< andytoshi>
the line i got is `set -o errexit; source ./ci/lint/06_script.sh` which is directly in .travis.yml
< MarcoFalke>
andytoshi: You'll need to run the scripts in order, as they "pollute" the environment with variables
< MarcoFalke>
s/in order/in the same bash/
< MarcoFalke>
cp ./ci/test_run_all.sh ./ci/lint_run_all.sh && vim ./ci/lint_run_all.sh
< MarcoFalke>
and then edit the file to run the same stuff that travis.yml runs
< andytoshi>
i don't have apt and stuff
< andytoshi>
i can't run the install scripts
< MarcoFalke>
oh, the linters only run on ubuntu. I don't think they are tested on other operating systems
< andytoshi>
as it happens, they pass on arch with very little modification
< andytoshi>
just that it doesn't really check submodules (this is still a pass :)) and the one python issue which I'll try to PR to add a mypy whitelist to
< andytoshi>
about sys.getwindowsversion not existing
< wumpus>
i don't get it, sys.getwindowsversion doesn't exist on ubuntu either
< MarcoFalke>
that part of the code is only executed when "os.name == 'nt'"
< wumpus>
that makes sense
< andytoshi>
yeah, it's just mypy that's complaining
< andytoshi>
it's not an actual problem
< andytoshi>
there are already mypy whitelists near that line related to windows stuff, i think this might've just been overlooked...but i'm surprissed that travis is ok with it
< jonatack>
also found another bug in the send rpc; unlike the other rpcs, the fee rate cannot be passed as a string, only a number. Opening a fix today.
< jonatack>
achow101: idk if it's a backport-worthy bug, up to others i guess
< provoostenator>
(hi)
< provoostenator>
It's marked experimental
< provoostenator>
So I guess it depends on how annoyhing it is in practice
< jonatack>
achow101: the fix in both cases is simple, the work is mostly writing good test coverage
< achow101>
doesn't seem backport worthy, but no strong opinion on that
< jonatack>
provoostenator: the help says it's an amount (string or number), and the fix is one line, but not up to me
< achow101>
not the first time we've had a known issues section in the release notes
< kanzure>
andytoshi: you can use import importlib; importlib.find_spec("somethingwindowsonly") and another function i forget.
< provoostenator>
Well we probalby get another RC regardless because of a sendaddrv2 compatiblity issue with btcd
< jonatack>
yess i'll just PR it and let ppl decide
< luke-jr>
need another RC for the already merged stuff anyway
< jonatack>
fjahr: i plan to review #19055 when i'm done with all the fee rate things