Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Standardize language around deposits #87

Closed
maurelian opened this issue Dec 17, 2021 · 12 comments · Fixed by #144
Closed

Standardize language around deposits #87

maurelian opened this issue Dec 17, 2021 · 12 comments · Fixed by #144
Assignees
Labels
design Design discussion specs

Comments

@maurelian
Copy link
Collaborator

maurelian commented Dec 17, 2021

We have a lot of things using the term "
deposit", which are similar but different.

  • Deposit
  • Deposit transaction
  • Deposit transaction type
  • L1 Transaction Deposits (Transaction)
  • L1 Attributes Deposits (Transaction)

It's fine for now, but this will get confusing eventually.

@maurelian maurelian modified the milestones: post-deposit-only, deposits-only Dec 21, 2021
@norswap
Copy link
Contributor

norswap commented Jan 7, 2022

Proposal:

  • a depositing transaction is a L1 transaction that submits a deposit by calling the deposit contract
  • a depositor is the L1 account that sends (is the tx.origin) of a depositing transaction
  • a deposit block transaction is a L2 transaction that was derived from L1 and is included in a deposit block
  • a transaction deposit is, depending on the context, any of:
    • the call to the deposit contract that "creates" a deposit (disambiguate by adding (L1 call))
    • the event generated to represent the deposit (disambiguate by adding (L1 event))
    • a deposit block transaction generate by the above (disambiguate by adding (L2 transaction))
  • a L1 attributes deposit is a deposit block transaction that submits the L1 block attributes to the L1 attributes contract (on L2)
  • a deposit is, depending on the context, both:
    • a transaction deposit
    • a deposit block transaction
    • (notice both meaning overlap)

I tried to be congruent with our current usage and make it easy to use the right term. e.g. it's useless to pin down the meaning of deposit to something hyper specific as the term is going to be used in all kinds of situations. The context will often be a guide, but this should provide a clear guideline on what can be used, and how to disambiguate if needed.

@norswap
Copy link
Contributor

norswap commented Jan 7, 2022

I'd also like to reintroduce the proposal to rename the "deposit feed contract" to "deposit contract". It's simpler and never ambiguous. Is this still controversial? (@ben-chain ?)

My main argument is that feed seems to entail a continuous nature (where we can process deposits one by one), whereas deposits are actually chunked by L1 blocks and are processed in batches. So the term is somewhat misleading.

@maurelian
Copy link
Collaborator Author

Just to add some context, the 'Feed' term only starts to make sense when you consider it with a sequencer included. So that you have multiple sources of transaction input. Then you might want to say "Deposit Feed" and "Sequencer Feed".

So maybe it's a term that would be included in the context of the Rollup Node specs, but I agree it seems OK to remove it from the name of the contract and the sections of the spec which are mostly focused on Deposits.

@norswap
Copy link
Contributor

norswap commented Jan 9, 2022

I understand what you are saying, and still I don't think it follows that the terms has a place.

Even when using the sequencer, deposits landing on an L1 block map to a deposit block. To me, opposing "deposit feed" and "sequencer feed" seem to suggest that these two feeds are somehow merged. They aren't. Instead, you could say that we have a "deposit block feed" and a "sequencer block feed" and those are merged. Not sure we really want to go there.

Here is a strawman explanation (that assumes some prior knowledge of the design):

Deposits are L2 transactions submitted to the deposit contract on L1. For each L1 block, the rollup node makes a deposit block containing the deposits that landed in that L1 block. The sequencer receives L2 transactions from users and makes sequencer blocks out of them. The L2 blockchain contains both deposit blocks and sequencer blocks. The sequencing window constrains how many sequencer blocks can appear in between deposit blocks.

It's really not clear to me how using the term "feed" can make this explanation any better.

@maurelian
Copy link
Collaborator Author

maurelian commented Jan 11, 2022

I think we should defer the 'feed' discussion. It's probably fairly easy to delete that from a few places if/when we agree to do so.

TBH, I'm struggling a bit with the proposed terminology, but it's difficult to provide concrete feedback on it. One idea I have that might help is to introduce the term "deposited". Then we can distinguish between concepts on L1 and L2 by using "depositing" on L1, and "deposited" on L2. Here's a quick diagram.

@norswap
Copy link
Contributor

norswap commented Jan 12, 2022

@maurelian

Hard agree that the "feed" stuff should not delay a discussion on deposit terms and can be postponed.

Really like the proposal of using "deposited" and "depositing".

Patching my proposal to include your suggestions. What I've added:

  • use deposited and depositing as per your suggestion
  • systematically use "transaction" for the term that specifically refer L1/L2 transaction, keep "deposit" vaguer
  • use user-deposited transaction to distinguish between those and the L1 attributes deposited transaction

Does that seem closer to what you had in mind?

Oh, and it would be great to have something like that chart in the docs later on!


  • a depositing call submits a deposit by calling the deposit contract
  • a depositing transaction is a L1 transaction makes at least one depositing call
  • a depositor is the L1 account that sends (is the tx.origin) oa depositing transaction
  • a deposited transaction is a L2 transaction that was derived from L1 and is included in a deposit block
  • a user-deposited transaction is a deposited transaction derived from a depositing call on L1
  • a L1 attributes deposited transaction is a deposited transaction that submits the L1 block attributes to the L1 attributes contract (on L2)
  • a user deposit is, depending on the context, any of:
    • a user-deposited transaction
    • a depositing call
    • the event generated by a depositing call
  • a deposit is either:
    • a user deposit
    • a L1 attributes deposited transaction

@maurelian
Copy link
Collaborator Author

  • a depositor is the L1 account that sends (is the tx.origin) oa depositing transaction

I think it should be msg.sender.

Otherwise this looks really good to me.

@norswap
Copy link
Contributor

norswap commented Jan 13, 2022

That was on purpose, I meant the depositor to be the user that sends a transaction that does deposit calls.
But I'm open to the alternative, the depositor being the contract that makes the depositing call?
Couldn't we call that the depositing contract instead?

@maurelian
Copy link
Collaborator Author

maurelian commented Jan 13, 2022

It just seems like tx.origin doesn't even enter into it. L2 has no awareness of that account IIRC?

Also I can imagine applications that makes deposits which there tx originator isn't even aware of, like a DeFi aggregator (ie. yearn, rari) that accepts a deposit of collateral on L1, then deposits it to L2 for some yield farming.

@norswap
Copy link
Contributor

norswap commented Jan 14, 2022

That's correct.

Fine by me to have by msg.sender - you make a good case - as long as it's defined clearly enough (since my intuition if I heard "depositor" would for it to be the transaction sender instead of the call sender).

@maurelian
Copy link
Collaborator Author

I think we've agreed on this. :)

@norswap
Copy link
Contributor

norswap commented Jan 20, 2022

@maurelian Reopening this for tracking that we actually change the spec accordingly. If you feel it should be a separate issue, feel free to re-close and open a new one!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design Design discussion specs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants