Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

Commit

Permalink
Trades localStorage fixes (#1117)
Browse files Browse the repository at this point in the history 
* Storing list of trades instead of maps to avoid JSON serialization issues

* Filter out potentially null trades

* Updated the way trades are serialized/deserialized

Also, now keeping track of trade ids for easy duplicate detection

* Finally some error handling...

Co-authored-by: Leandro Boscariol <leandro.boscariol@gnosis.io>
  • Loading branch information
@alfetopito
alfetopito and Leandro Boscariol authored Jun 24, 2020
1 parent 59de355 commit 9c59ef7
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 60 deletions.
7 changes: 4 additions & 3 deletions src/components/TradesWidget/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ const Trades: React.FC = () => {
const { networkId, userAddress, isConnected } = useWalletConnection()
const trades = useTrades()

const filteredTrades = useMemo(() => trades.filter(trade => isTradeSettled(trade) && !isTradeReverted(trade)), [
trades,
])
const filteredTrades = useMemo(
() => trades.filter(trade => trade && isTradeSettled(trade) && !isTradeReverted(trade)),
[trades],
)

const generateCsv = useCallback(
() =>
Expand Down
60 changes: 32 additions & 28 deletions src/hooks/useTrades.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,39 @@ export function useTrades(): Trade[] {

async function updateTrades(): Promise<void> {
if (userAddress && networkId) {
// Don't want to update on every block
// So instead, we get the latest block when the time comes
const toBlock = await web3.eth.getBlockNumber()
const params = {
userAddress,
networkId,
// fromBlock is inclusive. If set, add 1 to avoid duplicates, otherwise return undefined
fromBlock: !lastCheckedBlock ? lastCheckedBlock : lastCheckedBlock + 1,
toBlock,
orders,
try {
// Don't want to update on every block
// So instead, we get the latest block when the time comes
const toBlock = await web3.eth.getBlockNumber()
const params = {
userAddress,
networkId,
// fromBlock is inclusive. If set, add 1 to avoid duplicates, otherwise return undefined
fromBlock: !lastCheckedBlock ? lastCheckedBlock : lastCheckedBlock + 1,
toBlock,
orders,
}

// Check before expensive operation
if (cancelled) {
return
}

const { trades: newTrades, reverts } = await getTradesAndTradeReversions(params)

// Check before updating state
if (cancelled) {
return
}

dispatch(
newTrades.length > 0 || reverts.length > 0
? appendTrades({ lastCheckedBlock: toBlock, networkId, userAddress, trades: newTrades, reverts })
: updateLastCheckedBlock({ lastCheckedBlock: toBlock, networkId, userAddress }),
)
} catch (e) {
console.error(`Failed to update trades`, e)
}

// Check before expensive operation
if (cancelled) {
return
}

const { trades: newTrades, reverts } = await getTradesAndTradeReversions(params)

// Check before updating state
if (cancelled) {
return
}

dispatch(
newTrades.length > 0 || reverts.length > 0
? appendTrades({ lastCheckedBlock: toBlock, networkId, userAddress, trades: newTrades, reverts })
: updateLastCheckedBlock({ lastCheckedBlock: toBlock, networkId, userAddress }),
)
}
}

Expand Down
119 changes: 90 additions & 29 deletions src/reducers-actions/trades.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,28 @@ import { Trade, TradeReversion, EventWithBlockInfo } from 'api/exchange/Exchange

import { Actions } from 'reducers-actions'

import { logDebug, flattenMapOfLists, dateToBatchId, toBN, setStorageItem } from 'utils'
import { logDebug, dateToBatchId, toBN, setStorageItem, flattenMapOfLists } from 'utils'
import { TRADES_LOCAL_STORAGE_KEY } from 'const'

// ******** TYPES/INTERFACES

export type ActionTypes = 'OVERWRITE_TRADES' | 'APPEND_TRADES' | 'UPDATE_BLOCK'

export type TradesState = Record<string, TradesStatePerAccount>
export type SerializableTradesState = Record<string, SerializableTradesStatePerAccount>

interface TradesStatePerAccount {
trades: Trade[]
tradeIds: Set<string>
pendingTrades: Map<string, Trade[]>
lastCheckedBlock?: number
}

interface SerializableTradesStatePerAccount extends Omit<TradesStatePerAccount, 'tradeIds' | 'pendingTrades'> {
tradeIds: string[]
pendingTrades: [string, Trade[]][]
}

interface WithReverts {
reverts: TradeReversion[]
}
Expand All @@ -32,11 +39,11 @@ interface WithAccountInfo {

type OverwriteTradesActionType = Actions<
'OVERWRITE_TRADES',
Omit<TradesStatePerAccount, 'pendingTrades'> & WithReverts & WithAccountInfo
Pick<TradesStatePerAccount, 'trades' | 'lastCheckedBlock'> & WithReverts & WithAccountInfo
>
type AppendTradesActionType = Actions<
'APPEND_TRADES',
Required<Omit<TradesStatePerAccount, 'pendingTrades'>> & WithReverts & WithAccountInfo
Required<Pick<TradesStatePerAccount, 'trades' | 'lastCheckedBlock'>> & WithReverts & WithAccountInfo
>
type UpdateBlockActionType = Actions<
'UPDATE_BLOCK',
Expand Down Expand Up @@ -79,29 +86,34 @@ function buildTradeRevertKey(batchId: number, orderId: string): string {

// ******** HELPERS

function groupByRevertKey<T extends EventWithBlockInfo>(list: T[], initial?: Map<string, T[]>): Map<string, T[]> {
const map = initial || new Map<string, T[]>()
const seenIds = new Set<string>()
function groupByRevertKey<T extends EventWithBlockInfo>(
list: T[],
initial?: Map<string, T[]>,
seenTradeIds?: Set<string>,
): { group: Map<string, T[]>; seenIds: Set<string> } {
const group = initial || new Map<string, T[]>()
const seenIds = seenTradeIds || new Set<string>()
const newIds = new Set<string>()

list.forEach(item => {
// Avoid duplicate entries
if (seenIds.has(item.id)) {
if (seenIds.has(item.id) || newIds.has(item.id)) {
return
}
seenIds.add(item.id)
newIds.add(item.id)

const revertKey = buildTradeRevertKey(item.batchId, item.orderId)

if (map.has(revertKey)) {
const subList = map.get(revertKey) as T[]
if (group.has(revertKey)) {
const tradesList = group.get(revertKey) as T[]

subList.push(item)
tradesList.push(item)
} else {
map.set(revertKey, [item])
group.set(revertKey, [item])
}
})

return map
return { group, seenIds: newIds }
}

function sortByTimeAndPosition(a: EventWithBlockInfo, b: EventWithBlockInfo): number {
Expand All @@ -121,7 +133,7 @@ function getPendingTrades(tradesByRevertKey: Map<string, Trade[]>): Map<string,

// Filter out trades in that range (curr ... curr -2).
// The `revertKey` is composed by batchId|orderId, so this regex looks for the batchIds in the keys
const batchesRegex = new RegExp(`^(${currentBatchId}|${currentBatchId - 1}|${currentBatchId - 2})\|`)
const batchesRegex = new RegExp(`^(${currentBatchId}|${currentBatchId - 1}|${currentBatchId - 2})\\\|`)

const pending = new Map<string, Trade[]>()

Expand All @@ -138,10 +150,11 @@ function applyRevertsToTrades(
trades: Trade[],
reverts: TradeReversion[],
pendingTrades?: Map<string, Trade[]>,
): [Trade[], Map<string, Trade[]>] {
seenIds?: Set<string>,
): { trades: Trade[]; pendingTrades: Map<string, Trade[]>; seenIds: Set<string> } {
// Group trades by revertKey
const tradesByRevertKey = groupByRevertKey(trades, pendingTrades)
const revertsByRevertKey = groupByRevertKey(reverts)
const { group: tradesByRevertKey, seenIds: newTradeIds } = groupByRevertKey(trades, pendingTrades, seenIds)
const { group: revertsByRevertKey } = groupByRevertKey(reverts)

// Assumptions:
// 1. There can be more than one trade per batch for a given order (even if there are no reverts)
Expand All @@ -150,8 +163,8 @@ function applyRevertsToTrades(
// 3. Every revert matches 1 trade
// 4. Reverts match Trades by order or appearance (first Revert matches first Trade and so on)

revertsByRevertKey.forEach((reverts, revertKey) => {
reverts.sort(sortByTimeAndPosition)
revertsByRevertKey.forEach((revertsList, revertKey) => {
const reverts = revertsList.sort(sortByTimeAndPosition)
const trades = tradesByRevertKey.get(revertKey)?.sort(sortByTimeAndPosition)

if (trades) {
Expand Down Expand Up @@ -190,12 +203,28 @@ function applyRevertsToTrades(
}
})

return [flattenMapOfLists(tradesByRevertKey), getPendingTrades(tradesByRevertKey)]
// Transform groups into a single list
const newTrades = flattenMapOfLists(tradesByRevertKey)
// Remove old trades that were used only to help matching possible reverts to new trades
.filter(trade => trade && newTradeIds.has(trade.id))

// Merge existing and new set of ids
seenIds?.forEach(id => id && newTradeIds.add(id))

return {
trades: newTrades,
pendingTrades: getPendingTrades(tradesByRevertKey),
seenIds: newTradeIds,
}
}

// ******** INITIAL STATE / LOCAL STORAGE

const INITIAL_TRADES_STATE_SINGLE_NETWORK = { trades: [], pendingTrades: new Map<string, Trade[]>() }
const INITIAL_TRADES_STATE_SINGLE_ACCOUNT = {
trades: [],
pendingTrades: new Map<string, Trade[]>(),
tradeIds: new Set<string>(),
}

/**
* Custom json parser for BN and BigNumber values.
Expand All @@ -221,14 +250,42 @@ function reviver(key: string, value: unknown): unknown {
return value
}

function serialize(state: TradesState): SerializableTradesState {
const serialized = {}

Object.keys(state).forEach(accountKey => {
serialized[accountKey] = { ...state[accountKey], tradeIds: [], pendingTrades: [] }
state[accountKey].tradeIds?.forEach(id => serialized[accountKey].tradeIds.push(id))
state[accountKey].pendingTrades?.forEach((value, key) => serialized[accountKey].pendingTrades.push([key, value]))
})

return serialized
}

function deserialize(state: SerializableTradesState): TradesState {
const parsedState = {}

Object.keys(state).forEach(accountKey => {
parsedState[accountKey] = {
...state[accountKey],
tradeIds: new Set<string>(),
pendingTrades: new Map<string, Trade[]>(),
}
state[accountKey].tradeIds?.forEach(id => parsedState[accountKey].tradeIds.add(id))
state[accountKey].pendingTrades?.forEach(tuple => parsedState[accountKey].pendingTrades.set(...tuple))
})

return parsedState
}

function loadInitialState(): TradesState {
let state = {}

const localStorageOrders = localStorage.getItem(TRADES_LOCAL_STORAGE_KEY)

if (localStorageOrders) {
try {
state = JSON.parse(localStorageOrders, reviver)
state = deserialize(JSON.parse(localStorageOrders, reviver))
} catch (e) {
logDebug(`[reducer:trades] Failed to load localStorage`, e.msg)
}
Expand All @@ -248,21 +305,25 @@ export const reducer = (state: TradesState, action: ReducerActionType): TradesSt

const accountKey = buildAccountKey({ networkId, userAddress })

const { trades: currTrades, pendingTrades: currPendingTrades } =
state[accountKey] || INITIAL_TRADES_STATE_SINGLE_NETWORK
const { trades: currTrades, pendingTrades: currPendingTrades, tradeIds } =
state[accountKey] || INITIAL_TRADES_STATE_SINGLE_ACCOUNT

const [trades, pendingTrades] = applyRevertsToTrades(newTrades, reverts, currPendingTrades)
const { trades, pendingTrades, seenIds } = applyRevertsToTrades(newTrades, reverts, currPendingTrades, tradeIds)

return { ...state, [accountKey]: { trades: currTrades.concat(trades), lastCheckedBlock, pendingTrades } }
return {
...state,
[accountKey]: { trades: currTrades.concat(trades), lastCheckedBlock, pendingTrades, tradeIds: seenIds },
}
}
case 'OVERWRITE_TRADES': {
const { trades: newTrades, reverts, lastCheckedBlock, networkId, userAddress } = action.payload

const accountKey = buildAccountKey({ networkId, userAddress })
const tradeIds = new Set<string>()

const [trades, pendingTrades] = applyRevertsToTrades(newTrades, reverts)
const { trades, pendingTrades, seenIds } = applyRevertsToTrades(newTrades, reverts, undefined, tradeIds)

return { ...state, [accountKey]: { trades, lastCheckedBlock, pendingTrades } }
return { ...state, [accountKey]: { trades, lastCheckedBlock, pendingTrades, tradeIds: seenIds } }
}
case 'UPDATE_BLOCK': {
const { networkId, lastCheckedBlock } = action.payload
Expand All @@ -282,6 +343,6 @@ export async function sideEffect(state: TradesState, action: ReducerActionType):
case 'APPEND_TRADES':
case 'OVERWRITE_TRADES':
case 'UPDATE_BLOCK':
setStorageItem(TRADES_LOCAL_STORAGE_KEY, state)
setStorageItem(TRADES_LOCAL_STORAGE_KEY, serialize(state))
}
}
4 changes: 4 additions & 0 deletions src/utils/miscellaneous.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,7 @@ export async function retry<T extends () => any>(fn: T, options?: RetryOptions):
export function flattenMapOfLists<K, T>(map: Map<K, T[]>): T[] {
return Array.from(map.values()).reduce<T[]>((acc, list) => acc.concat(list), [])
}

export function flattenMapOfSets<K, T>(map: Map<K, Set<T>>): T[] {
return Array.from(map.values()).reduce<T[]>((acc, set) => acc.concat(Array.from(set)), [])
}

0 comments on commit 9c59ef7

Please sign in to comment.