Open sblackshear opened 3 years ago
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.
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 ?
This DIP seems to be stuck. What needs to happen to move it forward?
Can someone comment on whether this is a standards vs information DIP? Also, we need a response on @dahliamalkhi's comments.
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.
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/).
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?
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.
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.
This is a discussion and feedback thread for https://github.com/diem/dip/blob/main/dips/dip-11.md