docs: update README.md#5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the README.md to include comprehensive documentation on the ERC-7702 delegation mechanism, including its security model and instructions for adding, verifying, and revoking delegations. A technical correction was suggested for the revocation section to clarify that delegating to the zero address does not return the account to an empty code state, but rather points it to the zero address.
kaze-cow
left a comment
kaze-cow
left a comment
There was a problem hiding this comment.
the documentation still feels unnatural and difficult to read. some parts still feel too verbose.
please read the whole document carefully from beginning to end before submitting for review again 👍
| SolverEOA --> DirectTx --> Settlement | ||
| AuxEOAs --> DelegatedTx --> DelegatedSolver --> TargetCall --> Settlement |
There was a problem hiding this comment.
this diagram doesn't do a very good job at reinforcing the fact that the Solver EOA is ultimately what is being called by the AuxEOA (which, in turn, points its code delegation to this deployed contract). A person looking at this diagram could ceivably make the mistake in believing that AuxEOA is calling something separate.
There was a problem hiding this comment.
Brought in the same diagram from here: cowprotocol/docs#627 (comment)
| SolverEOA --> DirectTx --> Settlement | ||
| AuxEOAs --> DelegatedTx --> DelegatedSolver --> TargetCall --> Settlement |
|
|
||
| ### Add or replace delegation | ||
|
|
||
| After deployment, the solver EOA must authorize the delegate address. |
There was a problem hiding this comment.
should we mention, a EAO can only have one delegation (as per https://eips.ethereum.org/EIPS/eip-7702).
There was a problem hiding this comment.
I think that's kind of implied
There was a problem hiding this comment.
It's a nit, its implicit because we assume people really understand the details of ERC-7702. Because the title is "Add or replace delegation" I thought it doesn't harm to be explicit about this allowing only one delegation to a contract. Alternativelly or in addition, you could replace the word "Add" by "Set"
There was a problem hiding this comment.
Also, there's some refeferences in the README about ERC-7702, I think is a nice detail to make them a link to the ERC-7702 in Ethereu.org. Lets the reader get the "implied" context easier. Specially, you name it in the first phrase in the README
|
Let's get merged this, solver are already integrating and should help. We can apply enhancements to make it more clear, as we see them fit based on the experience onboarding them. @harisang let us know if this is good enough or we should improve anything |
|
@igorroncevic btw, name the PR somehing less generic. It should be almost obvious what changes we expect by the title. Github will default also to use the name of the PR as the commit message, which makes it even more important. |
The PR is updating |
kaze-cow
left a comment
kaze-cow
left a comment
There was a problem hiding this comment.
Unblocking myself, other reviews seem adequate.
3 participants
Description
Adds a short project README that explains what
Solver7702Delegateis and how solver operators can use it.Context
Solver7702Delegatelets a solver keep its existing allowlisted solver EOA while auxiliary EOAs provide extra submission nonce lanes through ERC-7702. The README gives GitHub readers the project context and points them to the fuller docs guide.Out of Scope
This PR only updates README documentation. It does not change contract code, deployment scripts, tests, or driver behavior.
Testing Instructions
Review the README and check that the documented commands and flows match the current repository tooling.