Skip to content
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(sdk): rename hook and update pckg vers #3216

Merged
merged 2 commits into from
Feb 27, 2025
Merged

Conversation

ga-reth
Copy link
Contributor

@ga-reth ga-reth commented Feb 27, 2025

  • rename hook
  • match version in package.json to published

issue: none

@ga-reth ga-reth merged commit 11b1683 into main Feb 27, 2025
19 checks passed
@ga-reth ga-reth deleted the gv/sdk-rename-hook branch February 27, 2025 17:38
Comment on lines +54 to 57
const { orderId, originData } = useGetOrderData({
status: wait.status,
logs: wait.data?.logs,
})
Copy link
Contributor

@kevinhalliday kevinhalliday Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OrderData has some meaning in ERC7683 already. it's the order we encode to open the order. and is not actually what we are getting here.

here we are parsing orderId and fillOriginData from an Open event.

Thoughts on this?

// return full resolved order, rather then first element of fillInstructions (which we call fillOrginData)
cosnt { 
    orderId,    // bytes32 orderid 
    resolved,  // ERC7683.ResolvedCrossChainOrder
 } = useParseOpen({ status, logs })
 
 // then pass full resolved order into did fill
const { filled } =  useDidFill({ orderId, resolved })

// then in did fill we can parse out the info from resolved we need 
const fillOriginData = resolved.fillInstructions[0].originData

That way we interfaces to useDidFill and useParseOpen don't need to change if we add more / change resolved fill instructions in contracts

other options for useParseOpen:

useParseResolved
useOrderOpen
useParseOpenEvent
useParseOrderOpen

something that says "we are taking logs from an open tx and getting order id / resolved order"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, it's specifically parsing the open event so I think some flavour of that sounds ideal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants