fix(WalletConnect): Fixing crashes

1. In some cases it was crashing on JSON.stringify for the model item
2. Avoid storing model objects
3. Fixing storybook
This commit is contained in:
Alex Jbanca 2024-09-17 10:42:01 +03:00 committed by Alex Jbanca
parent 1601da58ff
commit c769e42212
6 changed files with 141 additions and 114 deletions

View File

@ -216,7 +216,7 @@ Item {
if (d.activeTestCase < d.openPairTestCase)
return
let buttons = InspectionUtils.findVisualsByTypeName(dappsWorkflow.popup, "StatusButton")
let buttons = StoryBook.InspectionUtils.findVisualsByTypeName(dappsWorkflow.popup, "StatusButton")
if (buttons.length === 1) {
buttons[0].clicked()
}
@ -227,7 +227,7 @@ Item {
return
if (pairUriInput.text.length > 0) {
let items = InspectionUtils.findVisualsByTypeName(dappsWorkflow, "StatusBaseInput")
let items = StoryBook.InspectionUtils.findVisualsByTypeName(dappsWorkflow, "StatusBaseInput")
if (items.length === 1) {
items[0].text = pairUriInput.text
@ -241,9 +241,9 @@ Item {
return
}
let modals = InspectionUtils.findVisualsByTypeName(dappsWorkflow, "PairWCModal")
let modals = StoryBook.InspectionUtils.findVisualsByTypeName(dappsWorkflow, "PairWCModal")
if (modals.length === 1) {
let buttons = InspectionUtils.findVisualsByTypeName(modals[0].footer, "StatusButton")
let buttons = StoryBook.InspectionUtils.findVisualsByTypeName(modals[0].footer, "StatusButton")
if (buttons.length === 1 && buttons[0].enabled && walletConnectService.wcSDK.sdkReady) {
d.activeTestCase = d.noTestCase
buttons[0].clicked()
@ -307,6 +307,7 @@ Item {
signal dappsListReceived(string dappsJson)
signal userAuthenticated(string topic, string id, string password, string pin)
signal userAuthenticationFailed(string topic, string id)
signal signingResult(string topic, string id, string data)
function addWalletConnectSession(sessionJson) {
console.info("Persist Session", sessionJson)

View File

@ -528,7 +528,7 @@ Item {
const store = service.store
const testAddress = "0x3a"
const chainId = 2
const chainId = "2"
const method = "personal_sign"
const message = "hello world"
const params = [`"${DAppsHelpers.strToHex(message)}"`, `"${testAddress}"`]
@ -550,10 +550,10 @@ Item {
compare(request.dappName, Testing.dappName, "expected dappName to be set")
compare(request.dappUrl, Testing.dappUrl, "expected dappUrl to be set")
compare(request.dappIcon, Testing.dappFirstIcon, "expected dappIcon to be set")
verify(!!request.account, "expected account to be set")
compare(request.account.address, testAddress, "expected look up of the right account")
verify(!!request.network, "expected network to be set")
compare(request.network.chainId, chainId, "expected look up of the right network")
verify(!!request.accountAddress, "expected account to be set")
compare(request.accountAddress, testAddress, "expected look up of the right account")
verify(!!request.chainId, "expected network to be set")
compare(request.chainId, chainId, "expected look up of the right network")
verify(!!request.data, "expected data to be set")
compare(request.data.message, message, "expected message to be set")
}
@ -909,8 +909,8 @@ Item {
topic,
id: requestEvent.id,
method: Constants.personal_sign,
account,
network,
accountAddress: account.address,
chainId: network.chainId,
data: message,
preparedData: message
})

View File

@ -157,23 +157,35 @@ DappsComboBox {
sourceComponent: DAppSignRequestModal {
id: dappRequestModal
objectName: "dappsRequestModal"
loginType: request.account.migragedToKeycard ? Constants.LoginType.Keycard : root.loginType
formatBigNumber: (number, symbol, noSymbolOption) => root.wcService.walletRootStore.currencyStore.formatBigNumber(number, symbol, noSymbolOption)
visible: true
property var feesInfo: null
readonly property var account: accountEntry.available ? accountEntry.item : {
name: "",
address: "",
emoji: "",
colorId: 0
}
readonly property var network: networkEntry.available ? networkEntry.item : {
chainName: "",
iconUrl: ""
}
loginType: account.migragedToKeycard ? Constants.LoginType.Keycard : root.loginType
formatBigNumber: (number, symbol, noSymbolOption) => root.wcService.walletRootStore.currencyStore.formatBigNumber(number, symbol, noSymbolOption)
visible: true
dappUrl: request.dappUrl
dappIcon: request.dappIcon
dappName: request.dappName
accountColor: Utils.getColorForId(request.account.colorId)
accountName: request.account.name
accountAddress: request.account.address
accountEmoji: request.account.emoji
accountColor: Utils.getColorForId(account.colorId)
accountName: account.name
accountAddress: account.address
accountEmoji: account.emoji
networkName: request.network.chainName
networkIconPath: Style.svg(request.network.iconUrl)
networkName: network.chainName
networkIconPath: Style.svg(network.iconUrl)
fiatFees: request.maxFeesText
cryptoFees: request.maxFeesEthText
@ -237,6 +249,20 @@ DappsComboBox {
dappRequestModal.estimatedTime = WalletUtils.getLabelForEstimatedTxTime(estimatedTimeEnum)
}
}
ModelEntry {
id: accountEntry
sourceModel: root.wcService.validAccounts
key: "address"
value: request.accountAddress
}
ModelEntry {
id: networkEntry
sourceModel: root.wcService.flatNetworks
key: "chainId"
value: request.chainId
}
}
}

View File

@ -30,7 +30,7 @@ SQUtils.QObject {
/// Beware, it will fail if called multiple times before getting an answer
function authenticate(request, payload) {
return store.authenticateUser(request.topic, request.id, request.account.address, payload)
return store.authenticateUser(request.topic, request.id, request.accountAddress, payload)
}
signal sessionRequest(SessionRequestResolved request)
@ -150,20 +150,20 @@ SQUtils.QObject {
// }
function resolveAsync(event) {
const method = event.params.request.method
const res = lookupAccountFromEvent(event, method)
if(!res.success) {
console.info("Error finding account for event", JSON.stringify(event))
const { accountAddress, success } = lookupAccountFromEvent(event, method)
if(!success) {
console.info("Error finding accountAddress for event", JSON.stringify(event))
return { obj: null, code: resolveAsyncResult.error }
}
if (!res.account) {
console.info("Ignoring request for an account not in the current profile.")
if (!accountAddress) {
console.info("Account not found for event", JSON.stringify(event))
return { obj: null, code: resolveAsyncResult.ignored }
}
const account = res.account
let network = lookupNetworkFromEvent(event, method)
if(!network) {
console.error("Error finding network for event", JSON.stringify(event))
let chainId = lookupNetworkFromEvent(event, method)
if(!chainId) {
console.error("Error finding chainId for event", JSON.stringify(event))
return { obj: null, code: resolveAsyncResult.error }
}
@ -181,8 +181,8 @@ SQUtils.QObject {
topic: event.topic,
id: event.id,
method,
account,
network,
accountAddress,
chainId,
data,
preparedData: interpreted.preparedData,
maxFeesText: "?",
@ -212,19 +212,20 @@ SQUtils.QObject {
return
}
let estimatedTimeEnum = getEstimatedTimeInterval(data, method, obj.network.chainId)
let estimatedTimeEnum = getEstimatedTimeInterval(data, method, obj.chainId)
root.estimatedTimeUpdated(estimatedTimeEnum)
const mainNet = lookupMainnetNetwork()
let mainChainId = obj.network.chainId
let mainChainId = obj.chainId
if (!!mainNet) {
mainChainId = mainNet.chainId
} else {
console.error("Error finding mainnet network")
}
let st = getEstimatedFeesStatus(data, method, obj.network.chainId, mainChainId)
let st = getEstimatedFeesStatus(data, method, obj.chainId, mainChainId)
let fundsStatus = checkFundsStatus(st.feesInfo.maxFees, st.feesInfo.l1GasFee, account.address, obj.network.chainId, mainNet.chainId, interpreted.value)
let fundsStatus = checkFundsStatus(st.feesInfo.maxFees, st.feesInfo.l1GasFee, obj.accountAddress, obj.chainId, mainNet.chainId, interpreted.value)
root.maxFeesUpdated(st.fiatMaxFees, st.maxFeesEth, fundsStatus.haveEnoughFunds,
fundsStatus.haveEnoughForFees, st.symbol, st.feesInfo)
@ -237,7 +238,7 @@ SQUtils.QObject {
}
/// returns {
/// account
/// accountAddress
/// success
/// }
/// if account is null and success is true it means that the account was not found
@ -245,47 +246,39 @@ SQUtils.QObject {
let address = ""
if (method === SessionRequest.methods.personalSign.name) {
if (event.params.request.params.length < 2) {
return { account: null, success: false }
return { accountAddress: "", success: false }
}
address = event.params.request.params[1]
} else if (method === SessionRequest.methods.sign.name) {
if (event.params.request.params.length === 1) {
return { account: null, success: false }
return { accountAddress: "", success: false }
}
address = event.params.request.params[0]
} else if(method === SessionRequest.methods.signTypedData_v4.name ||
method === SessionRequest.methods.signTypedData.name)
{
if (event.params.request.params.length < 2) {
return { account: null, success: false }
return { accountAddress: "", success: false }
}
address = event.params.request.params[0]
} else if (d.isTransactionMethod(method)) {
if (event.params.request.params.length == 0) {
return { account: null, success: false }
return { accountAddress: "", success: false }
}
address = event.params.request.params[0].from
} else {
console.error("Unsupported method to lookup account: ", method)
return { account: null, success: false }
return { accountAddress: "", success: false }
}
const account = SQUtils.ModelUtils.getFirstModelEntryIf(root.accountsModel, (account) => {
return account.address.toLowerCase() === address.toLowerCase();
})
if (!account) {
return { account: null, success: true }
return { accountAddress: "", success: true }
}
// deep copy to avoid operations on the original object
try {
const accountCopy = JSON.parse(JSON.stringify(account))
return { account: accountCopy, success: true }
}
catch (e) {
console.error("Error parsing account", e)
return { account: null, success: false }
}
return { accountAddress: account.address, success: true }
}
/// Returns null if the network is not found
@ -300,13 +293,7 @@ SQUtils.QObject {
return null
}
// deep copy to avoid operations on the original object
try {
return JSON.parse(JSON.stringify(network))
} catch (e) {
console.error("Error parsing network", network)
return null
}
return network.chainId
}
/// Returns null if the network is not found
@ -384,14 +371,14 @@ SQUtils.QObject {
if (request.method === SessionRequest.methods.sign.name) {
store.signMessageUnsafe(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.personalSign.getMessageFromData(request.data),
password,
pin)
} else if (request.method === SessionRequest.methods.personalSign.name) {
store.signMessage(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.personalSign.getMessageFromData(request.data),
password,
pin)
@ -401,9 +388,9 @@ SQUtils.QObject {
let legacy = request.method === SessionRequest.methods.signTypedData.name
store.safeSignTypedData(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.signTypedData.getMessageFromData(request.data),
request.network.chainId,
request.chainId,
legacy,
password,
pin)
@ -430,8 +417,8 @@ SQUtils.QObject {
if (request.method === SessionRequest.methods.signTransaction.name) {
store.signTransaction(request.topic,
request.id,
request.account.address,
request.network.chainId,
request.accountAddress,
request.chainId,
txObj,
password,
pin)
@ -439,8 +426,8 @@ SQUtils.QObject {
store.sendTransaction(
request.topic,
request.id,
request.account.address,
request.network.chainId,
request.accountAddress,
request.chainId,
txObj,
password,
pin)
@ -539,7 +526,7 @@ SQUtils.QObject {
function getBalanceInEth(balances, address, chainId) {
const BigOps = SQUtils.AmountsArithmetic
let accEth = SQUtils.ModelUtils.getFirstModelEntryIf(balances, (balance) => {
return balance.account.toLowerCase() === address.toLowerCase() && balance.chainId === chainId
return balance.account.toLowerCase() === address.toLowerCase() && balance.chainId == chainId
})
if (!accEth) {
console.error("Error balance lookup for account ", address, " on chain ", chainId)

View File

@ -64,13 +64,13 @@ WalletConnectSDKBase {
function resolveAsync(event) {
let method = event.params.request.method
let account = lookupAccountFromEvent(event, method)
if(!account) {
console.error("Error finding account for event", JSON.stringify(event))
let accountAddress = lookupAccountFromEvent(event, method)
if(!accountAddress) {
console.error("Error finding accountAddress for event", JSON.stringify(event))
return null
}
let network = lookupNetworkFromEvent(event, method)
if(!network) {
let chainId = lookupNetworkFromEvent(event, method)
if(!chainId) {
console.error("Error finding network for event", JSON.stringify(event))
return null
}
@ -85,8 +85,8 @@ WalletConnectSDKBase {
topic: event.topic,
id: event.id,
method,
account,
network,
accountAddress,
chainId,
data,
maxFeesText: "?",
maxFeesEthText: "?",
@ -126,25 +126,25 @@ WalletConnectSDKBase {
var address = ""
if (method === SessionRequest.methods.personalSign.name) {
if (event.params.request.params.length < 2) {
return null
return address
}
address = event.params.request.params[0]
} else if (method === SessionRequest.methods.sign.name) {
if (event.params.request.params.length === 1) {
return null
return address
}
address = event.params.request.params[0]
} else if(method === SessionRequest.methods.signTypedData_v4.name ||
method === SessionRequest.methods.signTypedData.name)
{
if (event.params.request.params.length < 2) {
return null
return address
}
address = event.params.request.params[0]
} else if (method === SessionRequest.methods.signTransaction.name
|| method === SessionRequest.methods.sendTransaction.name) {
if (event.params.request.params.length == 0) {
return null
return address
}
address = event.params.request.params[0]
}
@ -153,17 +153,10 @@ WalletConnectSDKBase {
})
if (!account) {
return null
return address
}
// deep copy to avoid operations on the original object
try {
return JSON.parse(JSON.stringify(account))
}
catch (e) {
console.error("Error parsing account", e.message)
return null
}
return account.address
}
/// Returns null if the network is not found
@ -172,20 +165,13 @@ WalletConnectSDKBase {
return null
}
const chainId = DAppsHelpers.chainIdFromEip155(event.params.chainId)
const network = SQUtils.ModelUtils.getByKey(networksModule.flatNetworks, "chainId", chainId)
const network = SQUtils.ModelUtils.getByKey(root.walletStore.filteredFlatModel, "chainId", chainId)
if (!network) {
return null
}
// deep copy to avoid operations on the original object
try {
return JSON.parse(JSON.stringify(network))
}
catch (e) {
console.error("Error parsing network", e)
return null
}
return network.chainId
}
function extractMethodData(event, method) {
@ -248,14 +234,14 @@ WalletConnectSDKBase {
if (request.method === SessionRequest.methods.sign.name) {
store.signMessageUnsafe(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.personalSign.getMessageFromData(request.data),
password,
pin)
} else if (request.method === SessionRequest.methods.personalSign.name) {
store.signMessage(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.personalSign.getMessageFromData(request.data),
password,
pin)
@ -265,9 +251,9 @@ WalletConnectSDKBase {
let legacy = request.method === SessionRequest.methods.signTypedData.name
store.safeSignTypedData(request.topic,
request.id,
request.account.address,
request.accountAddress,
SessionRequest.methods.signTypedData.getMessageFromData(request.data),
request.network.chainId,
request.chainId,
legacy,
password,
pin)
@ -275,8 +261,8 @@ WalletConnectSDKBase {
let txObj = SessionRequest.methods.signTransaction.getTxObjFromData(request.data)
store.signTransaction(request.topic,
request.id,
request.account.address,
request.network.chainId,
request.accountAddress,
request.chainId,
txObj,
password,
pin)
@ -285,7 +271,7 @@ WalletConnectSDKBase {
store.sendTransaction(request.topic,
request.id,
request.account.address,
request.network.chainId,
request.chainId,
txObj,
password,
pin)
@ -430,7 +416,21 @@ WalletConnectSDKBase {
sourceComponent: DAppSignRequestModal {
id: dappRequestModal
objectName: "connectorDappsRequestModal"
loginType: request.account.migragedToKeycard ? Constants.LoginType.Keycard : root.loginType
readonly property var account: accountEntry.available ? accountEntry.model : {
"address": "",
"name": "",
"emoji": "",
"colorId": 0
}
readonly property var network: networkEntry.available ? networkEntry.model : {
"chainId": 0,
"chainName": "",
"iconUrl": ""
}
loginType: account.migragedToKeycard ? Constants.LoginType.Keycard : root.loginType
formatBigNumber: (number, symbol, noSymbolOption) => root.wcService.walletRootStore.currencyStore.formatBigNumber(number, symbol, noSymbolOption)
visible: true
@ -439,13 +439,13 @@ WalletConnectSDKBase {
dappUrl: request.dappUrl
dappIcon: request.dappIcon
accountColor: Utils.getColorForId(request.account.colorId)
accountName: request.account.name
accountAddress: request.account.address
accountEmoji: request.account.emoji
accountColor: Utils.getColorForId(account.colorId)
accountName: account.name
accountAddress: account.address
accountEmoji: account.emoji
networkName: request.network.chainName
networkIconPath: Style.svg(request.network.iconUrl)
networkName: network.chainName
networkIconPath: Style.svg(network.iconUrl)
fiatFees: request.maxFeesText
cryptoFees: request.maxFeesEthText
@ -503,6 +503,20 @@ WalletConnectSDKBase {
controller.rejectTransactionSigning(root.requestId)
root.wcService.displayToastMessage(qsTr("Failed to sign transaction from %1").arg(request.dappUrl), true)
}
ModelEntry {
id: networkEntry
sourceModel: root.wcService.flatNetworks
key: "chainId"
value: request.chainId
}
ModelEntry {
id: accountEntry
sourceModel: root.wcService.validAccounts
key: "address"
value: request.accountAddress
}
}
}

View File

@ -18,9 +18,8 @@ QObject {
required property string topic
required property string id
required property string method
required property var account
required property var network
required property string accountAddress
required property string chainId
required property var data
// Data prepared for display in a human readable format