[WIP] batch transactions #82
Conversation
| ) external nonReentrant whenNotPaused returns (uint256) { | ||
| address msgSender = msg.sender; | ||
| emit log_named_address("msg.sender", msgSender); | ||
| if (msg.sender == execAddr) { |
There was a problem hiding this comment.
this should be a modifier, that way we can always just use the msgSender variable. Or an alternative would be a library or internal function somehow, and we chang all other usages to msgSender (which falls back to msg.Sender if not the trusted proxy address)
| } | ||
| } | ||
|
|
||
| contract BatchUtil { |
There was a problem hiding this comment.
can be a library? perhaps should also be in a different file?
| emit log_named_address("msg.sender", msgSender); | ||
| if (msg.sender == execAddr) { | ||
| msgSender = unpackTrailingParamMsgSender(); | ||
| emit log_named_address("msgSender", msgSender); |
There was a problem hiding this comment.
should cut these logs?
| function setUpFullProtocol() public { | ||
| exec = new Exec(); | ||
| weth = new WETH(); | ||
| protocol = new HookProtocol( |
There was a problem hiding this comment.
does it make sense to have exec live on HookProtocol and then just make the HookProtocol address that all of these contracts already store the exec address?
| /// @dev This contract is intended to be an implementation referenced by a proxy | ||
| contract HookCoveredCallImplV1 is | ||
| IHookCoveredCall, | ||
| HookInstrumentERC721, |
There was a problem hiding this comment.
are there functions within here, like on the OZ contracts, that pay attention to msg.Sender... do we need to update them?
There was a problem hiding this comment.
It seems likely that we want to allow only a fixed set of external functions call with msg.sender, but perhaps there are some like safeTransferFrom where that is not the case?
No description provided.