Validation logic for Quark Builder #3
Conversation
src/builder/QuarkBuilder.sol
Outdated
| if (payment.chainIds.length != payment.maxCosts.length) { | ||
| revert InvalidInput(); | ||
| } |
There was a problem hiding this comment.
This will be obviated by refactoring the max costs data into an array-of-structs.
However, raises the question: if a chain id is not included in maxCosts, does that mean we should not consider assets on that chain? It is easily possible for the chainAccountsList to contain chains that have no specified max costs.
So, is that an error?
Or is that informative? Ie., if maxCosts is not empty, are those the only chains we should consider using?
There was a problem hiding this comment.
This will be obviated by refactoring the max costs data into an array-of-structs.
Yeah, this was removed after I added the MaxPaymentCost struct.
However, raises the question: if a chain id is not included in maxCosts, does that mean we should not consider assets on that chain? It is easily possible for the
chainAccountsListto contain chains that have no specified max costs.So, is that an error? Or is that informative? Ie., if maxCosts is not empty, are those the only chains we should consider using?
Hmm, interesting question. I don't think we should require the client to supply max costs for every chain. So, we could either 1) not construct Quark Operations on chains where max cost is unset or 2) choose some default value if max cost is unset.
I do prefer 1, since there really isn't a good default value that works for all Quark Operations and payment tokens for 2. It's also a bit scary to set a value for it that's not specified by the client.
There was a problem hiding this comment.
Agreed, I think if no max is provided, we should assume a max of 0.
There was a problem hiding this comment.
With the edge case that, if maxCosts is empty, we must still return a result that can be provided to simulate.
We may expect the client to explicitly pass in some large numbers for maxCosts, alternatively. 🤷
It's also perhaps a situation that will overlap with our handling of "maximum" or "unlimited" in the case of "transfer max" -- if we choose a sigil like uint256(-1) to represent "max" or "no limit," then maybe that's what we should use to explicitly construct simulations ignoring cost limits...
kevincheng96
force-pushed
the
kevin/quark-builder
branch
from
da7d9bc to
b04f11e
Compare
Fix some of that broke code
kevincheng96
force-pushed
the
kevin/validation
branch
from
e4ff01f to
649a734
Compare
had to rebase on main so that I had access to bridger code --------- Co-authored-by: Kevin Cheng <[email protected]>
No description provided.