Skip to content

Conversation

@eyqs
Copy link
Contributor

@eyqs eyqs commented Oct 10, 2024

The DepositRequest and RedeemRequest events were mistyped as RequestDeposit and RequestRedeem.

Also adds minor punctuation fixes.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Oct 10, 2024

File ERCS/erc-7540.md

Requires 2 more reviewers from @axic, @g11tech, @gcolvin, @lightclient, @xinbenlv

Copy link
Contributor

@0xTimepunk 0xTimepunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@peculiarity
Copy link

peculiarity commented Oct 10, 2024

Implementations MUST support an additional overloaded deposit and mint method on the specification from ERC-4626, with an additional controller input of type address:

When the Deposit event is emitted, the first parameter MUST be the controller, and the second parameter MUST be the receiver.

IMO that Deposit event is new and is part of the standard therefore it must be added under the Events section with the correct signature.

@eyqs
Copy link
Contributor Author

eyqs commented Oct 10, 2024

I think the Deposit event remains the same with the same parameters and parameter names. This EIP just specifies that that the implementation should emit Deposit(controller, receiver, assets, shares) - i.e. the sender is always the controller and the owner is always the receiver.

Copy link
Contributor

@vikramarun vikramarun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@SamWilsn SamWilsn merged commit c0c9eb3 into ethereum:master Dec 3, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants