Skip to content

Commit

Permalink
Remove unnecessary tx meta properties (#8489)
Browse files Browse the repository at this point in the history 
* Remove `estimatedGas` property from `txMeta`

The `estimatedGas` property was a cache of the gas value estimated for
a transaction when the default gas limit was set. This property wasn't
used anywhere. It may have been useful for debugging purposes, but the
same gas estimate is already stored on the `history` property so it
should be present in state logs regardless.

* Remove `gasLimitSpecified` txMeta property

The `gasLimitSpecified` property of `txMeta` wasn't used for anything.
It might have been useful for debugging purposes, but whether or not
the gas limit was specified can also be determined from looking at the
transaction history, so it's not a huge loss.

* Remove `gasPriceSpecified` txMeta property

The `gasPriceSpecified` property of `txMeta` wasn't used for anything.
It might have been useful for debugging purposes, but whether or not
the gas price was specified can also be determined from looking at the
transaction history, so it's not a huge loss.

* Remove `simpleSend` txMeta property

The `simpleSend` property of `txMeta` was used to ensure a buffer was
not added to the gas limit during gas estimation for simple send
transactions. It was made redundant by #8484, which accomplishes this
without the use of this property.
  • Loading branch information
@Gudahtt
Gudahtt authored May 1, 2020
1 parent 92592fc commit 165666b
Show file tree
Hide file tree
Showing 14 changed files with 35 additions and 614 deletions.
3 changes: 0 additions & 3 deletions app/scripts/background.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ initialize().catch(log.error)
* @property {boolean} loadingDefaults - TODO: Document
* @property {Object} txParams - The tx params as passed to the network provider.
* @property {Object[]} history - A history of mutations to this TransactionMeta object.
* @property {boolean} gasPriceSpecified - True if the suggesting dapp specified a gas price, prevents auto-estimation.
* @property {boolean} gasLimitSpecified - True if the suggesting dapp specified a gas limit, prevents auto-estimation.
* @property {string} estimatedGas - A hex string represented the estimated gas limit required to complete the transaction.
* @property {string} origin - A string representing the interface that suggested the transaction.
* @property {Object} nonceDetails - A metadata object containing information used to derive the suggested nonce, useful for debugging nonce issues.
* @property {string} rawTx - A hex string of the final signed transaction, ready to submit to the network.
Expand Down
3 changes: 0 additions & 3 deletions app/scripts/controllers/transactions/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ txMeta = {
"value": "0x3b9aca00"
},
...], // I've removed most of history for this
"gasPriceSpecified": false, //whether or not the user/dapp has specified gasPrice
"gasLimitSpecified": false, //whether or not the user/dapp has specified gas
"estimatedGas": "5208",
"origin": "MetaMask", //debug
"nonceDetails": {
"params": {
Expand Down
6 changes: 0 additions & 6 deletions app/scripts/controllers/transactions/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ export default class TransactionController extends EventEmitter {
async addTxGasDefaults (txMeta, getCodeResponse) {
const txParams = txMeta.txParams

txMeta.gasPriceSpecified = Boolean(txParams.gasPrice)
let gasPrice = txParams.gasPrice
if (!gasPrice) {
gasPrice = this.getGasPrice ? this.getGasPrice() : await this.query.gasPrice()
Expand All @@ -278,8 +277,6 @@ export default class TransactionController extends EventEmitter {
// set gasLimit

if (txParams.gas) {
txMeta.estimatedGas = txParams.gas
txMeta.gasLimitSpecified = Boolean(txParams.gas)
return txMeta
} else if (
txParams.to &&
Expand All @@ -298,9 +295,6 @@ export default class TransactionController extends EventEmitter {

// This is a standard ether simple send, gas requirement is exactly 21k
txParams.gas = SIMPLE_GAS_COST
txMeta.estimatedGas = SIMPLE_GAS_COST
txMeta.simpleSend = true
txMeta.gasLimitSpecified = Boolean(txParams.gas)
return txMeta
}

Expand Down
3 changes: 1 addition & 2 deletions app/scripts/controllers/transactions/tx-gas-utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,11 @@ export default class TxGasUtil {
@param {string} estimatedGasHex - the estimated gas hex
*/
setTxGas (txMeta, blockGasLimitHex, estimatedGasHex) {
txMeta.estimatedGas = addHexPrefix(estimatedGasHex)
const txParams = txMeta.txParams

// if gasLimit not originally specified,
// try adding an additional gas buffer to our estimation for safety
const recommendedGasHex = this.addGasBuffer(txMeta.estimatedGas, blockGasLimitHex)
const recommendedGasHex = this.addGasBuffer(addHexPrefix(estimatedGasHex), blockGasLimitHex)
txParams.gas = recommendedGasHex
return
}
Expand Down
132 changes: 3 additions & 129 deletions development/states/tx-list-items.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@
"message": "Error: intrinsic gas too low",
"stack": "Error: some error"
},
"estimatedGas": "0xcf08",
"gasLimitSpecified": true,
"gasPriceSpecified": true,
"history": [
{
"id": 4068311466147836,
Expand All @@ -99,21 +96,6 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": true
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": true
},
{
"op": "add",
"path": "/estimatedGas",
"value": "0xcf08"
}
],
[
Expand Down Expand Up @@ -270,21 +252,6 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": true
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": true
},
{
"op": "add",
"path": "/estimatedGas",
"value": "0xcf08"
}
],
[
Expand All @@ -303,16 +270,10 @@
"note": "txStateManager: setting status to approved"
}
]
],
"gasPriceSpecified": true,
"gasLimitSpecified": true,
"estimatedGas": "0xcf08"
]
},
{
"estimatedGas": "8d41",
"firstRetryBlockNumber": "0x2cbc70",
"gasLimitSpecified": false,
"gasPriceSpecified": false,
"hash": "0xfbd997bf9bb85ca1598952ca23e7910502d527e06cb6ee1bbe7e7dd59d6909cd",
"history": [
{
Expand Down Expand Up @@ -347,21 +308,6 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": false
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": false
},
{
"op": "add",
"path": "/estimatedGas",
"value": "8d41"
}
],
[
Expand Down Expand Up @@ -517,10 +463,7 @@
}
},
{
"estimatedGas": "8d41",
"firstRetryBlockNumber": "0x2cbc70",
"gasLimitSpecified": false,
"gasPriceSpecified": false,
"hash": "0xfbd997bf9bb85ca1598952ca23e7910502d527e06cb6ee1bbe7e7dd59d6909cd",
"history": [
{
Expand Down Expand Up @@ -555,21 +498,6 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": false
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": false
},
{
"op": "add",
"path": "/estimatedGas",
"value": "8d41"
}
],
[
Expand Down Expand Up @@ -759,21 +687,6 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": true
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": true
},
{
"op": "add",
"path": "/estimatedGas",
"value": "0xcf08"
}
],
[],
Expand Down Expand Up @@ -873,9 +786,6 @@
}
]
],
"gasPriceSpecified": true,
"gasLimitSpecified": true,
"estimatedGas": "0xcf08",
"nonceDetails": {
"params": {
"highestLocalNonce": 3,
Expand Down Expand Up @@ -936,27 +846,9 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": true
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": true
},
{
"op": "add",
"path": "/estimatedGas",
"value": "0xcf08"
}
]
],
"gasPriceSpecified": true,
"gasLimitSpecified": true,
"estimatedGas": "0xcf08"
]
}
],
"unapprovedTxs": {
Expand Down Expand Up @@ -993,27 +885,9 @@
"op": "replace",
"path": "/loadingDefaults",
"value": false
},
{
"op": "add",
"path": "/gasPriceSpecified",
"value": true
},
{
"op": "add",
"path": "/gasLimitSpecified",
"value": true
},
{
"op": "add",
"path": "/estimatedGas",
"value": "0xcf08"
}
]
],
"gasPriceSpecified": true,
"gasLimitSpecified": true,
"estimatedGas": "0xcf08"
]
}
},
"unapprovedMsgs": {
Expand Down
Loading

0 comments on commit 165666b

Please sign in to comment.