Skip to content

Latest commit

 

History

History
286 lines (248 loc) · 23.8 KB

20200707-meeting-development.md

File metadata and controls

286 lines (248 loc) · 23.8 KB

Meeting Notes: Development, Jul 07 2020

Development meeting held @ 3PM UTC in grincoin#dev channel on Keybase. Meeting lasted ~ 1h 30 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
  • dburkett
  • joltz
  • kurt3
  • lehnberg
  • paouky
  • phyro
  • quentinlesceller
  • tromp
  • yeastplume

(apologies if I missed someone - submit a PR or contact @lehnberg to add)

Agenda points & Actions

1. Retrospective

  • yeastplume: We released 4.0.0 last friday, and it seems to be propagating itself into the field without too much issue. Other than that I think a few people are taking a well-deserved bit of rest, trying to remember where they left their kids etc. But 5.0.0 (and perhaps 4.1.0) planning is kicking off right about now, I expect we'll have plenty to say about upcoming work during this meeting.
    • lehnberg: Great job all the devs for shipping last week!
      • 💯: yeastplume

2. Agenda review

The proposed agenda was accepted without modifications.

3. Action point follow ups from previous meetings

  • quentinlesceller: Can be considered done but I want to add the proper channel for announcements in there. Will do today.
    • yeastplume: Great, yes will be handy to have our announcement channels in a handy list.

