Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

1334/only update connected wallet #1684

Merged
merged 5 commits into from
Nov 8, 2021
Merged

1334/only update connected wallet #1684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d2fa1a
Only update cancelled orders for connected wallet
Oct 21, 2021
cae8cd0
Only check for unfillable orders for connected wallet
Oct 21, 2021
c6cc3eb
Only check pending orders for connected wallet
Oct 21, 2021
bf70822
Always create a new state object when adding an order (#1686)
alfetopito Oct 25, 2021
68b0b2e
Refactor: comparing account with order.owner lowercased for safety
Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 28 additions & 12 deletions src/custom/state/orders/reducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ function deleteOrderById(state: Required<OrdersState>, chainId: ChainId, id: str
delete stateForChain.cancelled[id]
}

function addOrderToState(
state: Required<OrdersState>,
chainId: ChainId,
id: string,
status: OrderTypeKeys,
order: SerializedOrder
): void {
// Attempt to fix `TypeError: Cannot add property <x>, object is not extensible`
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you got the idea from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cant_define_property_object_not_extensible

..but, I don't really get it, one reason we use reduxjs/toolkit it's because we can mutate fearlessly the state. Who do you suggest is using preventExtensions?

We have similar logics in all other reducers. Should we be consider with the others? why this is happening now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not completely sure what was causing it, but this seemed to be the place where issues Mareen was facing came from.
For more context check the PR where this was added #1686
I was not able to reproduce it, but as you can see from the from PR, it seemed that the problem was trying to edit a readonly object.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷

// seen on https://user-images.githubusercontent.com/34510341/138450105-bb94a2d1-656e-4e15-ae99-df9fb33c8ca4.png
// by creating a new object instead of trying to edit the existing one
// Seems to be due to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/preventExtensions
// but only happened on Chrome
state[chainId][status] = { ...state[chainId][status], [id]: { order, id } }
}

function popOrder(state: OrdersState, chainId: ChainId, status: OrderStatus, id: string): OrderObject | undefined {
const orderObj = state?.[chainId]?.[status]?.[id]
if (orderObj) {
Expand All @@ -140,12 +155,12 @@ export default createReducer(initialState, (builder) =>
const { order, id, chainId } = action.payload

const orderStateList = order.status === OrderStatus.PRESIGNATURE_PENDING ? 'presignaturePending' : 'pending'
state[chainId][orderStateList][id] = { order, id }

addOrderToState(state, chainId, id, orderStateList, order)
})
.addCase(preSignOrders, (state, action) => {
prefillState(state, action)
const { ids, chainId } = action.payload
const pendingOrders = state[chainId].pending

ids.forEach((id) => {
const orderObject = getOrderById(state, chainId, id)
Expand All @@ -154,7 +169,8 @@ export default createReducer(initialState, (builder) =>
deleteOrderById(state, chainId, id)

orderObject.order.status = OrderStatus.PENDING
pendingOrders[id] = orderObject

addOrderToState(state, chainId, id, 'pending', orderObject.order)
}
})
})
Expand Down Expand Up @@ -195,7 +211,7 @@ export default createReducer(initialState, (builder) =>
: newOrder

// add order to respective state
state[chainId][status][id] = { order, id }
addOrderToState(state, chainId, id, status, order)
})
})
.addCase(fulfillOrder, (state, action) => {
Expand All @@ -212,7 +228,7 @@ export default createReducer(initialState, (builder) =>
orderObject.order.fulfilledTransactionHash = transactionHash
orderObject.order.isCancelling = false

state[chainId].fulfilled[id] = orderObject
addOrderToState(state, chainId, id, 'fulfilled', orderObject.order)
}
})
.addCase(fulfillOrdersBatch, (state, action) => {
Expand All @@ -235,7 +251,7 @@ export default createReducer(initialState, (builder) =>

orderObject.order.apiAdditionalInfo = apiAdditionalInfo

state[chainId].fulfilled[id] = orderObject
addOrderToState(state, chainId, id, 'fulfilled', orderObject.order)
}
})
})
Expand All @@ -251,13 +267,12 @@ export default createReducer(initialState, (builder) =>
orderObject.order.status = OrderStatus.EXPIRED
orderObject.order.isCancelling = false

state[chainId].expired[id] = orderObject
addOrderToState(state, chainId, id, 'expired', orderObject.order)
}
})
.addCase(expireOrdersBatch, (state, action) => {
prefillState(state, action)
const { ids, chainId } = action.payload
const fulfilledOrders = state[chainId].expired

// if there are any newly fulfilled orders
// update them
Expand All @@ -269,7 +284,8 @@ export default createReducer(initialState, (builder) =>

orderObject.order.status = OrderStatus.EXPIRED
orderObject.order.isCancelling = false
fulfilledOrders[id] = orderObject

addOrderToState(state, chainId, id, 'expired', orderObject.order)
}
})
})
Expand All @@ -295,13 +311,12 @@ export default createReducer(initialState, (builder) =>
orderObject.order.status = OrderStatus.CANCELLED
orderObject.order.isCancelling = false

state[chainId].cancelled[id] = orderObject
addOrderToState(state, chainId, id, 'cancelled', orderObject.order)
}
})
.addCase(cancelOrdersBatch, (state, action) => {
prefillState(state, action)
const { ids, chainId } = action.payload
const cancelledOrders = state[chainId].cancelled

ids.forEach((id) => {
const orderObject = getOrderById(state, chainId, id)
Expand All @@ -311,7 +326,8 @@ export default createReducer(initialState, (builder) =>

orderObject.order.status = OrderStatus.CANCELLED
orderObject.order.isCancelling = false
cancelledOrders[id] = orderObject

addOrderToState(state, chainId, id, 'cancelled', orderObject.order)
}
})
})
Expand Down
19 changes: 12 additions & 7 deletions src/custom/state/orders/updaters/CancelledOrdersUpdater.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { fetchOrderPopupData, OrderLogPopupMixData } from 'state/orders/updaters
* period and say it's cancelled even though in some cases it might actually be filled.
*/
export function CancelledOrdersUpdater(): null {
const { chainId } = useActiveWeb3React()
const { chainId, account } = useActiveWeb3React()

const cancelled = useCancelledOrders({ chainId })

Expand All @@ -38,10 +38,15 @@ export function CancelledOrdersUpdater(): null {
const fulfillOrdersBatch = useFulfillOrdersBatch()

const updateOrders = useCallback(
async (chainId: ChainId) => {
// Filter orders created in the last 5 min, no further
async (chainId: ChainId, account: string) => {
const lowerCaseAccount = account.toLowerCase()
// Filter orders:
// - Owned by the current connected account
// - Created in the last 5 min, no further
const pending = cancelledRef.current.filter(
(order) => Date.now() - new Date(order.creationTime).getTime() < CANCELLED_ORDERS_PENDING_TIME
({ owner, creationTime }) =>
owner.toLowerCase() === lowerCaseAccount &&
Date.now() - new Date(creationTime).getTime() < CANCELLED_ORDERS_PENDING_TIME
)

if (pending.length === 0) {
Expand Down Expand Up @@ -74,14 +79,14 @@ export function CancelledOrdersUpdater(): null {
)

useEffect(() => {
if (!chainId) {
if (!chainId || !account) {
return
}

const interval = setInterval(() => updateOrders(chainId), OPERATOR_API_POLL_INTERVAL)
const interval = setInterval(() => updateOrders(chainId, account), OPERATOR_API_POLL_INTERVAL)

return () => clearInterval(interval)
}, [chainId, updateOrders])
}, [account, chainId, updateOrders])

return null
}
20 changes: 12 additions & 8 deletions src/custom/state/orders/updaters/PendingOrdersUpdater.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function _getNewlyPreSignedOrders(allPendingOrders: Order[], signedOrdersIds: Or
}

export function PendingOrdersUpdater(): null {
const { chainId } = useActiveWeb3React()
const { chainId, account } = useActiveWeb3React()

const pending = usePendingOrders({ chainId })

Expand All @@ -50,15 +50,19 @@ export function PendingOrdersUpdater(): null {
const presignOrders = usePresignOrders()

const updateOrders = useCallback(
async (chainId: ChainId) => {
async (chainId: ChainId, account: string) => {
const lowerCaseAccount = account.toLowerCase()
// Only check pending orders of current connected account
const pending = pendingRef.current.filter(({ owner }) => owner.toLowerCase() === lowerCaseAccount)

// Exit early when there are no pending orders
if (pendingRef.current.length === 0) {
if (pending.length === 0) {
return
}

// Iterate over pending orders fetching API data
const unfilteredOrdersData = await Promise.all(
pendingRef.current.map(async (orderFromStore) => fetchOrderPopupData(orderFromStore, chainId))
pending.map(async (orderFromStore) => fetchOrderPopupData(orderFromStore, chainId))
)

// Group resolved promises by status
Expand All @@ -76,7 +80,7 @@ export function PendingOrdersUpdater(): null {
if (presigned.length > 0) {
// Only mark as presigned the orders we were not aware of their new state
const presignedOrderIds = presigned as OrderID[]
const ordersPresignaturePendingSigned = _getNewlyPreSignedOrders(pendingRef.current, presignedOrderIds)
const ordersPresignaturePendingSigned = _getNewlyPreSignedOrders(pending, presignedOrderIds)

if (ordersPresignaturePendingSigned.length > 0) {
presignOrders({
Expand Down Expand Up @@ -111,14 +115,14 @@ export function PendingOrdersUpdater(): null {
)

useEffect(() => {
if (!chainId) {
if (!chainId || !account) {
return
}

const interval = setInterval(() => updateOrders(chainId), OPERATOR_API_POLL_INTERVAL)
const interval = setInterval(() => updateOrders(chainId, account), OPERATOR_API_POLL_INTERVAL)

return () => clearInterval(interval)
}, [chainId, updateOrders])
}, [account, chainId, updateOrders])

return null
}
16 changes: 12 additions & 4 deletions src/custom/state/orders/updaters/UnfillableOrdersUpdater.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async function _getOrderPrice(chainId: ChainId, order: Order) {
* Updater that checks whether pending orders are still "fillable"
*/
export function UnfillableOrdersUpdater(): null {
const { chainId } = useActiveWeb3React()
const { chainId, account } = useActiveWeb3React()
const pending = usePendingOrders({ chainId })
const setIsOrderUnfillable = useSetIsOrderUnfillable()

Expand All @@ -70,11 +70,19 @@ export function UnfillableOrdersUpdater(): null {
)

const updatePending = useCallback(() => {
if (!chainId || pendingRef.current.length === 0) {
if (!chainId || !account) {
return
}

pendingRef.current.forEach((order, index) =>
const lowerCaseAccount = account.toLowerCase()
// Only check pending orders of the connected account
const pending = pendingRef.current.filter(({ owner }) => owner.toLowerCase() === lowerCaseAccount)

if (pending.length === 0) {
return
}

pending.forEach((order, index) =>
_getOrderPrice(chainId, order).then((price) => {
console.debug(
`[UnfillableOrdersUpdater::updateUnfillable] did we get any price? ${order.id.slice(0, 8)}|${index}`,
Expand All @@ -83,7 +91,7 @@ export function UnfillableOrdersUpdater(): null {
price?.amount && updateIsUnfillableFlag(chainId, order, price)
})
)
}, [chainId, updateIsUnfillableFlag])
}, [account, chainId, updateIsUnfillableFlag])

useEffect(() => {
updatePending()
Expand Down