< sipa> achow101: have you tried creating a wallet with the latest sqlite, and then opening it with a verdion that uses an older one?
< sipa> or is that exactly what you're tesfing here
< achow101> sipa: that's exactly what I did
< achow101> (as well as run all functional tests)
< sipa> cool
< luke-jr> what if new sqlite exits uncleanly?
< achow101> luke-jr: I'd guess that it'd work fine, but I'm not sure how to test that
< achow101> i'd have to kill bitcoind in the middle of a write somehow
< luke-jr> gdb breakpoint and the kill command? <.<
< achow101> hmm, ok..
< achow101> luke-jr: handles it just fine
< luke-jr> +1
< hebasto> ja: #13478 is linked to #20104 as it is the recent discussion about minimum Qt version, and it lists arguments that should be considered in upcoming discussion
< gribble> https://github.com/bitcoin/bitcoin/issues/13478 | [RFC] gui: Minimum required Qt5 · Issue #13478 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/20104 | [RFC] qt: Minimum required Qt5 · Issue #20104 · bitcoin/bitcoin · GitHub
< hebasto> achow101: https://sqlite.org/src/info/fda22108 looks scary, should we bump minimum sqlite up to 3.18.1 ?
< vasild> "test/test_bitcoin: export of symbol in6addr_loopback not allowed"
< vasild> hmm
< bitcoin-git> [bitcoin] fanquake pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/0b2abaa666d6...af22322dab1a
< bitcoin-git> bitcoin/master 79f3d9b Pieter Wuille: Mention BIP155 in doc/bips.md
< bitcoin-git> bitcoin/master 56f9dba Pieter Wuille: Only relay IPv4, IPv6, Tor addresses
< bitcoin-git> bitcoin/master af22322 fanquake: Merge #20119: BIP155 follow-ups
< bitcoin-git> [bitcoin] fanquake merged pull request #20119: BIP155 follow-ups (master...202010_bip155_followup) https://github.com/bitcoin/bitcoin/pull/20119
< vasild> sipa:
< vasild> - CService(CNetAddr(in6addr_loopback), 0 /* port */),
< vasild> + CService(CNetAddr(in6_addr(IN6ADDR_LOOPBACK_INIT)), 0 /* port */),
< vasild> this should fix it
< sipa> vasild: that also works, i guess
< vasild> I don't fully understand this check, why don't we want to export any symbols?
< vasild> I mean - I can't judge if the above is better than adding in6addr_loopback to the list of exceptions
< bitcoin-git> [bitcoin] vasild opened pull request #20129: tests: don't export in6addr_loopback (master...fix_export_of_in6addr_loopback) https://github.com/bitcoin/bitcoin/pull/20129
< vasild> anyway - opened a PR, lets figure it out there
< sipa> vasild: added in #4089
< gribble> https://github.com/bitcoin/bitcoin/issues/4089 | devtools: add script to check symbols from Linux gitian executables by laanwj · Pull Request #4089 · bitcoin/bitcoin · GitHub
< vasild> "This makes sure they are still compatible with the minimum supported Linux distribution versions."
< sipa> yeah, if we'd accidentally introduce a dependency on a symbol that's only available in a recent glibc for example, you can't run the binary on old systems
< vasild> I see
< kallewoof> sipa: maybe you realized, but you did s/fSuccess/fuccess/.
< sipa> kallewoof: i guess there will be an "Updates 2020/10/12" in that case :)
< sipa> i shouldn't be making these changes at 2:24 am
< kallewoof> haha
< sipa> wait
< sipa> of course i was just testing if anyone was paying attention!
< bitcoin-git> [bitcoin] S3RK opened pull request #20130: Wallet: remove db mode string (master...wallet_remove_mode_3) https://github.com/bitcoin/bitcoin/pull/20130
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20131: test: Remove unused nVersion=1 in p2p tests (master...2010-testnVersion) https://github.com/bitcoin/bitcoin/pull/20131
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #20097: fuzz: Version handshake (master...2010-fuzzHandshake) https://github.com/bitcoin/bitcoin/pull/20097
< jonatack> there are now "73 hidden items" in #19988 that can no longer be loaded (the link does nothing) :/
< gribble> https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub
< willcl_ark> jonatack: seems like their gateway 502's when trying to load the comments, at the URL in this paste: https://0bin.net/paste/8-ysvxxl#vimu92Oplo3sSYaKT4n9EHDzDBE-Y2Daf1qUlblgunA
< jonatack> willcl_ark: indeed, it's been the case for a few days now, only the number of hidden comments keeps growing. i'm reviewing without the discussion.
< jonatack> hebasto mentioned it as well last week
< willcl_ark> jonatack: ah I see. How irritating for review.
< wumpus> that's *really* bad
< wumpus> did anyone report this to github yet? we can't keep using the platform if this is the case
< jonatack> wumpus: it looks like fanquake reported it 2-3 days ago: http://www.erisian.com.au/bitcoin-core-dev/log-2020-10-10.html#l-42
< fanquake> I have reported it to GitHub, and followed up with an employee today.
< michaelfolkson> Anyone know if GitLab suffers from this problem (hidden comments)? This specific bug should be resolved but they never get round to addressing the terrible hidden comments UX either
< michaelfolkson> I've never used GitLab
< luke-jr> sipa: imports vs exports?
< luke-jr> michaelfolkson: I have a GitLab repo that exceeded an arbitrary repo size limit years ago; opened an issue on their tracker, no response; pokes on Twitter, was told they'd look at it, still no response.. years later
< michaelfolkson> Fair enough. So that wouldn't improve the situation
< luke-jr> well, no idea if they have this issue, but it seems getting support is at least not likely
< hebasto> could gh cli show all comments?
< luke-jr> lol mishmash License: MIT Apache-2.0 BSD BSD-2 MPL-2.0
< michaelfolkson> Linus quote from Working in Public book. GitHub is "fine for hosting, but the pull requests and the online commit editing are just pure garbage" :) Old quote though
< michaelfolkson> hebasto: I don't think any of the gh cli commands are related to viewing PR comments https://cli.github.com/manual/gh_pr
< hebasto> michaelfolkson: so it's useless for that
< michaelfolkson> I think so. At least from looking at the docs. They seem to pushing a workflow where you approve the PR using the CLI but where discussion and review is done outside of the PR
< jonatack> hebasto: i use gh cli a little and keep updating it, but the features added so far aren't what i'm hoping for. no getting the comments yet.
< jonatack> michaelfolkson: exactly
< jonatack> michaelfolkson: the globocorps and startup missions i was on until late 2018 used gitlab and mattermost instead of github and slack, which i was happy about, and we didn't have any issues at all -- i definitely preferred gitlab
< jonatack> s/used/mostly used/
< jonatack> michaelfolkson: but iirc the idea was to move away from any centralised service, if a migration were to happen
< michaelfolkson> Right. GitHub seems to be deteriorating to me. Often happens post acquisition by megacorp.
< wumpus> michaelfolkson: gitlab seems to handle things fine, e.g. freedesktop uses their own gitlab instance to host some active high-profile projects such as Mesa, no big issues from what i know
< michaelfolkson> Maybe I should try it if I am going to have an informed view rather than a speculative view...
< wumpus> they were also really, really careful to transition from olle mailinglist-based FOSS development, they never trusted github
< wumpus> (possibly because of Linus' opinion as you quoted :-) )
< wumpus> jonatack: being able to see the diff in-line *with reviewer comments* in the terminal would be a great cli feature, wish it could do that
< wumpus> would save me a lot of switching between browser and terminal
< jonatack> yesss this is what i was hoping gh cli would add
< jonatack> whoever the PM is who is driving their features roadmap has very different priorities than ours
< achow101> hebasto: I don't think that's a problem for us. we don't use replace or auto-vacuumed databases
< luke-jr> is there a way we can more strongly mark #18818 as a blocker? let's not forget part was backported to 0.20 …
< gribble> https://github.com/bitcoin/bitcoin/issues/18818 | Fix release tarball generated by gitian by luke-jr · Pull Request #18818 · bitcoin/bitcoin · GitHub
< promag> achow101: feel free to cherry pick, otherwise I'll update it only after yours is merged #20125
< gribble> https://github.com/bitcoin/bitcoin/issues/20125 | rpc, wallet: Expose database format in getwalletinfo by promag · Pull Request #20125 · bitcoin/bitcoin · GitHub
< hebasto> achow101: thanks
< bitcoin-git> [bitcoin] laanwj pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/af22322dab1a...f79a4a895279
< bitcoin-git> bitcoin/master d4dde24 Hennadii Stepanov: net: Add CNode::m_inbound_onion data member
< bitcoin-git> bitcoin/master 49fba9c Hennadii Stepanov: net: Add CNode::ConnectedThroughNetwork member function
< bitcoin-git> bitcoin/master 3984b78 Hennadii Stepanov: test: Add tests for CNode::ConnectedThroughNetwork
< bitcoin-git> [bitcoin] laanwj merged pull request #19998: net: Add CNode::ConnectedThroughNetwork member function (master...200922-istor) https://github.com/bitcoin/bitcoin/pull/19998
< luke-jr> jonatack: meshcollider: if you have a moment to re-ACK #19502 we can get it in :P
< gribble> https://github.com/bitcoin/bitcoin/issues/19502 | Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks by luke-jr · Pull Request #19502 · bitcoin/bitcoin · GitHub
< sipa> luke-jr: imports vs exports?
< MarcoFalke> I guess we could have moved the gui monotree to gitlab for comparison
< MarcoFalke> Downside would be that all gui ppl need to create another account for review/pulls
< sipa> MarcoFalke: if we need to move, i'd suggest moving everything
< sipa> having split environments is even more annoying
< MarcoFalke> Moving everything will be so much pain that not seeing review comments on a few prs seems almost acceptable
< sipa> yes
< sipa> that doesn't mean it's not something we should consider if problems linger\
< luke-jr> MarcoFalke: even on GtiHub, everyone has to create another repo…
< aj> looks like the api still works fwiw. http://azure.erisian.com.au/~aj/tmp/19988_COMMENTS.txt
< aj> generated by -- for a in `seq 1 18`; do curl -s https://api.github.com/repos/bitcoin/bitcoin/pulls/19988/comments?page=$a >19988_comments.$a; echo $a; done and for a in `seq 1 18`; do curl -s https://api.github.com/repos/bitcoin/bitcoin/pulls/19988/comments?page=$a >19988_comments.$a; echo $a; done
< aj> err
< aj> generated by -- for a in `seq 1 18`; do curl -s https://api.github.com/repos/bitcoin/bitcoin/pulls/19988/comments?page=$a >19988_comments.$a; echo $a; done and cat 19988_comments.? 19988_comments.?? | jq -r '.[] | (.user.login + " " + .path + ":" + ((.line//.original_line | tostring) // empty) + " " + (.created_at), .body, "--------------------------")' > 19988_COMMENTS.txt
< sipa> aj: the "octodroid" android app also still works on it, though slowly
< aj> sipa: does that mean giving your github pw to a thid party app dev?
< sipa> aj: it's open source, and *obviously* i reviewed the code and compiled it myself before using *cough*
< bitcoin-git> [bitcoin] practicalswift opened pull request #20137: tests: Update UBSan suppressions file with suppressions needed for clang 12 (current trunk) (master...clang-12-ubsan-suppressions) https://github.com/bitcoin/bitcoin/pull/20137
< luke-jr> sipa: ah, but did you compile your compiler yourself? ;)
< luke-jr> I guess step 1 to migrating to a decentralised system would be to write a nice GUI app for GitHub's API?
< luke-jr> (not Android because who wants to dev on their phone? XD)
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20138: net: Assume that SetCommonVersion is called at most once per peer (master...2010-netVersionOnlyOnce) https://github.com/bitcoin/bitcoin/pull/20138
< MarcoFalke> Ugh, is there any reason why GitHub would delete whole issues? #17298 is gone
< gribble> https://github.com/bitcoin/bitcoin/issues/17298 | HTTP Error 404: Not Found
< aj> MarcoFalke: yikes
< gwillen> what issue was it?
< fjahr> What was it? Definitely admins can delete issues but I wouldn't know why github would do it on it's own.
< gwillen> are you sure it existed? (and is whatever you thought it was?)
< MarcoFalke> sometimes spam is deleted, I think
< gwillen> I can't find any reference to it
< sipa> i can't find any reference to it either
< fjahr> Yeah, maybe that's what happened, spam issue was deleted by an admin
< bitcoin-git> [bitcoin] stackman27 opened pull request #20139: Removed unused warning and formatted RPC result (master...upgradewallet_rpc_cleanup) https://github.com/bitcoin/bitcoin/pull/20139
< aj> "MinGW Runtime Assertion - Assertion failed! (#17298)"
< gribble> https://github.com/bitcoin/bitcoin/issues/17298 | HTTP Error 404: Not Found
< aj> closed by the submitter 31st oct 2019, doesn't seem at all spammy
< gwillen> one thing I notice is that its title is identical to the title of an older issue
< gwillen> which makes me wonder if github did some kind of erroneous database cleanup or something, although that would be crazy
< sipa> i can't imagine that i would have deleted this issue; i can't speak for other maintainers
< gwillen> (the older issue is #11812)
< gribble> https://github.com/bitcoin/bitcoin/issues/11812 | MinGW Runtime Assertion - Assertion failed! · Issue #11812 · bitcoin/bitcoin · GitHub
< gwillen> (I am taking aj's word for the contents of the newer issue since I can't see it -- I guess you had the page saved or something?)
< sipa> i have the old issue in my email
< sipa> yes, it's what aj says
< aj> gwillen: no it was when i was getting emails for everything
< sipa> SMTP notification is best notification
< MarcoFalke> The GitHub database optimization sounds plausible (and horrible)
< sipa> fanquake: do you have any insight?
< MarcoFalke> I've seen this happen at least three times. I can look up all the issues that were deleted, if needed.
< aj> ^-- works
< sipa> heh, so if it was deleted it's certainly not well deleted
< gwillen> the /events endpoints also still works, and the /labels endpoint gives an ... interesting failure (https://api.github.com/repos/bitcoin/bitcoin/issues/17298/labels)
< gwillen> it seems like the issue is labelled with a dangling label... I wonder if that's related to it being missing.
< sipa> can we try creating an issue and actually maintainer-deleting it, to see if the api result is identical?
< sipa> if not, it's more evidence of a screwup on github's side we should report
< aj> is what i get when i delete an issue; not a 404
< aj> and comments go away
< bitcoin-git> [bitcoin] sipa opened pull request #20140: Restore compatibility with old CSubNet serialization (master...202010_subnet_ser_compact) https://github.com/bitcoin/bitcoin/pull/20140
< sipa> aj: definitely looks like a github issue...
< sipa> MarcoFalke: we should report this
< achow101> can confirm, doesn't look like spam
< gwillen> hmmm, none of the submitter's comments seem to appear in /comments
< luke-jr> aj: test the dangling label theory?
< gwillen> I think I was mistaken about the dangling label, I think the ID it's complaining about is the issue itself
< gwillen> (github uses numeric IDs for the REST API, but string IDs for the graphQL API)
< gwillen> who was the submitter of the bug? I see a reply to a user named "StevenLee-CG" -- was that the submitter? That account doesn't seem to exist.
< gwillen> Which makes me wonder if what happened was they deleted the account, with prejudice, and all associated objects, or something.
< sipa> that seems plausible
< gwillen> seems kind of rude.
< sipa> aj: so your suggestion is removing the final commit from #19988 ?
< gribble> https://github.com/bitcoin/bitcoin/issues/19988 | Overhaul transaction request logic by sipa · Pull Request #19988 · bitcoin/bitcoin · GitHub
< aj> luke-jr: https://api.github.com/repos/ajtowns/test-repo/issues/10/labels previously returned a "bug" label, after issue deletion just "Not found"
< luke-jr> aj: I mean make an issue, give it a label, then delete the label
< luke-jr> but gwillen thinks it's not an issue, so..
< aj> sipa: moving that commit to a separate PR maybe? are there any benefits to that patch other than simplifying/deleting code?
< achow101> gwillen: when a user deletes their account, the issue should go to the "ghost" account
< gwillen> I don't think this was a voluntary deletion
< gwillen> I think this was some kind of aggressive admin deletion, like "account deleted for being a spammer" or for copyright violation or something
< gwillen> otherwise it would be weird for it to leave the database in an inconsistent state like this (although ... that's weird anyway, and perhaps their code is just bad)
< achow101> gwillen: perhaps. the user doesn't seem like a spammer though
< gwillen> do we have an example of a comment or issue filed by a subsequently-deleted user?
< sipa> aj: timing going backwards significantly seems like a problem in both variants really
< sipa> aj: and the only real solution is using a steady clock
< sipa> i'd say the code is a bit simpler now with the "now-monotonization" in it, and people have already looked at it
< sipa> so i'd rather keep it
< aj> sipa: in the original code it just means some announcements sit around until time catches up (in DELAYED or in REQUESTED if a notfound/tx doesn't come in) which doesn't seem a big deal?
< sipa> or all timeout instantly
< sipa> when it jumps forward
< aj> sipa: right, that's jumping forward. but it only means the single thing that's "next" will get queued up, which seems fine?
< sipa> i guess my comment is more about time jumping, not so much the backwards aspect of it
< sipa> hmm, true
< sipa> jnewbery: here?
< aj> sipa: (hidden motivation is that i don't want to re-review all the original code to update my ack while knowing that it's about to be removed and the logic switched around in the last commit)
< sipa> ok that's fair
< jnewbery> sipa: hi
< sipa> jnewbery: i'm inclined to just remove the monotonic time last commit based on aj's comments above
< sipa> wdyt?
< jnewbery> fine by me. I have no strong opinion. I ACKed it before, I ACKed it after
< jnewbery> I would like to freeze the PR soon and get it merged (as I'm sure you would). Seems ready, and any loose ends can be tidied up after feature freeze
< sipa> jnewbery: regarding the invariants (but this is for a future PR), if TxRequestTracker maintains its own time (which it can do even if backward/forward at both allowed), then it's indeed possible to just enforce the invariants all the time with no real API complication
< sipa> so i like that idea
< jnewbery> sipa: yeah, the only thing I think we might need to be careful about is whether that increases to cost of ReceivedInv() and RequestedTx() in the worst case, and whether that can be exploited
< jnewbery> but I think it's ok
< fanquake> sipa: unsure. I can bring it up with GitHub if someone hasn’t already