feat(solver): check target before accept #2478
Conversation
kevinhalliday
requested review from
corverroos,
sideninja,
chmllr and
kc1116
as code owners
|
|
||
| func (t Targets) ForCall(call bindings.SolveCall) (Target, bool) { | ||
| for _, target := range t { | ||
| if target.IsAllowedCall(call) { |
There was a problem hiding this comment.
I think we should error if multiple targets are matched?
There was a problem hiding this comment.
mmm, should we use the inbox or outbox contract address to identify the target? Picking any target that matches feels a bit loose?
There was a problem hiding this comment.
non matching calls will also be hard to debug since we can't see what rules are expected and why they are failing
There was a problem hiding this comment.
I think we should error if multiple targets are matched?
Yeah would help us catch bugs / bad target config.
mmm, should we use the inbox or outbox contract address to identify the target? Picking any target that matches feels a bit loose?
We will still be executing through the outbox. And there is a difference between outbox allowing or not allowing a call (which it does). And a target saying "yes this is an allowed call".
We can also add an outbox check.
non matching calls will also be hard to debug since we can't see what rules are expected and why they are failing
Yeah good point we do need to consider debugging.
"is this call one we support?" (target.IsAllowedCall)
Here we filter out chains / contracts / functions selectors we don't support. Here I think we can
Request for unsupported call dest_chain="holeskey" target_contract="0x..." target_selector="0x..."
then "should I make this supported call?" (target.Verify)
Here we can log the error returned by verify
Request rejected for target reason="insufficient deposits" target="symbiotic" dest_chain="holeskey" ...
kevinhalliday
force-pushed
the
kh/solving
branch
from
0e7c942 to
d4f7433
Compare
Only accept if know target, and call / deposits are verified.
issue: none