Development meeting held @ 3PM UTC in grincoin#dev channel on Keybase. Meeting lasted ~ 120 min.
Notes are truncated, and conversations sorted based on topic and not always chronological. Quotes are edited for brevity and clarity, and not always exact.
Community attendance:
- antiochp
- deeev
- jaspervdm
- joltz
- lehnberg
- paouky
- phyro
- quentinlesceller
- tromp
- vegycslol
(apologies if I missed someone - submit a PR or contact @lehnberg to add)
- antiochp: Jasper and I continue to make progress on pibd prep work. I believe there is also a PR coming shortly related to "late locking" on wallet side of things. @tromp has prs up for fees RFC and daa rfc.
- 🚀: paouky, phyro
The proposed agenda was reviewed and accepted with a point added to discuss the recent re-org attack.
-
lehnberg: I've been a blocker there, just been swamped with other stuff. Asked @joltz to give me a few days more to action the ones I'm meant to contact and then update our doc.
- paouky: What doc is that?
- joltz: Just a doc we have been using between ourselves to track who we have contacted etc. Based on the services doc you compiled.
- 👍: paouky
- lehnberg: Yes, can share a list and status of the companies we've contacted once we've concluded, just don't want to blast people's names and personal contact details out in public.
-
joltz: Have been continuing follow ups with points of contact at various exchanges. Receiving mostly canned responses but continuing to follow up until we hopefully get some actions.
- 👍: antiochp
- antiochp: Funding request has been withdrawn due to time constraints.
- antiochp: I think we are mining on testnet right?
- quentinlesceller: Unsure if someone is mining right now. At least not on testnet grinmint.
- jaspervdm: I havent seen a block in like 5d.
- quentinlesceller: Okay so the cheapest solutions was to use a vps (the provider you mentionned @antiochp can't recall here).
- jaspervdm: I recall @tromp wanting to ask lolliedieb if he had spare gpu power.
- tromp: I was mining on a friend's machine for almost a week, but stopped after noise complaints:( Lolliedieb doesn't have a gpu to spare.
- quentinlesceller: Hostkey that's the cheapest right now.
- jaspervdm: Ok so lets pull the trigger on the hostkey one?
- tromp: Alternatively, we take johndavies up on his offer and let the chain progress at a slower pace.
- quentinlesceller: Tromp which one is the smallest machine we can get. https://www.hostkey.com/gpu-servers#/.
- tromp: I'd go for a 2080ti but don't see it listed. They mention them on top though?!
- paouky: We can probably find someone with a 2080ti and pay him a rate that is above normal market rewards for mining with this gpu?
- quentinlesceller: I do think we need to invest a bit in order to have a stable testnet running. No 2080ti what about 2x1080ti or 1 x 3080 or 1 x 3090?
- tromp: Are we ok with testnet progressing with avg blocktime > 60s ? e.g. 80s?
- jaspervdm: Yes I think so. I don't think there is an advantage in getting 2x 1080ti compared to a single 1080ti. What matters is we have a stable flow of blocks.
- tromp: In that case a single 1080Ti would be ok.
- antiochp: Yeah as long as they are coming in regularly
- quentinlesceller: Yes that'd be 96€ per month right now, seems fair.
- antiochp: Lets just do that at 96€ for now? We can always revisit this.
- 👍: phyro, lehnberg, joltz, jaspervdm
- quentinlesceller: Yep agree too.
- lehnberg: Defo worth it at least until HF4.
- quentinlesceller: I'm CAD/USD based so I'd appreciate if someone else can manage the invoicing. I can manage the setup if needed.
- antiochp: I feel like mainnet dev is likely to slow down somewhat after HF4 and testnet will become more important.
- lehnberg: You're probably right, I'm just saying "it's a no brainer to commit until January, let's evaluate after that if we want to extend"
- 👍: jaspervdm, antiochp
- lehnberg: You're probably right, I'm just saying "it's a no brainer to commit until January, let's evaluate after that if we want to extend"
- quentinlesceller: We can have 12% off if we 1 year or 6% off for 6 months?
- antiochp: Maybe just monthly to start?
- 👍: tromp, vegycslol, lehnberg, quentinlesceller
- antiochp: Maybe just monthly to start?
- antiochp: Is anybody able to do EUR based invoices?
- jaspervdm: Yeah I could rent the server if you want, if someone else can take care of the actual admin i.e. setting up the miner.
- quentinlesceller: Yep can do.
- antiochp: Awesome - @quentinlesceller and @jaspervdm you can take this from here?
- jaspervdm: yes
- quentinlesceller: Yep
- jaspervdm: Update on the hostkey server, only after you confirm the order they state the the 96EUR is without VAT. so total cost will actually be 116.16EUR.
- 👍: lehnberg, antiochp, joltz, phyro
-
joltz: Thanks to @quentinlesceller for catching it early on his saturday night, otherwise response time would have been slower.
- 👍: lehnberg, antiochp, deeev
-
antiochp: Onto the reorg that we experienced over the weekend, writeup here. Thanks to @deeev, @paouky, and @mcmmike for putting this together.
-
joltz: I'm blocking off a little time this week- my hope is to come up with a simple algo based on nicehash data, pool data and node data to provide a 1-5 health score for the network. This can then be called from grin.mw to provide a color code score for network health with associated recommendations based on the current level.
- 🚀: antiochp, phyro, quentinlesceller, lehnberg, paouky, bladedoyle
- lehnberg: Would be awesome.
- jaspervdm: That would be great. Of course NH isn't the only possible source of attacks as there are large GPU farms out there (that can be rented at a premium) but it's the only publicly available information.
- mcmmike: Please also consider looking at https://github.com/bladedoyle/grin_nicehash_defender. I am testing it at the moment and if we would decentralize it with a central amount of funds to scare attackers, it would be great.
- 👍: joltz, deeev
-
deeev: Should we increase overall the transaction cache timer limit in case of new large reorg on all node? I've personally updated my own one to 1440.
- tromp: Ideally, exchanges would have dynamic confirmation times depending on amounts deposited. But if they insist on static numbers, then I'd strongly recommend using at least 1440 blocks (1 day).
- quentinlesceller: Seems unlikely to happen though.
- antiochp: @deeev you are talking about the "reorg cache" that we use to re-add txs to the local pool?
- deeev: Yes, in the reorg some txs were not broadcast again because of the 30 min limit I guess.
- antiochp: Yeah we'd need to look and see if that would actually re-broadcast or not (I suspect it only affects the local pool). I don't recall.
- jaspervdm: So it would be good for pools to increase that value on their stratum nodes.
- antiochp: We also need to be careful as that is in memory and 1440 blocks worth of txs could (potentially) be a large # bytes. Can somebody open an issue on github for this (if not already on there)? So we can track discussion of this.
- deeev: Will do
- 👍: antiochp
-
antiochp: The grinnode alerting worked but highlighted some "gaps" in our internal logging (which the alerting is driven off).
- tromp: Did I understand correctly that many nodes went into sync mode to do the reorg?
- antiochp: Yes - the "fork" was withheld so nodes only saw it once it was 5 blocks beyond current chain head.
- antiochp: Which triggers sync mode to catch up.
- tromp: But that's not sync from scratch is it?
- antiochp: No we have a "catch up sync".
- lehnberg: Is that a good thing?
- antiochp: Not sure if "good" but I think it's the only way to handle this.
- 👍: lehnberg
- antiochp: if your local node suddenly sees a competing chain that is multiple blocks ahead it needs to request multiple blocks (and my never have seen them via broadcast or relay). "Catch up sync" is basically the same impl of the first stage (request multiple headers) and final stage of full sync which is "full blocks to catch up to latest header".
-
lehnberg: Btw, do we still have no idea whether the attack was successful?
- joltz: There were some successful double spends but I don't know for sure whether any entity lost funds as a result.
- deeev: I guess the 2nd reorg was a fail, but it seems to have been successful for other as big dump happened at the same time they reached ~10 confirmation.
- jaspervdm: Exchanges are only requiring 10 confirmations? Seems awfully low.
- deeev: Some as Kucoin do afaik.
- joltz: Yes, 10 confs in grin is risky for any exchange.
-
lehnberg: Just feels like it was a test this weekend and that they will be coming back for more.
- phyro: Yes, we should expect more. I agree with @deeev, we need to expect these re-orgs to be longer. The current ones were around 30-40min, if they go over 1h then at least a window of 30min won't get rebroadcasted which opens up the users from txs in that window for a double-spend.
-
tromp: Can we recommend confs >= 1440 to exchanges?
- 👍: phyro, deeev
- deeev: Would be great to know the targeted exchange.
- joltz: We can recommend 1440 to be safe but it may be a hard pill to swallow for exchanges and users.
- phyro: Better safe than sorry.
- tromp: Lack of slatepack support is a harder pill to swallow. :(
- joltz: It doesn't help that we don't have full buy in from other ecosystem wallets that will continue to support deprecated methods.
- lehnberg: Let's not confuse topics, slatepack has little to do with the re-org.
- jaspervdm: I'd rather leave that up to the exchanges themselves. I'm sure they have their own risk assessment. If we don't make any recommendations we can't be blamed for it either.
- antiochp: Kind of agree with @jaspervdm, it's up to exchanges to decide their internal rules for this.
- tromp: We are implicitly recommending 10 because of the wallet default.
- lehnberg: Yeah, I believe our current recommendation is in the software, and it's 10 confs. :/
- jaspervdm: How so? We don't provide any software that keeps user balances and allows trades with them after 10 confs. We have 10 confs by default on re-spending of received funds on the wallet but thats a different topic imo.
- tromp: Doesn't seem all that different to me.
- antiochp: I guess
grin-wallet infodisplays "confirmed" funds which defaults to 10 confs. - jaspervdm: Presumably the double spends involved withdrawing some other crypto.
- phyro: How about a statement that they should consider increasing the confirmation times to make them less likely a target of a 51?
- paouky: It definitely makes sense to chage the default to 30, even without considering the recent reorgs, 10 is super low to begin with.
- antiochp: 10 is bitcoins 6 rounded arbitrarily up to 10.
- paouky: We can choose 60 confs to be on the same side of bitcoin.
- tromp: For grin-wallet to receive, say, 100k Grin, it would indeed be wise to require more confs on the inputs.
- 👍: phyro
- antiochp: Yes agreed - 10 was simply to minimize the chance of the wallet getting messed up due to "natural" 1-depth reorg. But not sure that can be automated reliably.
- joltz: The problem with basing it on # of grin is that there is no way to know the actual value of the grin. 10k could be <$1 in theory.
- jaspervdm: Currently there are no restrictions on the receiver side on the age of the inputs, it all happens on the sender side.
- antiochp: No we use # confs when displaying balance with wallet info.
- phyro: Number of grins = number of seconds for confirmation, with a minimum of 30 min or so lol.
-
paouky: Can we agree as a first step to at least increase the default confs for grin-wallet balance?
- antiochp: I'm not convinced we want to - having 30 makes wallet cli confusing (funds take ages to arrive). An exchange accepting funds is an entirely different use case to somebody trying grin for the first time. And the wallet defaults were biased toward new user trying it out "wait 10 mins for funds to confirm" for example (where 10 mins was roughly equivalent to a single bitcoin block).
- joltz: My main concern with increasing this number now is that services will blindly use it again without understanding the implications.
- jaspervdm: We could have different defaults for the cli and the api. But yeah, regardless of what we put the default api value at, I don't think exchanges should be using it blindly. So we might as well leave it at 10.
- antiochp: Maybe the api should require an explicit value in there, but hard to do this now.
- jaspervdm: Yeah would require us to introduce a new api version (technically changing default value does too).
- joltz: I'm not sure it makes sense to force all uses and tx's to require 30 min. It would be up to each use case to increase this based on their risk model. Otherwise it is a potential reduction in overall usability imo.
- antiochp: Yes wallet is already too complex for new users.
-
antiochp: Ok I'm not sure how we resolve the "default confs" debate given we only have the reference wallet impl shared by cli and api.
- deeev: Maybe adding a new category?
- mcmmike: Make it optional in the grin-wallet.toml file to increase the block-waiting-time.
- phyro: Conf times are a per-transaction thing really. If I make a tx with a person I trust, I don't need many confs.
- jaspervdm: Currently the min number of confirmations on inputs is already a cli argument and api configuration field. I think its fine as-is.
- 👍: quentinlesceller, antiochp
- joltz: Or even simplify it as risk/confidence with low (10 confs), medium (60 confs), high (1440 confs)
- phyro: Add paranoid (1440 * 7).
- vegycslol: I don't think changing default confirmations will change anything. Exchanges are an exception, we need it to be nice for majority of users. I doubt exchanges are that stupid to not understand the consequences of having a fixed low confirmations, they just don't want to do it because they would lose users imo.
- lehnberg: Happy to leave as is.
- joltz: It might be more appropriately managed at a higher level- in exchange software or wallet gui wrapper.
- 👍: antiochp
- tromp: What I would like is a configurable option to use max(10,v/30) age for output status confirmed, as suggested in https://forum.grin.mw/t/improvement-proposal-dynamic-tx-confirmation-times/7945/3
-
lehnberg: #287
-
antiochp: Lots of in progress items.
-
lehnberg: Yes, anything that's inaccurate and needs an update?
-
jaspervdm: So two weeks until targeted beta release date. jaspervdm
- lehnberg: Does it still feel realistic?
- jaspervdm: From my side, I think yes.
- lehnberg: If you had to take the lead on @yeastplume's items, would you manage?
- jaspervdm: For #4 yes, #5 I'd have to look at it. If its merely removing some code to support http that should be doable as well.
- lehnberg: Okay, just thinking, as we've not heard from yeast in a while now.
-
tromp: My HF4 and fixdaa PRs are ready for review. Fixfees still has failing
test_transaction_json_ser_deserdue to string vs number encoding, which jasper is helping me with.- jaspervdm: Oh right, I will fix that asap. We do have to discuss increasing slate version for your changes though. That would mean supporting old slate (pre-HF) and new slate at the same time.
- tromp: If we use same u64 encoding for FeeFields, then there's no real difference with new slates.
- jaspervdm: But there is a semantic difference. If an upgraded wallet communicates with an non-upgraded wallet they disagree on the actual fee.
- tromp: They can only disagree if wallet uses a positive shift. Which the wallet can't do yet.
- antiochp: 0 shift is effectively backward compatible right?
- tromp: Yes
- jaspervdm: Ok
- tromp: All existing fees are implicitly
0 fee_shift. - antiochp: And we're not necessarily looking to expose this in the wallet yet, just internal impls.
- tromp: It could be many years before wallets have need to support positive fee shift. When blocks fill up.
- antiochp: So can we get away with no slate version change for now?
- tromp: Yes
- 👍: antiochp
- jaspervdm: Ok good
- tromp: Just need to force the same u64 encoding for
FeeFieldsas we had for plain fee.- 👍: antiochp
- jaspervdm: Yep will get on that.
- antiochp: @tromp you are blocked on those other PRs waiting for review?
- tromp: Yes, awaiting review now. Need to properly assign reviewers on PRs.
- antiochp: Ok I'll aim to take a look over the next couple of days.
-
jaspervdm: For the PIBD there are 4-5 PRs that are all nearing completion and hopefully we can move to merge the first ones of them soon.
- antiochp: Yes PIBD work is getting close to integrating a few separate PRs.
-
tromp: Can we come back to the slate version issue? there is one actual change. In some cases, the field name is changed from "fee" to "feef".
- jaspervdm: Oh right. What if we stick to the "fee" name? We don't have a need to support fee and feef simultaneously right?
- tromp: No. Let's first get all tests passing with current feef, and then I'll change it back to fee, and make tests pass again.
- antiochp: Shall we wrap the dev meeting up and continue with that? Anybody have anything else? otherwise I think we're done?
- 👍: quentinlesceller, jaspervdm
- jaspervdm: Sounds good. In conclusion I think there are no major roadblocks for any of the points on the list to get the beta out in 2 weeks.
- 👍: antiochp
None.
Meeting adjourned.