-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(contracts/solve): refactor order state #2719
Conversation
view | ||
returns (ResolvedCrossChainOrder memory order, OrderState memory state, StatusUpdate[] memory history) | ||
{ | ||
return (_orders[id], _orderState[id], _orderHistory[id]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Struggled thinking of a more appropraite "wrapping" struct name. Everything felt forced. I actually think SolvertNetOrder
is decent, in that a SolverNetOrder
wraps a ResolvedCrossChainOrder
But didn't love that we us overloading the use of "order". And we just needed this to name a return type. Tuple should suffice, as this is view is mostly for off chain and easy to unpack.
Less terms == better
/** | ||
* @dev Resolve the onchain order. | ||
*/ | ||
function resolve(OnchainCrossChainOrder calldata order) external view returns (ResolvedCrossChainOrder memory); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this as it was in ERC7683
intercace, which we inherit
/** | ||
* @notice Returns the order parameters for the given order ID. | ||
*/ | ||
function getOrderParams(bytes32 id) external view returns (SolverNetOrderParams memory); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed view,s getOrder
serves everything. can add back if we feel a need for it
Refactor order state types / logic.
SolverNet
prefix to typenamesgetOrder
, instead of forcing name on wrapped structissue: none