though otoh it did get a lot of review, and it's an optional thing disabled by default
Yes. I agree. Just it looks like there is no clear conceptual ACK on serving filters after BIP157 (don't get me wrong, I'm all for serving BIP157).
I just think 6 days was to short for a reasonable discussion
but it's sure very fast given that we're kind of in slow times
One thing that's not getting faster is how long it takes to run all 160 tests of the functional test suite
luke-jr: look, I disagree, personally I'd prefer not to include to autogenerated cruft at all, and there's more people that don't, so I don't see it as a regression. Spliting up the PR makes sure there is at least progress on parts that we do agree on. IF that isn't a positive thing I honestly don't know.
jonasschnelli: the conceptual discussion is ongoing (at least for me) on bip157, but the changes in that PR didn't seem dangerous on their own after reviewing them, and reversible before the next release
jonatack: I agree. Though an progressive merge behaviour is usually not the mode the project has worked in the past
I think that's an okay one, it could be written as "This could be handlded much more gracefully" without losing it's meaning, it's not like the one in the tests a fragment "todo: def base58 decoding" *why*
jonasschnelli: 18877 is the first 3 commits from 16442, which has been open for almost a year. It had 7 ACKs + 2 implicit ACKs from jimpo and me + 6 concept ACKs. How can one year and 15 [concept] ACKs possibly be considered too short or not enough discussion?
having a decode function could have saved me going that that rabbit hole
given that base58_decoded is needed, for me it seems kind of nonsensical to reinvent the wheel and manually code it out again; that's as absurd as if someone would implemented sha256 by hand in the test suite
of course base58 is not part of the python standard library, but i'd either use a tested widespread library, or (if we
don't want dependencies) copy it from that library
jonasschnelli: ah, you're right. I should have linked back to the full PR in 18877. The last comment before merge lists out all the places that people have [concept] ACKed.
MarcoFalke: 18964 is not for 0.20 is it?
I don't think it's unreasonable to add a decode function if the encode function is there. At least that means we can test the encoder with round-trips.
(but agree that we should remove TODOs if they're of only marginal benefit to the project. The issue system seems like a more appropriate place for those)
[23:19] <luke-jr> would be nice if Travis sent an email to the PR creator when it failed :x
You can enable travis in your own repo to get emails
Btw, I have a lot of good first issues to work on. If someone is out there that wants them, please contact me. If I don't hear back, I will put them on the main repo maybe next week.
how do conflicting transactions fit into the most recent sorting of listttransactions? do they get sorted as the most recent like the negative confirmations number might imply? or at their original height? also, are there any guarantees for the ordering of unconfirmed transactions among themselves?