Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIP11: Diem Accounts #155

Open
sblackshear opened this issue Mar 26, 2021 · 9 comments
Open

DIP11: Diem Accounts #155

sblackshear opened this issue Mar 26, 2021 · 9 comments
Assignees

Comments

@sblackshear
Copy link
Contributor

This is a discussion and feedback thread for https://github.com/diem/dip/blob/main/dips/dip-11.md

@dahliamalkhi
Copy link
Contributor

First, this is an awesome DIP!

At a high level, first I'd like to propose that the title indicate this DIP is about account and the framework for determining to admit transactions on their behalf to the blockchain (so, something like "Diem Accounts and Transactions they may Submit".

Second, I think it will be helpful to start the second part (framework for determining to admit transactions) with a high-level skeleton: pre-processing conditions on transactions -- conditions during provisional processing and gas calculation -- epilogue processing. Then provide details for each part.

@aching
Copy link
Contributor

aching commented May 3, 2021

This has been in draft since 3/15 - 1.5 months. It's also been implemented and deployed for sometime, so let's get it moved to the next phase.

In addition to @dahliamalkhi's comments, would you consider this to be a standards DIP rather than informational @sblackshear ?

@bob-wilson
Copy link
Contributor

This DIP seems to be stuck. What needs to happen to move it forward?

@aching
Copy link
Contributor

aching commented Jun 23, 2021

Can someone comment on whether this is a standards vs information DIP? Also, we need a response on @dahliamalkhi's comments.

@bob-wilson
Copy link
Contributor

When I first discussed this with @sblackshear months ago, he suggested that we make it an informational DIP. That makes sense to me, since this was written after the fact to describe how things work. Is there some criteria that would argue in favor of making it a standards DIP?

I don't have an opinion on Dahlia's suggestion to change the title. Either way is fine with me.

With regard to Dahlia's second comment, I share the feeling that the text is a bit awkward. The issue here is that the DIP is focused on Diem Accounts, but we also wanted to include a specification of the prologue and epilogue functions and their relationship to transactions, because those functions are included as part of the DiemAccount Move module. I extracted this info about prologue and epilogue functions from the Move Adapter specification, but it is missing a bunch of context here. I guess I can try to add a bit more, and maybe refer to the Move Adapter specification as well.

I also just realized that this DIP is out of date because we've modified the adapter spec to cover multi-agent transactions. We also need to update the adapter spec for CRSN. Both multi-agent and CRSN have separate DIPs. Should we update this DIP as well to reflect changes from those features? It would seem strange to publish an informational DIP that is already outdated.

@aching
Copy link
Contributor

aching commented Jun 23, 2021

In my opinion, per https://dip.diem.com/overview

"Informational DIPs describe a Diem project design issue, or provide general guidelines or information to the Diem community, but do not propose a material change or addition to DPN. Informational DIPs do not necessarily represent Diem community consensus or a recommendation, so users and implementers are free to ignore Informational DIPs or follow their advice."

I agree that publishing DIP standards after they have been deployed is the wrong order, however, I still think they are standards. Ethereum also has very few informational EIPs (e.g. https://eips.ethereum.org/).

@aching
Copy link
Contributor

aching commented Jun 23, 2021

You also ask a great question about publishing out-of-date DIPs. I agree it's pretty weird to publish it out-of-date. Maybe the text could be slightly reworded to take those into account?

@bob-wilson
Copy link
Contributor

You also ask a great question about publishing out-of-date DIPs. I agree it's pretty weird to publish it out-of-date. Maybe the text could be slightly reworded to take those into account?

We could, but it does make me wonder if a DIP like this is the right way to specify details of the accounts. It's almost certain that there will be other changes in the future that will make whatever we include here out-of-date. Maybe it's better to put this in the form of a specification (as we've already done for the content about the prologue and epilogue functions) that we keep up to date as we go? At the least, this would be an argument for making this an informational DIP.

@aching
Copy link
Contributor

aching commented Jun 25, 2021

The difference between readable documentation and changes to specifications that you bring up in a problem for sure. Coupled with the confusion on DIPs after specifications have been deployed, this is more confusing.

Perhaps the best example of the way to do things is to think of the DIPs as authoritative, but we can aggregate/re-format/fix typos that information into more comprehensive documentation - e.g. https://github.com/diem/diem/tree/main/specifications/transactions/offchain-api

In this case, I agree the accounts are already part of the specification but not written out as part of a DIP (like consensus). I'd be fine if you simply add the information to the specifications and then focus on the multi-agent as a new specification DIP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants