fix(permit): typefy hooks and allow only 1 permit #4726
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| : mergedHooks | ||
|
|
||
| console.log(`bug:updateHooksOnAppData`, noDuplicateHooks) | ||
| const filteredHooks = filterHooks(doc.metadata.hooks, preHooksFilter, postHooksFilter) |
There was a problem hiding this comment.
We shouldn't merge it here, because at this point we can't tell which hook is what
There was a problem hiding this comment.
Although this might have broken the hook store logic. Will need to address that, but probably in a different PR.
There was a problem hiding this comment.
Fixed the merging logic
| const preInteractionHooks = preHooks.map<TypedCowHook>((hookDetails) => | ||
| cowHookToTypedCowHook(hookDetails.hook, 'hookStore') | ||
| ) | ||
| const postInteractionHooks = postHooks.map<TypedCowHook>((hookDetails) => | ||
| cowHookToTypedCowHook(hookDetails.hook, 'hookStore') | ||
| ) | ||
| const hooks = buildAppDataHooks<TypedCowHook[], TypedAppDataHooks>({ | ||
| preInteractionHooks: permitData | ||
| ? preInteractionHooks.concat([cowHookToTypedCowHook(permitData, 'permit')]) | ||
| : preInteractionHooks, |
There was a problem hiding this comment.
Whatever hooks are created in the hook store, type then accordingly for later.
| @@ -107,4 +107,4 @@ export const removeAppDataFromUploadQueueAtom = atom( | |||
| /** | |||
| * In memory atom for storing the current appData hooks info | |||
| */ | |||
| export const appDataHooksAtom = atom<AppDataHooks | undefined>(undefined) | |||
| export const appDataHooksAtom = atom<TypedAppDataHooks | undefined>(undefined) | |||
There was a problem hiding this comment.
Store typed hooks instead of plain ones.
|
|
||
| import { AppDataHooks, CowHook, TypedAppDataHooks, TypedCowHook } from '../types' | ||
|
|
||
| export function cowHookToTypedCowHook(hook: CowHook, type: TypedCowHook['type']): TypedCowHook { |
There was a problem hiding this comment.
Utils for converting back and forth between the different hook formats (typed and regular)
| return rest | ||
| } | ||
|
|
||
| export function addPermitHookToHooks(hooks: TypedAppDataHooks | undefined, permit: CowHook): AppDataHooks | undefined { |
There was a problem hiding this comment.
The "merger" or regular hooks with permit hooks.
Takes care of replacing whatever permit there was before, if any.
| const hooks = buildAppDataHooks({ | ||
| preInteractionHooks: [permitData], | ||
| }) | ||
| const hooks = addPermitHookToHooks(typedHooks, permitData) |
There was a problem hiding this comment.
Here is where the "merge" happens.
Permit is not added to the order anywhere else.
|
Hey @alfetopito , I've been able to sign a permit request in the wallet, but a LIMIT order placing has failed due to this error The next issue: I see that a trade hook is recorded into the next trade:
AR: a hook inside appdata is displayed https://dev.explorer.cow.fi/orders/0x41ba5826bdf78d434447cec347ea679ef724430f347cdc2de[…]a3c00a92ec5f96b1ad2527ab41b3932efeda58669776d9?tab=overview |
There was a problem hiding this comment.
Hey @alfetopito , great job, thank you for fixing this!
I have found only one issue, but it should not be a blocker for the current hotfix as it is related to the hooks store and can be addressed later. I opened #4727 task for this.
Summary
Follow up/complement to #4721 and #4719
Makes sure there is ever only 1 permit, and only set it if really needed.
To Test
See test steps on #4719 and #4721