4. Status of Grin v4.0.0

  • lehnberg: What's outstanding at this point? Is the hard fork ETA Jul 19, still?

    • quentinlesceller: July 16 2020, around 11 pm UTC.
      • 👍: lehnberg
    • yeastplume: I don't think anything, and no major issues appear to have been uncovered thus far.
  • lehnberg: There's a grin-miner PR pending right?

    • yeastplume: Ah, yes. That will give us a grin-miner 4.0.0.
    • quentinlesceller: Yep.
    • lehnberg: Which should arrive shortly prior to hard fork.
      • 👍: yeastplume
  • lehnberg: That's it?

    • yeastplume: Think so. I think we can remove the 4.0.0 status item from the agenda, just add an action point for grin-miner 4.0.0 release.
    • antiochp: Those changes are limited to grin-miner only? No changes in grin or grin-wallet, right?
    • yeastplume: Yes, literally just the addition of a new plugin.
      • 👍: antiochp
  • lehnberg: What are the odds of there being an exchange still around supporting grin in 9 days?

    • yeastplume: I remain hopeful.
    • lehnberg: I'm saying this with a bit of a wink, but yeah there's some degree of seriousness as well. Is there anything we ought to do here to smoothen the transition?
    • joltz: I think the changes we are making now will make it more likely for exchanges to be able to support grin in the long term.
      • 👍: yeastplume
      • lehnberg: Yes, I hope so too.
    • quentinlesceller: Doc.
      • 💯: joltz, lehnberg
    • lehnberg: Friction with scheduled hard forks is unavoidable.
    • antiochp: Agreed - might be a bit rocky for a bit, but hopefully easier for them to support more robustly in the future.
    • lehnberg: Mining pools I would imagine are all aware, right?
    • antiochp: Slatepack + "mostly lock free" should make things significantly easier (once this is all in place).
  • lehnberg: @quentinlesceller we did announce in the ecosystem chats and so on, correct?

    • quentinlesceller: Yes I did.
      • 👍: lehnberg
  • lehnberg: Ok, I'm just worried for no good reason I guess. 😥

    • joltz: It's good to be worried - lots can go wrong when changing parts once you are already flying. :)
    • lehnberg: We're definitely better at getting our ducks in a row, earlier. Third time's the charm. :)
    • lehnberg: Yet, I still can't wait until the last one is behind us.
      • 👍: quentinlesceller
  • lehnberg: Is there anything new to add there? @yeastplume I imagine you've not had a chance to dig deep into this with all the 4.0.0 stuff going on.

  • yeastplume: Not really, but am at least familiar enough with the issue and the thread, which has been filled out quite a bit with discussion and opinions. Happy to discuss a bit now.

  • antiochp: My thoughts on that are here - https://forum.grin.mw/t/replay-attacks-and-possible-mitigations/7415/13

    Seems to boil down to -

    1. try to prevent it at consensus level, or
    2. have nodes/wallets aware of the risk and mitigate locally.
    • yeastplume: I'm a bit wary of adding more complexity to the wallet, but can also see the cost of enforcing at consensus level is significant for something that's so infrequent.
    • phyro: May I add that preventing it at consensus level makes ideas like merging kernel points impossible, because you need to keep them around forever, so the very light "nimble" node becomes impossible (I think).
    • lehnberg: If we'd decide to go with 1), it would be prime candidate for 5.0.0. If 2), then it's something that can be iterated on irrespectively.
    • yeastplume: Right, and the nice thing about 2) is that it can go in at any time.
    • antiochp: We also have no scalable way of actually implementing (1) - its a non-starter. We'd need to track history globally for all time.
      • lehnberg: Sounds very un-mimblewimbly.
      • yeastplume: Right.
      • antiochp: Which would appear incompatible with mw (at least in spirit).
      • lehnberg: Well, that killed the vibe quickly. Haha.
      • antiochp: ¯_(ツ)_/¯
  • yeastplume: So we can try and hide as much of the detail as possible in the wallet restore function, then have some sort of alert at the end of the restore process if needed. Developers will need to pay attention to it and handle.

  • kurt3: @antiochp a message can be added to the signature to help verification, have you considered this approach? It augments the size of the kernel by 5%, or less.

    • antiochp: Maybe I'm forgetting something - @kurt3 can you explain? I'm definitely not purposefully ignoring anything.
    • kurt3: I posted the idea in my original post. On kernel uniqueness. And it was disccussed at least multiple times in #crypto.
    • antiochp: Adding pos to the kernel msg? Or counter?
    • phyro: I think one could add "max_height" validity block height to the kernel and sign it. If the replay attacker reused the kernel after that height, it would be invalid. I'm not sure I recall it correctly though, could you confirm kurt? It doesn't prevent the attack, it shrinks the window of opportunity.
    • kurt3: Block - 1, block + 50 or something like that in the signature yes. It improves verification a lot. Thats the goal.
    • antiochp: If its max_height then I think we're keen on keeping to bitcoin's "once a tx is valid it will not become invalid" (exception being a double spend) rule of thumb. And as @phyro says - it mitigates but does not prevent.
    • phyro: Maybe in theory, it might prevent it completely if combined with nrd.
    • kurt3: Combines with nrd I also provided an idea. After looking at antioch proposal. The scalability cost is minimal.
    • antiochp: I do think we should handle this locally (on wallet restore and via wallet tx history) and not at the consensus level.
    • kurt3: At restore good luck.
    • antiochp: Can you explain? Wallets are only ever concerned with local risk due to replay - there is no benefit to handling this at consensus level across all utxos.
    • dburkett: I can. It's a ux nightmare. How do you indicate to the user that there's a possibility of transaction replay in a way they'll understand?
    • antiochp: One option would be to automatically sweep in this scenario (has its own set of tradeoffs though).
    • dburkett: A lot of complexity involved with that, but yeah that's an option. Still ends up with a confusing transaction history, but not as bad of a ux as just expecting users to understand.
  • dburkett: The only real tradeoff is, like antioch said, transactions eventually expire. The big issue with that is not actually the transaction pool (txs should expire after being in the pool for days). But with transaction building. There's now a time limit on when a transaction must be received and finalized.

  • joltz: Mitigating in the wallet with accompanying well-written and easily findable documentation for developers might end up with an overall reduced security risk compared to making consensus-level changes. The key is mitigating potential loss and what the cost for that is.

  • phyro: It adds a bit to the kernel size since you'd probably want to make that default, so around 5% kernel increase I believe. Also, additional consensus validation for ibd etc.

  • tromp: I'm in favor of keeping monotonicity; once valid, a tx should remain valid as long as its inputs remain unspent.

    • dburkett: Why?
    • tromp: Simplifies mempool management.
    • dburkett: I know it sounds like a nice principle. But what is the practical reasoning? Mempool management is way simpler than wallet management. Like, 10x simpler. Fwiw, I don't have a strong opinion either way. But mw already creates enough difficult situations for the wallet developers. I'd hate to needlessly add complexity there.
  • lehnberg: I don't have a horse in this race, but the complexities around wallet state mgmt is only relevant during restore, correct?

    • dburkett: No, not at all. The complexity is in the sweeping, and showing transaction history.
    • lehnberg: ...applicable, when the wallet is being restored from seed, correct?
    • dburkett: No. At any time.
    • lehnberg: Okay, then I'm not following the discussion properly, and need to read up more about it.
  • tromp: Also don't like mandatory tx expiry.

  • joltz: Couldn't it be configurable for the wallet? With the default option being the most restrictive for the user such that we prevent them from getting burned at a cost of making decisions for them. Then they can always configure manually if they don't want funds to auto sweep or whatever we decide is the safety net.

    • dburkett: Still, that's adding complexity to the wallet.
    • joltz: I agree but if complexity has to be added somewhere, why not the wallet instead of the chain? We already are stacked in that direction.
    • dburkett: Because grin already is one of the hardest coins to develop a wallet for.
    • joltz: I guess at some point the wallet becomes too complicated and useless for our simple chain taken too far.
  • antiochp: May also be important to take baby steps here as well - starting to highlight "at risk" outputs may be an initial step.

  • tromp: Complicating the consensus model should be last resort.

  • dburkett: Theoretically, yes. Practically, I don't know if I agree.

  • lehnberg: So @dburkett what do you suggest should be done here? For the sake of clarity, do you think the way to go is what @kurt3 was proposing above?

    • dburkett: I don't know. More thought is needed. But blindly passing it off to the wallet because of simplicity principles will further discourage third party wallet devs to take a stab at grin. There are far fewer node implementations than wallet implementations.
    • lehnberg: I'm not sure I agree with the assessment that it would be done blindly. But yeah perhaps the best way to ensure enough thought is given to this is to continue lighting up that forum thread.
      • 👍: dburkett
  • yeastplume: There are quite a few more action points to go, can we leave it there for now and continue discussion in the thread now that it's been re-energized?

    • 👍: antiochp, phyro
  • joltz: 👍 we have to find the right balance here.

  • yeastplume: This is a bit related to 5.0.0 planning, and there was movement in the thread since last. It might make sense to see if there's agreement about approach here?

    • antiochp: Its closely related to 5.0.0 planning, yes. I'm making the argument to not do this for 5.0.0 - https://forum.grin.mw/t/coinbase-outputs-as-transaction-outputs/7441/38.
      • 👍: joltz, dburkett
      • yeastplume: And not make a potentially contentious change of something that's been working well enough on our last scheduled hardfork?
    • antiochp: We can leave the enticing possibility of a contentious issue for a future unscheduled hardfork...
    • lehnberg: It's great that you stuck your neck out in the thread with a clear opinion..
      • yeastplume: He can expect punishment shortly.
      • antiochp: Lol.
  • quentinlesceller: Agree with @antiochp here doesn't seems to be worth it.

  • lehnberg: It would be good to hear if there's any strong disagreement with that position at this point.

  • antiochp: Its definitely worthy of further discussion and is an interesting point (why follow bitcoin if there is no legit reason for it etc.). But flipside is - it's not without various risks and unknowns.

    • lehnberg: Yes, I agree. For me it's a fear of us screwing something up badly and then needing to fix it in panic mode. Like I said somewhere, if we were in testnet, or heck if we had a couple of extra future forks even, I'd be keen on experimenting a bit more.
    • antiochp: Adding this to the final hf4 would be a curious decision for something contentious like this.
  • tromp: Removing the coinbase feature from inputs should be less controversial though. We shouldn't have to specify whether an input is a coinbase or not.

    • antiochp: Ah yes - that is worth considering @tromp. And is related to "duplicate outputs".
    • dburkett: That doesn't really need a hardfork though. It's just a p2p message change.
  • quentinlesceller: What's the risks of removing the output maturity rule?

  • phyro: Bitcoin had a different tradeoff, much clearer in favor of maturity rule in their case. Risk is a reorg where coinbase outputs are building The transaction graph. The whole graph gets cut off.

  • tromp: The risk is a loss to buyer of fresh coinbase in case of deep reorgs.

  • joltz: I guess a miner could buy a bunch of asics with their coinbase and hope they ship before reorg hits merchant node.

    • quentinlesceller: Are the maths any different that a double spend attack?
  • antiochp: And buyer of fresh coinbase (or descendent outputs) without knowing it is coinbase.

    • 👍: phyro
  • paouky: How deep of a reorg do you think is possible? These risks seem a bit far fetched to be meaningful.

  • tromp: Let's say a few dozen blocks.

  • joltz: Would have to think about it more. Intuitively it sounds cheaper but might be wrong.

  • quentinlesceller: Like what are the difference in odds (if any) between double spending a coinbase and a regular output.

  • paouky: These risk even if they play off, are not detrimental. I think they are bearable. But I'm not sure how much privacy benefIt do we actually get from this change. It boils down to whether it's worth the small risk. I'm not sure myself, that is. I'm sure a few of you understand it really well.

  • lehnberg: I think we're quite confident that it's not adding much in terms of privacy, other than cosmetic obfuscation. We have to assume anyone serious about deanonymizing grin transactions is monitoring the mempool, or am I wrong about that?

    • joltz: That is a fair assumption.
    • antiochp: I think that's fair.
  • lehnberg: And we can also say that this change would not add any mempool obfuscation, correct?

  • joltz: No one is going to squint and only look at the chain data only.

    • lehnberg: I'm sure some will, but these are the same people that will not be able to tell less from having one extra Indistinguishable output in the mix. i.e. the people that only look at the chain in order to deanonymize is hardly the people we are designing for in this case. I think..
  • tromp: I think this is a case of: If we could've changed this before launch, there is a strong argument to be made. But given that we launched with maturity, we should perhaps stick with it for now.

    • 👍: antiochp, quentinlesceller
    • lehnberg: Yeah I think that's where I'm at mentally. And I'm kind of sad to say so.
  • phyro: Mempool might be temporary. Theoretically people could be using whatever crypto outside the chain which could improve the p2p sniffing.

  • joltz: It does make it a little more expensive maybe.

  • phyro: That is just my guess though, not saying it will happen.

  • lehnberg: If seems like a lost opportunity to be even simpler and more elegant.

  • tromp: The coinbase kernel will always be part of our consensus model now.

  • paouky: There's no community consensus on block maturity, most do not even know it exists. So I disagree on that one one @tromp @lehnberg.

  • antiochp: I would like to be convinced that there would be no subtle but significant change to the incentives introduced by removing the maturity rule.

  • lehnberg: @paouky let's flip it instead. Why do you think this is important?

  • tromp: In blockchain, you don't change consensus model unless there is broad community consensus for a change.

    • paouky: Ok fair enough.
  • paouky: As I mentioned, I do not understand the full privacy implications like some of you. But I think the proposal should be judged as if we're before launch. Making a change in maturity is a technical consensus change, But not a mental one, if you know what I mean.

  • paouky: You know better.

  • lehnberg: If we get it wrong, it's going to be hard to fix given that this is our last hard fork.

  • tromp: Although that is more relevant after the first 2 years of grin.

  • yeastplume: Okay, so sounds like this is punted for now, with more discussion to be had regarding inputs.

7. v5.0.0 planning

  • lehnberg: Planning issue: #287 I suppose I ought to get everything into a nice list for next meeting. And we can try to take stuff out from it. So, we've talked about replay attacks, that's a contender. We've talked about coinbase rewards, that's a contender (although leaning towards punting). Since last dev meeting, newcomers to join the list are:

    • "max block weight". mimblewimble/grin#3368.
    • Fees mimblewimble/grin#3369. Yeast wrote
    • consolidation of wallet commands / api functions (e.g no need for separate 'receive', 'finalize' etc commands) doesn't have to be 5.0.0, but a 4.1.0 should contain the 'final' version of all wallet commands so everyone has time to adjust before 5.0.0
    • method of slatepack address cycling
    • more optimised sql backend option for scalability needs (does not need any particular hf or major/minor release)
    • (nebulous thought) any changes/standards that might be desirable for an offline transaction relaying solution? Highly dependent on what direction we decide to take with this.
  • lehnberg: @yeastplume want to expand on your nebulous thought there? What were you thinking?

    • yeastplume: No, too nebulous to form into words at the moment.
  • lehnberg: I also wanted to propose we throw in: Bulletproof iteration. Both bulletproof+ (which could reduce future chain size by 15% moving forward). But also, there may be some optimization in verification, if I'm not mistaken. Think it's in some tari post somewhere. There's a rust imp of bp+ done already at https://github.com/kzen-networks/bulletproofs.

    • quentinlesceller: Updating our forks of libsecp256k1 would optimize a lot of things.
  • tromp: We should be happy to take that size reduction. Given that ibd and block size are dominated by rangeproofs. Even if there were a small verification time penalty. But I don't know if we want to wait until our preferred library supports it.

    • 👍: yeastplume
  • lehnberg: Someone like jasper would be great for this type of work. Hopefully we'll see him back in time to be able to take it on.

  • quentinlesceller: No hf needed though.

  • lehnberg: Oh, bp is not consensus?

  • tromp: The format is.

  • lehnberg: Dang I never learn...

    • 😂: paouky
  • tromp: The verification optimizations are not.

    • lehnberg: Ok, then that's great!
  • quentinlesceller: Stuff like that mimblewimble/rust-secp256k1-zkp#71.

    • lehnberg: Is not? Or is?
    • quentinlesceller: Not consensus breaking. Afaik. It's just optimization and safety stuff.
    • paouky: Secp256k1, what qeunting and tromp and talking about, is not about bp+ @lehnberg.
  • tromp: Consensus-breaking: What's in the proof. Not breaking: How you verify it.

  • paouky: I see your confusion.

  • quentinlesceller: I don't want to derail the 5.0.0 stuff since we can do it any time. But better do it soon :)

  • tromp: We should not implement the space savings until the improvements are blessed by the cryptographers community. In case there's some bug lurking in the proofs.

    • 👍: joltz, antiochp
    • lehnberg: Okay, so that's an argument against trying to fit bp+ into 5.0.0, correct?
    • tromp: Correct.
    • lehnberg: 👍
    • tromp: It's a bit young to mandate inclusion in 5.0.0.
    • antiochp: Not just an argument against, but likely a blocker.
    • tromp: No big deal if we get the space savings a year or 2 later.
  • lehnberg: Understood. So aside from those points and features mentioned above, are there more worth mentioning today? Anything we're missing?

    • joltz: The sooner we get payjoin in the better for privacy imo. Not consensus breaking though.
    • dburkett: I don't believe we get many privacy benefits from payjoin..
    • joltz: It breaks directionality.
    • dburkett: It's mostly valuable for obscuring amounts.
    • joltz: Which is a major heuristic for analysis.
    • dburkett: But it leaks inputs to potential senders.
    • joltz: Not when implemented correctly.
      • dburkett: How would you avoid that?.
    • dburkett: There's privacy tradeoffs. The sender needs to know all inputs being spent.
    • joltz: Ah I see, yes it does leak the one input. I thought you were talking about utxo harvesting.
      • 👍: dburkett
    • joltz: That is not a bad tradeoff to leak one input between parties that mutually agree to break the entire graph of directionality for the network imo.
    • lehnberg: I don't see how it can be a detriment, if it's opt-in / consensual.
    • dburkett: Yea, but how do you mutually agree? Will it be a user decision? Or Will wallet decide at random?.
    • joltz: Both wallets would have to have --payjoin flag or whatever. However they want to configure.
    • tromp: Wallet decides by setting.
    • joltz: You only need a few parties using payjoin to get the privacy benefits for the whole network.
    • dburkett: Then it's not mutual. The receiver has to expose to every possible sender.
    • joltz: Payjoin isn't for privacy of alice and bob it is for privacy of the whole network. Which include alice and bob.
    • dburkett: Well, technically mutual, but you can't just approve for a trusted sender..
  • yeastplume: We're way over time and I need to run, so going to allow @lehnberg to wrap up the 5.0.0 planning issue, or call an end to the 'official' meeting. Thanks everyone!

    • joltz: Thanks @yeastplume 👋
    • 👋: antiochp
  • joltz: @dburkett it is nice if like a merchant runs payjoin on their server and customers can choose to use it or not.

    • dburkett: True.
  • lehnberg: As part of the agenda point, we also had: "soft fork proponent arguments published?"

    @dburkett I know you've a lot on your plate these days, but are you planning on making that soft fork argument post on the forum? Should we keep it as an open question on the planning list?

    • dburkett: Yea, I've been meaning to do that.. Keeping it as an open question would be nice.
    • lehnberg: Cool, then let's do that.
      • 👌: dburkett

8. Other questions

None.

Meeting adjourned.