fix(suite): deep-clone form values before assignment in reducer to prevent RHF bug

This commit is contained in:
Jan Komarek
2023-06-21 16:54:22 +02:00
committed by Matěj Kříž
parent e6fe6f1a3a
commit dc8de1075c
6 changed files with 30 additions and 76 deletions

View File

@@ -8,24 +8,14 @@ import type {
EthereumNetworkInfo,
MiscNetworkInfo,
} from '../types/coinInfo';
import { cloneObject } from '@trezor/utils';
const bitcoinNetworks: BitcoinNetworkInfo[] = [];
const ethereumNetworks: EthereumNetworkInfo[] = [];
const miscNetworks: MiscNetworkInfo[] = [];
// TODO: replace by structuredClone() after updating TS
export function cloneCoinInfo<T>(info: T): T {
const jsonString = JSON.stringify(info);
if (jsonString === undefined) {
// jsonString === undefined IF and only IF obj === undefined
// therefore no need to clone
return info;
}
return JSON.parse(jsonString);
}
export const getBitcoinNetwork = (pathOrName: number[] | string) => {
const networks = cloneCoinInfo(bitcoinNetworks);
const networks = cloneObject(bitcoinNetworks);
if (typeof pathOrName === 'string') {
const name = pathOrName.toLowerCase();
return networks.find(
@@ -40,7 +30,7 @@ export const getBitcoinNetwork = (pathOrName: number[] | string) => {
};
export const getEthereumNetwork = (pathOrName: number[] | string) => {
const networks = cloneCoinInfo(ethereumNetworks);
const networks = cloneObject(ethereumNetworks);
if (typeof pathOrName === 'string') {
const name = pathOrName.toLowerCase();
return networks.find(
@@ -52,7 +42,7 @@ export const getEthereumNetwork = (pathOrName: number[] | string) => {
};
export const getMiscNetwork = (pathOrName: number[] | string) => {
const networks = cloneCoinInfo(miscNetworks);
const networks = cloneObject(miscNetworks);
if (typeof pathOrName === 'string') {
const name = pathOrName.toLowerCase();
return networks.find(
@@ -95,7 +85,7 @@ export const getBech32Network = (coin: BitcoinNetworkInfo) => {
// fix coinInfo network values from path (segwit/legacy)
export const fixCoinInfoNetwork = (ci: BitcoinNetworkInfo, path: number[]) => {
const coinInfo = cloneCoinInfo(ci);
const coinInfo = cloneObject(ci);
if (path[0] === toHardened(84)) {
const bech32Network = getBech32Network(coinInfo);
if (bech32Network) {
@@ -131,7 +121,7 @@ const detectBtcVersion = (data: { subversion?: string }) => {
// TODO: https://github.com/trezor/trezor-suite/issues/4886
export const getCoinInfoByHash = (hash: string, networkInfo: any) => {
const networks = cloneCoinInfo(bitcoinNetworks);
const networks = cloneObject(bitcoinNetworks);
const result = networks.find(
info => hash.toLowerCase() === info.hashGenesisBlock.toLowerCase(),
);

View File

@@ -1,44 +0,0 @@
// origin: https://github.com/trezor/connect/blob/develop/src/js/utils/objectUtils.js
export function clone<T>(obj: T): T {
const jsonString = JSON.stringify(obj);
if (jsonString === undefined) {
// jsonString === undefined IF and only IF obj === undefined
// therefore no need to clone
return obj;
}
return JSON.parse(jsonString);
}
export function entries<T>(obj: { [key: string]: T }): Array<[string, T]> {
const keys: string[] = Object.keys(obj);
return keys.map(key => [key, obj[key]]);
}
export function deepClone(_obj: any, _hash: any = new WeakMap()) {
// if (Object(obj) !== obj) return obj; // primitives
// if (hash.has(obj)) return hash.get(obj); // cyclic reference
// const result = Array.isArray(obj) ? [] : obj.constructor ? new obj.constructor() : Object.create(null);
// hash.set(obj, result);
// if (obj instanceof Map) { Array.from(obj, ([key, val]) => result.set(key, deepClone(val, hash))); }
// return Object.assign(result, ...Object.keys(obj).map(
// key => ({ [key]: deepClone(obj[key], hash) })));
}
export function snapshot(obj: any) {
if (obj == null || typeof obj !== 'object') {
return obj;
}
const temp = new obj.constructor();
Object.keys(temp).forEach(key => {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
temp[key] = snapshot(obj[key]);
}
});
return temp;
}
export function objectValues<X>(object: { [key: string]: X }): X[] {
return Object.keys(object).map(key => object[key]);
}

View File

@@ -30,6 +30,7 @@ import {
PrecomposedTransactionFinal,
PrecomposedTransactionFinalCardano,
} from '@suite-common/wallet-types';
import { cloneObject } from '@trezor/utils';
import * as sendFormBitcoinActions from './send/sendFormBitcoinActions';
import * as sendFormEthereumActions from './send/sendFormEthereumActions';
import * as sendFormRippleActions from './send/sendFormRippleActions';
@@ -103,19 +104,6 @@ export const removeDraft = () => (dispatch: Dispatch, getState: GetState) => {
}
};
// TODO: replace by structuredClone() after updating TS
const clone = <T>(info: T): T => {
const jsonString = JSON.stringify(info);
if (jsonString === undefined) {
// jsonString === undefined IF and only IF obj === undefined
// therefore no need to clone
return info;
}
return JSON.parse(jsonString);
};
export const convertDrafts = () => (dispatch: Dispatch, getState: GetState) => {
const { route } = getState().router;
@@ -150,7 +138,7 @@ export const convertDrafts = () => (dispatch: Dispatch, getState: GetState) => {
const conversionToUse =
areSatsSelected && areSatsSupported ? amountToSatoshi : formatAmount;
const updatedDraft = clone(draft);
const updatedDraft = cloneObject(draft);
const decimals = getAccountDecimals(relatedAccount.symbol)!;
updatedDraft.outputs.forEach(output => {

View File

@@ -4,6 +4,7 @@ import { SEND } from 'src/actions/wallet/constants';
import { Action } from 'src/types/suite';
import { FormState, PrecomposedTransactionFinal, TxFinalCardano } from 'src/types/wallet/sendForm';
import { accountsActions } from '@suite-common/wallet-core';
import { cloneObject } from '@trezor/utils';
export interface SendState {
drafts: {
@@ -30,7 +31,11 @@ const sendFormReducer = (state: SendState = initialState, action: Action): SendS
});
break;
case SEND.STORE_DRAFT:
draft.drafts[action.key] = action.formState;
// Deep-cloning to prevent buggy interaction between react-hook-form and immer, see https://github.com/orgs/react-hook-form/discussions/3715#discussioncomment-2151458
// Otherwise, whenever the outputs fieldArray is updated after the form draft or precomposedForm is saved, there is na error:
// TypeError: Cannot assign to read only property of object '#<Object>'
// This might not be necessary in the future when the dependencies are upgraded.
draft.drafts[action.key] = cloneObject(action.formState);
break;
case SEND.REMOVE_DRAFT:
delete draft.drafts[action.key];
@@ -46,7 +51,11 @@ const sendFormReducer = (state: SendState = initialState, action: Action): SendS
case SEND.REQUEST_SIGN_TRANSACTION:
if (action.payload) {
draft.precomposedTx = action.payload.transactionInfo;
draft.precomposedForm = action.payload.formValues;
// Deep-cloning to prevent buggy interaction between react-hook-form and immer, see https://github.com/orgs/react-hook-form/discussions/3715#discussioncomment-2151458
// Otherwise, whenever the outputs fieldArray is updated after the form draft or precomposedForm is saved, there is na error:
// TypeError: Cannot assign to read only property of object '#<Object>'
// This might not be necessary in the future when the dependencies are upgraded.
draft.precomposedForm = cloneObject(action.payload.formValues);
} else {
delete draft.precomposedTx;
delete draft.precomposedForm;

View File

@@ -0,0 +1,10 @@
// Makes a deep copy of an object.
export const cloneObject = <T>(obj: T): T => {
const jsonString = JSON.stringify(obj);
if (jsonString === undefined) {
// jsonString === undefined IF and only IF obj === undefined
// therefore no need to clone
return obj;
}
return JSON.parse(jsonString);
};

View File

@@ -5,6 +5,7 @@ export * from './arrayToDictionary';
export * as bufferUtils from './bufferUtils';
export * from './bytesToHumanReadable';
export * from './capitalizeFirstLetter';
export * from './cloneObject';
export * from './countBytesInString';
export * from './createDeferred';
export * from './createTimeoutPromise';