19:00:09 <laanwj> #startmeeting 
19:00:10 <core-meetingbot`> Meeting started Thu Mar 17 19:00:09 2022 UTC.  The chair is laanwj. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:10 <core-meetingbot`> Available commands: action commands idea info link nick
19:00:13 <achow101> hi
19:00:17 <stickies-v> hi
19:00:43 <hebasto> hi
19:00:55 <laanwj> #bitcoin -core-dev Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball
19:00:56 <laanwj> morcos nehan NicolasDorier paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:01:03 <provoostenator> hi
19:01:23 <cfields_> hi
19:01:24 <laanwj> welcome to the weekly bitcoin-core-dev general meeting
19:01:28 <dongcarl> hi
19:02:09 <laanwj> there has been one proposed topic: important changes in 23.0 to cover in the new RC Testing Guide (stickies-v)
19:02:14 <MarcoFalke> #proposedmeetingtopic Adjusted time offset warning
19:02:15 <laanwj> any last minute additional ones?
19:02:32 <laanwj> MarcoFalke: ok
19:03:02 <michaelfolkson> hi
19:03:30 <laanwj> #topic High priority for review
19:03:30 <core-meetingbot`> topic: High priority for review
19:03:37 <laanwj> let's start with high prio as usual
19:04:03 <laanwj> https://github.com/bitcoin/bitcoin/projects/8  at the moment there's 8 blockers, 1 chasing concept ACK
19:04:21 <laanwj> anything to add/remove, or that's almost ready for merge?
19:04:39 <laanwj> #24058 was added outside the meeting last week
19:04:42 <gribble> https://github.com/bitcoin/bitcoin/issues/24058 | BIP-322 basic support by kallewoof · Pull Request #24058 · bitcoin/bitcoin · GitHub
19:04:55 <MarcoFalke> #23595 for me pls
19:04:55 <gribble> https://github.com/bitcoin/bitcoin/issues/23595 | util: Add ParseHex () helper by MarcoFalke · Pull Request #23595 · bitcoin/bitcoin · GitHub
19:06:05 <laanwj> MarcoFalke: added
19:06:42 <MarcoFalke> dank
19:06:45 <jonatack> hi
19:06:52 <luke-jr> maybe add #22693 for me
19:06:53 <gribble> https://github.com/bitcoin/bitcoin/issues/22693 | RPC/Wallet: Add "use_txids" to output of getaddressinfo by luke-jr · Pull Request #22693 · bitcoin/bitcoin · GitHub
19:07:23 <laanwj> luke-jr: added
19:07:36 <luke-jr> ty
19:07:42 <jonatack> may i please add #24555 (targets backport to v23)
19:07:44 <gribble> https://github.com/bitcoin/bitcoin/issues/24555 | doc: create initial doc/cjdns.md for CJDNS how-to documentation by jonatack · Pull Request #24555 · bitcoin/bitcoin · GitHub
19:08:12 <jeremyrubin> hi
19:08:28 <laanwj> jonatack: done
19:08:37 <jonatack> laanwj: thank you
19:10:01 <laanwj> i guess that was it for high prio, let's move to next topic
19:10:13 <laanwj> #topic Important changes in 23.0 to cover in the new RC Testing Guide (stickies-v)
19:10:14 <core-meetingbot`> topic: Important changes in 23.0 to cover in the new RC Testing Guide (stickies-v)
19:10:29 <stickies-v> Context: I've started working on updating the 23.0 RC Testing Guide, and I'd like to get some early feedback on which changes are considered important and useful to test since we don't have too much until release date.
19:10:42 <stickies-v> From the release notes (https://github.com/bitcoin-core/bitcoin-devwiki/wiki/23.0-Release-Notes-draft), I've selected the 5 changes I think most worthwhile to test, and 1 extra if I have time to get to it. I'd very much appreciate feedback on if you think anything else should really be included, and if maybe something can be omitted.
19:10:56 <stickies-v> The changes I've selected are (in random order, summarized from release notes):
19:10:57 <luke-jr> stickies-v: release dates are not firm; it's ready when it's ready
19:11:24 <stickies-v> 1. The strong preference for only connecting to peers that listen the standard port 8333 has been removed.  (#23542)
19:11:29 <stickies-v> 2. Descriptor wallets are now the default wallet type. Newly created wallets will use descriptors unless descriptors=false is set during createwallet.
19:11:30 <gribble> https://github.com/bitcoin/bitcoin/issues/23542 | net: open p2p connections to nodes that listen on non-default ports by vasild · Pull Request #23542 · bitcoin/bitcoin · GitHub
19:11:35 <stickies-v> 3. The validateaddress RPC now returns an error_locations array for invalid addresses, with the indices of invalid character locations in the address (if known). (#16807)
19:11:38 <gribble> https://github.com/bitcoin/bitcoin/issues/16807 | Let validateaddress locate error in Bech32 address by meshcollider · Pull Request #16807 · bitcoin/bitcoin · GitHub
19:11:39 <stickies-v> 4. The getblock RPC command and /rest/block/ REST endpoint now support verbosity level 3 containing transaction inputs' prevout information.
19:11:43 <stickies-v> 5. Information on soft fork status has been moved from getblockchaininfo to the new getdeploymentinfo RPC which allows querying soft fork status at any block, rather than just at the chain tip. (#23508)
19:11:49 <gribble> https://github.com/bitcoin/bitcoin/issues/23508 | Add getdeploymentinfo RPC by ajtowns · Pull Request #23508 · bitcoin/bitcoin · GitHub
19:12:02 <laanwj> i don't think 1 is straightforward to test
19:12:02 <stickies-v> The additional change if I get to it in time is:
19:12:02 <stickies-v> 6. A bitcoind node will only rumour addresses to inbound peers after they've sent an ADDR, ADDRV2, or GETADDR message. (#21528)
19:12:09 <gribble> https://github.com/bitcoin/bitcoin/issues/21528 | [p2p] Reduce addr blackholes by amitiuttarwar · Pull Request #21528 · bitcoin/bitcoin · GitHub
19:12:31 <laanwj> because nothing will really give you non-8333 ports frequently yet (e.g. the DNS seeds strongly prefer 8333)
19:12:34 <luke-jr> laanwj: well, testers can at least verify they have 8 peers that look randomish?
19:12:58 <michaelfolkson> And 2 isn't new for this release. Not sure if that should be a focus?
19:13:01 <laanwj> sure, you can test that you still get usable peers
19:13:15 <hebasto> owners of Apple M1 could test native arm64 binaries
19:13:23 <laanwj> hebasto: yes, that's a good one
19:13:32 <luke-jr> and they don't get deleted :P
19:13:42 <hebasto> indeed
19:13:49 <stickies-v> hebasto: thanks I'll include that
19:14:05 <laanwj> stickies-v: thanks for working on this btw
19:14:06 <jonatack> stickies: would be nice to test bitcoind over CJDNS, including IBD with -onlynet=cjdns and dns seeds
19:15:30 <stickies-v> jonatack: cool, will add that too
19:16:05 <jonatack> stickies-v:  great
19:16:13 <stickies-v> I was thinking the non-default port could be tested on regtest, since it does seem like quite an important change for the network going forward?
19:17:51 <stickies-v> achow101: what do you think about michaelfolkson's comment regarding descriptor wallet becoming default not being important to include in the testing as it's not a new change?
19:18:02 <jonatack> stickies-v: you might be able to easily recycle the I2P testing in the last release testing, and link to doc/cjdns.md in #24555
19:18:04 <gribble> https://github.com/bitcoin/bitcoin/issues/24555 | doc: create initial doc/cjdns.md for CJDNS how-to documentation by jonatack · Pull Request #24555 · bitcoin/bitcoin · GitHub
19:18:15 <laanwj> i don't think port preference ever plays a role on regtest, as all connections are manual
19:18:28 <luke-jr> were there any major changes to descriptor wallets between 21/22.x and 23.x?
19:18:33 <luke-jr> besides adding Taproot
19:18:56 <jonatack> (e.g. convert the I2P testing section for v22 to CJDNS)
19:19:08 <stickies-v> hmm okay didn't know that, will reconsider 1. then and leave it out if I don't find any good way to test efficiently
19:19:14 <achow101> stickies-v: I don't feel strongly either way. frankly, it's such a small change that there isn't much that could go wrong
19:19:17 <laanwj> but if you consider it like that, all functional tests test non-default ports, as they pseudo-randomly assign ports
19:20:15 <MarcoFalke> I think the most important testing is of the parts that everyone assumes *not* to be changed.
19:20:52 <luke-jr> true
19:21:04 <MarcoFalke> Surely, it is important that new features are tested, but even more importantly everything else shouldn't break
19:21:04 <jonatack> 3, 4, and 5 may be well-tested already (if I'm not mistaken), not sure if there is much value in people checking those
19:21:08 <laanwj> also testing it with third-party programs is always interesting (i guess e.g. joinmarket, c-lightning are covered)
19:21:09 <lightlike> I think it should work to addpeeradress a couple of fantasy addresses with default/non default ports and take statistics what it tries to connect to.
19:22:04 <luke-jr> new features are probably more tested already too
19:22:10 <MarcoFalke> Jup, any business or third party software should have a test suite to check that their use of Bitcoin Core doesn't break with an upgrade
19:22:28 <stickies-v> MarcoFalke: that's a good point but I'm not sure if that's something we can make actionable in this testing guide?
19:22:52 <stickies-v> I think it's more of an intro get people familiar with testing and with what's changing
19:22:56 <laanwj> right, the testing guide is more about giving peopel specific focus what they could test, if it's just "what you are doing already" it's short
19:24:17 <stickies-v> alright thank you for the ideas everyone, I'll look into all of these suggestions and revert next week with a draft of the guide
19:24:29 <michaelfolkson> Ok makes sense. Leave it up to your judgment then stickies-v :)
19:24:30 <jonatack> right, the reason i mention the cjdns testing is also that there are more moving parts, platforms, cases and possible issues that may not have been seen yet
19:24:31 <laanwj> thanks!
19:24:40 <laanwj> let's move to next topic
19:24:45 <laanwj> #topic Adjusted time offset warning (MarcoFalke)
19:24:45 <core-meetingbot`> topic: Adjusted time offset warning (MarcoFalke)
19:25:07 <MarcoFalke> So while the adjusted time was improved over the last couple of years, it still has some shortcomings
19:25:18 <laanwj> i've always felt really uncomfortable with the time adjustment code
19:25:31 <laanwj> can't we just like phase it out
19:25:45 <MarcoFalke> Just to mention a few: (1) non-monotonic (2) adjusted by peers ...
19:26:04 <MarcoFalke> So I was thinking to simply remove it and replace it with a stronger warning mechanism
19:26:27 <laanwj> adjusted by peers based on unauthenticated cleartext data, based on strange criteria which we're too scared to touch
19:26:36 <laanwj> ack
19:26:47 <luke-jr> sounds like a good idea IMO
19:27:01 <MarcoFalke> The question is how to warn?
19:27:12 <laanwj> isn't there a warnings field on some RPC call
19:27:13 <lightlike> what would that "stronger warning mechanism" look like?
19:27:20 <luke-jr> laanwj: GUI can probably use improvement
19:27:23 <MarcoFalke> We can't warn at startup, as only the local system time is available
19:27:38 <luke-jr> maybe the sync overlay could be a good place for it
19:28:11 <MarcoFalke> I think the GUI is already good. There will be a orange warning, very hard to miss
19:28:50 <laanwj> yes that seems fine to use
19:28:55 <MarcoFalke> (For reference there is a long thread at #4521 with cross-links)
19:28:56 <gribble> https://github.com/bitcoin/bitcoin/issues/4521 | AddTimeData will never update nTimeOffset past 199 samples · Issue #4521 · bitcoin/bitcoin · GitHub
19:28:57 <laanwj> it doesn't have to be at startup
19:29:23 <laanwj> warnings can be added at any time (it is, or was, used for some other network conditions too)
19:29:26 <MarcoFalke> So one option I was thinking about was to just shut down the node. Does that seem too agressive?
19:29:33 <laanwj> way too aggressive
19:29:47 <laanwj> just log a message and make it available on a RPC call
19:29:56 <laanwj> like we've always done for warnings
19:30:19 <MarcoFalke> It seems unlikely the user will ping the RPC regularly to get the warning
19:30:22 <laanwj> making it possible for peers to shut down your node by flooding you with massaged time values is even worse than we have now
19:30:30 <laanwj> for fact, they do
19:30:48 <luke-jr> maybe safe mode wasn't a terrible idea
19:30:49 <laanwj> or at least used to when warnings were more common i don't know nowadays
19:31:02 <MarcoFalke> ok, good point
19:31:25 <luke-jr> laanwj: otoh, if all your peers are, maybe you want new peers..
19:31:27 <MarcoFalke> luke-jr: I don't think there is a need to shut down the wallet? I think this mostly affects the mining code
19:31:45 <luke-jr> MarcoFalke: I would expect miners to be monitoring the node state closely
19:32:12 <laanwj> i think it would be valid to strip out the time adjustment system completely, just leave it up to the user's responsibility to have their time correct, like other software does
19:32:14 <MarcoFalke> So my other idea was to throw an exception in getblocktemplate. Does that seem too agressive?
19:32:22 <laanwj> having a warning is nice but i don't see it as cirical
19:32:47 <laanwj> not sure why this would need a more aggressive mechanism than other warnings
19:32:57 <luke-jr> MarcoFalke: also, miners are likely to internally only have peers with their own nodes
19:33:14 <luke-jr> so throwing in GBT won't be helpful IMO
19:33:21 <jeremyrubin> could also have something like walletnotify where we call a script to get the time
19:34:05 <jeremyrubin> that would make it possible for pluggin in an arbiitrary time source if users don't want their system time
19:34:09 <laanwj> come on, it's 2022, computers tend to have correct system time these days
19:34:26 <luke-jr> jeremyrubin: idk why we would support not using system time
19:34:45 <laanwj> if you want to mess aroudn with the time for whatever reason there's faketime and even time namespaces in newer linux kernels
19:34:48 <MarcoFalke> ok, I mean I am fine just removing it. I just wanted to see if there is something we could do better
19:34:52 <luke-jr> laanwj: exactly
19:35:34 <luke-jr> we have mockable time too
19:35:41 <jeremyrubin> i just mean that if you want behavior for "my peers think the time is different" you could let that be configured as a script callback
19:35:42 <MarcoFalke> luke-jr: Not for main-net
19:35:50 <laanwj> MarcoFalke: so i would say the proposal to change it to a warning is fine, for now, it can always be fully removed later if we want, but i don't think it needs a more aggressive mechanism
19:36:06 <luke-jr> MarcoFalke: could be trivially enabled if desired
19:36:09 <MarcoFalke> laanwj: I think a warning already exists
19:36:19 <MarcoFalke> I can add a new dedicated RPC for it, too
19:36:26 <laanwj> not sure a dedicated RPC would be better
19:36:37 <jeremyrubin> (you can also crontab and parse the log for the warning, which is fine)
19:36:38 <laanwj> things like warnings are better grouped, so people don't have to listen for everything separately
19:36:47 <MarcoFalke> I guess a dedicated RPC would make jeremyrubin also happy
19:37:14 <laanwj> don't we have the time delta in RPC already somewhere
19:37:27 <MarcoFalke> getpeerinfo, maybe?
19:37:46 <MarcoFalke> At least the GUI shows it in the peers tab
19:37:48 <jeremyrubin> ponders the nature of happiness
19:38:19 <jeremyrubin> i will say that a good portion of nodes might be running on centralized time servers
19:38:52 <laanwj> i think it's ok to make the information available, dont' think a new RPC is worth it
19:38:54 <jeremyrubin> so this might be giving someone somewhere the "brick all the nodes" key
19:39:01 <jonatack> getpeerinfo has a timeoffset field in seconds
19:39:03 <jeremyrubin> but i don't think it's the biggest deal
19:39:11 <laanwj> it's not like the time adjustment scheme really protected against that
19:39:39 <luke-jr> jeremyrubin: … someone who can centrally control your server's time, can probably just shut off the power too
19:39:40 <laanwj> if you can sabotage a centralized timeserver sure you could mess up things for a bit until people figure it out
19:40:09 <jeremyrubin> luke-jr: i would expect apple can change my laptops time, but i don't think they can shut my power off
19:40:23 <laanwj> they could probably delete all software from your system at least
19:40:35 <luke-jr> that seems naive, but I understand - thought you meant VPS with a shared kernel
19:40:44 <MarcoFalke> adjusted time really only protects against a 30 or 60 minute offset (DST/time zone) for at most half a year
19:41:40 <laanwj> in any case, if you're serious about mitigating attacks on timeservers i'd guess that has a much bigger scope than bitcoin
19:41:58 <laanwj> like, run your own atomic clock and adjust your system time to it
19:42:39 <MarcoFalke> adjust your system time to Bitcoin (timechain) MTP
19:43:06 <laanwj> btw, ntp also allows for only very small on the fly changes
19:43:09 <MarcoFalke> (obviously don't do that)
19:43:20 <laanwj> it's not like your ntp server can suddenly warp you back hours or days
19:45:06 <luke-jr> MarcoFalke: might not be terrible tbh
19:45:12 <luke-jr> if you have a working monotonic clock
19:45:23 <laanwj> at boot up it will probably accept any time, especially on embedded systems without a RTC, but that's also easiest to detect
19:45:37 <MarcoFalke> luke-jr: You might miss blocks if MTP is lagging too much
19:46:53 <laanwj> it's also hardly a 'brick all the nodes' key if you have to wait for all servers running nodes to reboot first :)
19:47:39 <Earnest> monotonic vs bootime might be worth considering as monotonic clocks tend to stop when systems sleep afaiui
19:48:06 <luke-jr> MarcoFalke: well, you'd have to do MTP + 30 minutes at least. a bit of buffer would help handle stuff like this.
19:48:21 <Earnest> Ah, nevermind https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d6ed449afdb38f89a7b38ec50e367559e1b8f71f
19:48:56 <luke-jr> laanwj: modern systems constantly sync their clocks over NTP, not just at boot
19:48:58 <laanwj> Earnest: monotonic clocks are only used for measuring time intervals, so that's fine, usually
19:49:05 <luke-jr> though maybe only within some limited drift
19:49:09 <laanwj> luke-jr: yes, but only minimal adjustments
19:49:50 <laanwj> at least that's how linux ntpd works, i have no idea about other OSes
19:50:20 <luke-jr> lots of people use systemd OS now :p
19:50:55 <luke-jr> (it ate ntpd)
19:51:23 <jonatack> getnetworkinfo has a timeoffset field that is our GetTimeOffset() result, IIUC,  as opposed to the offset for each peer in getpeerinfo
19:51:25 <Earnest> As systemd-timesyncd which is sntpd implementation
19:51:42 <laanwj> i mean, with all the complications and exceptions it's not realistic as an attack on bitcoin, that's all i was trying to say
19:52:14 <laanwj> jonatack: nice, so that too already exists
19:53:27 <luke-jr> maybe getblocktemplate should throw if there's a known better-except-for-time-just-barely block?
19:53:41 <luke-jr> seems more likely to cause than solve problems tho
19:54:14 <MarcoFalke> luke-jr: How often did that happen in the last 10 years?
19:54:26 <luke-jr> MarcoFalke: no clue
19:54:43 <jonatack> (and only outbound peers offsets contribute to our timedata samples to make it harder for peers to tamper with our adjusted time... src/net_processing.cpp::2761)
19:54:56 <luke-jr> kinda doubt miners would risk getting so close to the time cutoff
19:55:00 <Guest28> stickies-v: 1 can be tested for non default ports, you just need to look for nodes that are using such ports. There are different ways to do it.
19:56:32 <laanwj> right i would be surprised if miners weren't already really careful about having correct time on their machines, to rule that out as a reason to miss a block reward
19:57:48 <laanwj> any well-meaning protective mechanism added on top might just get in the way in case of some edge case
19:58:26 <MarcoFalke> Even if one miner is back by 1h and another one forward by 1h, it shouldn't lead to any issues
19:58:37 <MarcoFalke> Only if the difference is more than 2h
19:58:42 <laanwj> sure
19:59:32 <MarcoFalke> Well, thanks everyone for the input!
20:00:25 <laanwj> it's time to close the meeting
20:00:28 <laanwj> #endmeeting