Pass vars to delegate call #20
Conversation
|
We could pass the entire payload when using this strategy. This gives us most of the above (+ As per my comment in #19 I think this would be better as a new flag. |
src/modules/Calls.sol
Outdated
| (success) = LibOptim.delegatecall( | ||
| call.to, | ||
| gasLimit == 0 ? gasleft() : gasLimit, | ||
| abi.encode(_opHash, _startingGas, i, numCalls, _decoded.space, call.data) |
There was a problem hiding this comment.
Can we reverse this so the encoded values overflow the standard encoding of a call.data call? Then "extensions" that don't need the data can ignore it without needing to know our special encoding.
abi.encodePacked(call.data, extraData)
We can provide a library for those wanting to extract the additional values.
There was a problem hiding this comment.
I think this is more risky, I worry about undefined behavior when something "extra" may be added to a normal call that may have weird side-effects on the module, if the module didn't account for it; trying to make a "sane default" seems tricky, because we don't want people randomly delegating to addresses hoping for the resulting behavior to be correct, we want explicit intent and behavior.
If you want to delegate to a very very very specific address, and that address expects just call.data (this is a very weird scenario) then you could delegate to a proxy that strips all the extra data and re-delegates.
I can see reverting the order on abi.encode, because it allows you to safely decode call.data while ignoring everything else, (I mean you can still do it in assembly if you know where call.data is).
I guess you want to maintain compatibility with solidity's function selector? I can see that, but I don't really see it being that useful in this context.
There was a problem hiding this comment.
My concern is about introducing custom encoding to a standard operation. We are no longer just delegating the call in the payload, we are reencoding the provided call data. We wouldn't add this to a normal call and I don't think we should add it for delegate call either.
I support of a new flag so we can be explicit about when the data is being modified. Then we can keep delegateCall as a standard operation.
There was a problem hiding this comment.
We don't really have room for another flag, unless we drop some other functionality (or add another byte).
call and delegateCall means two different things, with two very distinct uses, which usage of delegateCall do you see would clash with this feature? can you think of any concrete example?
|
feels ugly and hacky..? maybe we just skip this "native fee tools" thing entirely in v3..? personally as it relates to #18 .. i think option 1 is the cleanest, but I understand we dont want to do the rework right now, so we can just punt this to v4 ..? ill leave for you guys to decide.. but the above feels gnarly and weird.. I hate the ideas of "extend to do {some random undefined generic thing}" .. if this is to use for native fees, then it should be written explicitly. I understand we dont want to rework our encoding functions now, but if thats the only reason, doesnt feel like a good reason.. |
|
It is not the only reason, any native fee capabilities has to work for L1 and for all L2s (and sidechains, etc etc.) Many L2s already have gas pricing that does not work like L1, so trying to make a "one size fits all" is almost impossible. I think having extensions handle this is a great solution, because different extensions can implement fees that adapt to their respective L2s |
This is an alternative to #19, and the necessary scaffolding needed to implement proper fee refunds.
The idea is that we pass additional information when
delegateCall: true, information that may be either be hard (or impossible) to compute from within the "implementation contract".opHash: Can't be included oncalldatabecauseopHashdepends on the calldata, circular dependencystartingGas: Not directly observable, unless an special entrypoint (and transient storage) is usedi: Could be passed, but couldn't be verified.numCalls: Could be passed, but couldn't be verifiedspace: Could be passed, but couldn't be verified (there is no record of which nonce incremented last)Any other ideas of things that we may want to pass on
delegateCall?