This repository was archived by the owner on Nov 10, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 357
(Feature) Refactor gas calculation #1756
Merged
dasanra
merged 29 commits into
feature/1048-tx-will-fail-warning
from
feature/refactor-gas-calculation
Jan 11, 2021
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
f768d4c
Refactor proccessTransaction to use getPreValidatedSignatures method
Agupane 694e450
Fix default export of ApproveTxModal
Agupane 2c5f21d
Rename estimateExecTransactionGas to estimateGasForTransactionCreation
Agupane 40ec65d
Make estimateGasForTransactionCreation throw error instead of 0 gas
Agupane a1ac679
Adds estimateGasForTransactionExecution and estimateGasForTransaction…
Agupane 25bbd33
Move estimateTransactionGas to useEstimateTransactionGas
Agupane 7895924
Type and refactor generateSignaturesFromTxConfirmations
Agupane d8d64c9
Uses confirmations to estimateGasForTransactionExecution
Agupane 88d36f8
Adds TransactionFeesText component
Agupane 7c6ba58
Pass more parameters to estimateGasForTransactionExecution
Agupane 9c5bb19
Merge with 1048-tx-will-fail-warning
Agupane c6efa91
Removes unnecessary parameter in getNewTxNonce
Agupane f9fb706
Moves checkIfOffChainSignatureIsPossible to safeTxSigner.ts
Agupane e0b3af6
Fix check for null confirmations
Agupane d819531
Uses checkIfOffChainSignatureIsPossible on createTransaction.ts
Agupane 4ab94ac
Move TransactionFailText inside TransactionFees component
Agupane bee96c5
Pass safeTxGas to useEstimateTransactionGas.tsx
Agupane 63cab3f
Fix gas iteration on estimateGasForTransactionExecution
Agupane 00919c7
Fix estimateGasForTransactionExecution calculation
Agupane d25e907
Fix generateSignaturesFromTxConfirmations calculation
Agupane fccd8d1
Remove unnecessary Promise and await
289b6e2
Fix estimateGasForTransactionExecution for preApproving owner case
Agupane b8d0878
Merge branch 'feature/refactor-gas-calculation' of https://github.com…
Agupane 7ca4b64
Improve logging
Agupane 7f29a64
Merge branch 'feature/1048-tx-will-fail-warning' into feature/refacto…
Agupane 71fb956
Remove log
Agupane 40dabdd
Uses operation in useEstimateTransactionGas
Agupane 64d4fc3
Merge branch 'feature/1048-tx-will-fail-warning' into feature/refacto…
Agupane 0a77aa3
Fix missing dependency
Agupane File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import React from 'react' | ||
| import { EstimationStatus } from 'src/logic/hooks/useEstimateTransactionGas' | ||
| import Paragraph from 'src/components/layout/Paragraph' | ||
| import { getNetworkInfo } from 'src/config' | ||
| import { TransactionFailText } from 'src/components/TransactionFailText' | ||
|
|
||
| type TransactionFailTextProps = { | ||
| txEstimationExecutionStatus: EstimationStatus | ||
| gasCostFormatted: string | ||
| isExecution: boolean | ||
| isCreation: boolean | ||
| isOffChainSignature: boolean | ||
| } | ||
| const { nativeCoin } = getNetworkInfo() | ||
|
|
||
| export const TransactionFees = ({ | ||
| gasCostFormatted, | ||
| isExecution, | ||
| isCreation, | ||
| isOffChainSignature, | ||
| txEstimationExecutionStatus, | ||
| }: TransactionFailTextProps): React.ReactElement | null => { | ||
| let transactionAction | ||
| if (isCreation) { | ||
| transactionAction = 'create' | ||
| } else if (isExecution) { | ||
| transactionAction = 'execute' | ||
| } else { | ||
| transactionAction = 'approve' | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <Paragraph> | ||
| You're about to {transactionAction} a transaction and will have to confirm it with your currently connected | ||
| wallet. | ||
| {!isOffChainSignature && | ||
| ` Make sure you have ${gasCostFormatted} (fee price) ${nativeCoin.name} in this wallet to fund this confirmation.`} | ||
| </Paragraph> | ||
| <TransactionFailText txEstimationExecutionStatus={txEstimationExecutionStatus} isExecution={isExecution} /> | ||
| </> | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,61 @@ | ||
| // https://docs.gnosis.io/safe/docs/docs5/#pre-validated-signatures | ||
| // https://github.com/gnosis/safe-contracts/blob/master/test/gnosisSafeTeamEdition.js#L26 | ||
| export const generateSignaturesFromTxConfirmations = (confirmations, preApprovingOwner) => { | ||
| // The constant parts need to be sorted so that the recovered signers are sorted ascending | ||
| // (natural order) by address (not checksummed). | ||
| const confirmationsMap = confirmations.reduce((map, obj) => { | ||
| map[obj.owner.toLowerCase()] = obj // eslint-disable-line no-param-reassign | ||
| return map | ||
| }, {}) | ||
| import { List } from 'immutable' | ||
| import { Confirmation } from 'src/logic/safe/store/models/types/confirmation' | ||
| import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions' | ||
| import semverSatisfies from 'semver/functions/satisfies' | ||
| import { SAFE_VERSION_FOR_OFFCHAIN_SIGNATURES } from './transactions/offchainSigner' | ||
|
|
||
| // Here we're checking that safe contract version is greater or equal 1.1.1, but | ||
| // theoretically EIP712 should also work for 1.0.0 contracts | ||
| // Also, offchain signatures are not working for ledger/trezor wallet because of a bug in their library: | ||
| // https://github.com/LedgerHQ/ledgerjs/issues/378 | ||
| // Couldn't find an issue for trezor but the error is almost the same | ||
| export const checkIfOffChainSignatureIsPossible = ( | ||
| isExecution: boolean, | ||
| isSmartContractWallet: boolean, | ||
| safeVersion?: string, | ||
| ): boolean => | ||
| !isExecution && | ||
| !isSmartContractWallet && | ||
| !!safeVersion && | ||
| semverSatisfies(safeVersion, SAFE_VERSION_FOR_OFFCHAIN_SIGNATURES) | ||
|
|
||
| // https://docs.gnosis.io/safe/docs/contracts_signatures/#pre-validated-signatures | ||
| export const getPreValidatedSignatures = (from: string, initialString: string = EMPTY_DATA): string => { | ||
| return `${initialString}000000000000000000000000${from.replace( | ||
| EMPTY_DATA, | ||
| '', | ||
| )}000000000000000000000000000000000000000000000000000000000000000001` | ||
| } | ||
|
|
||
| export const generateSignaturesFromTxConfirmations = ( | ||
| confirmations?: List<Confirmation>, | ||
| preApprovingOwner?: string, | ||
| ): string => { | ||
| let confirmationsMap = | ||
| confirmations?.map((value) => { | ||
| return { | ||
| signature: value.signature, | ||
| owner: value.owner.toLowerCase(), | ||
| } | ||
| }) || List([]) | ||
|
|
||
| if (preApprovingOwner) { | ||
| confirmationsMap[preApprovingOwner.toLowerCase()] = { owner: preApprovingOwner } | ||
| confirmationsMap = confirmationsMap.push({ owner: preApprovingOwner, signature: null }) | ||
| } | ||
|
|
||
| // The constant parts need to be sorted so that the recovered signers are sorted ascending | ||
| // (natural order) by address (not checksummed). | ||
| confirmationsMap = confirmationsMap.sort((ownerA, ownerB) => ownerA.owner.localeCompare(ownerB.owner)) | ||
|
|
||
| let sigs = '0x' | ||
| Object.keys(confirmationsMap) | ||
| .sort() | ||
| .forEach((addr) => { | ||
| const conf = confirmationsMap[addr] | ||
| if (conf.signature) { | ||
| sigs += conf.signature.slice(2) | ||
| } else { | ||
| // https://docs.gnosis.io/safe/docs/docs5/#pre-validated-signatures | ||
| sigs += `000000000000000000000000${addr.replace( | ||
| '0x', | ||
| '', | ||
| )}000000000000000000000000000000000000000000000000000000000000000001` | ||
| } | ||
| }) | ||
| confirmationsMap.forEach(({ signature, owner }) => { | ||
| if (signature) { | ||
| sigs += signature.slice(2) | ||
| } else { | ||
| // https://docs.gnosis.io/safe/docs/contracts_signatures/#pre-validated-signatures | ||
| sigs += getPreValidatedSignatures(owner, '') | ||
| } | ||
| }) | ||
|
|
||
| return sigs | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure to understand this, are we checking the checkbox status somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not needed, it's the same logic that uses the checkbox in
ApproveTxModalhere: