mirror of https://github.com/embarklabs/embark.git
fix(@embark/deployment): don't over estimate gas when running tests against non-simulator nodes
When running tests against non-simulated blockchain nodes, even for simplex
Smart Contracts, deployment transactions would exceed the block gas limit.
E.g. running `embark blockchain` in one process and `embark test --node embark`
in another, inside our demo application, will throw an error when Embark attempts
to deploy its `SimpleStorage`:
```
Compiling contracts
Compilation done
[SimpleStorage]: error deploying =SimpleStorage= due to error: Returned error: exceeds block gas limit
Error deploying contracts. Please fix errors to continue.
Error deploying contracts. Please fix errors to continue.
terminating due to error
Error deploying contracts. Please fix errors to continue.
```
The reason for that is because in https://github.com/embark-framework/embark/pull/1650, we've introduced a static
gas estimation for Smart Contract deployment that is just right below Ganache's
maximum gas limit of `6721975`, since Ganache tends to underestimate gas for
complex Smart Contracts due to its [low base fee](8ad1ab29de/lib/utils/gasEstimation.js (L33-L39)
).
The static gas estimation would apply any time we're in a test context, but we
didn't take into account the case where tests are executed against nodes
other than the simulated environment.
As mentioned in the comments in the linked PR:
> If this is not spec'ed at all, I wonder what complications it could cause when
> at some point we maybe switch to not using Ganache anymore for tests, or even
> the user itself (which I think is a reasonable thing to do).
This causes the error described above because we easily reach the block gas limit
with just two Smart Contracts and Embark already deploys a few Smart Contracts for
ENS.
So basically what we want is to use the static gas estimation when we know
the node we're connecting to is Ganache. In all other cases we can rely on the
standardized gas estimation offered by the node.
This commit is contained in:
parent
421c340613
commit
d76a82a30a
|
@ -715,6 +715,10 @@ class BlockchainConnector {
|
|||
});
|
||||
}
|
||||
|
||||
getClientVersion(cb) {
|
||||
this.web3._requestManager.send({method: 'web3_clientVersion', params: []}, cb);
|
||||
}
|
||||
|
||||
getNetworkId() {
|
||||
return this.web3.eth.net.getId();
|
||||
}
|
||||
|
|
|
@ -5,6 +5,7 @@ const {ZERO_ADDRESS} = AddressUtils;
|
|||
|
||||
// Check out definition 97 of the yellow paper: https://ethereum.github.io/yellowpaper/paper.pdf
|
||||
const MAX_CONTRACT_BYTECODE_LENGTH = 24576;
|
||||
const GANACHE_CLIENT_VERSION_NAME = "EthereumJS TestRPC";
|
||||
|
||||
class ContractDeployer {
|
||||
constructor(options) {
|
||||
|
@ -320,7 +321,8 @@ class ContractDeployer {
|
|||
next();
|
||||
},
|
||||
function estimateCorrectGas(next) {
|
||||
if (contract._skipGasEstimations) {
|
||||
self.blockchain.getClientVersion((err, version) => {
|
||||
if (version.split('/')[0] === GANACHE_CLIENT_VERSION_NAME) {
|
||||
// This is Ganache's gas limit. We subtract 1 so we don't reach the limit.
|
||||
//
|
||||
// We do this because Ganache's gas estimates are wrong (contract creation
|
||||
|
@ -338,6 +340,7 @@ class ContractDeployer {
|
|||
});
|
||||
}
|
||||
next();
|
||||
});
|
||||
},
|
||||
function deployTheContract(next) {
|
||||
let estimatedCost = contract.gas * contract.gasPrice;
|
||||
|
|
|
@ -36,7 +36,6 @@ class DeployManager {
|
|||
this.events.setCommandHandler('deploy:contracts:test', (cb) => {
|
||||
self.fatalErrors = true;
|
||||
self.deployOnlyOnConfig = true;
|
||||
self.skipGasEstimations = true;
|
||||
self.deployContracts(cb);
|
||||
});
|
||||
}
|
||||
|
@ -71,7 +70,6 @@ class DeployManager {
|
|||
callback = result;
|
||||
}
|
||||
contract._gasLimit = self.gasLimit;
|
||||
contract._skipGasEstimations = self.skipGasEstimations;
|
||||
self.events.request('deploy:contract', contract, (err) => {
|
||||
if (err) {
|
||||
contract.error = err.message || err;
|
||||
|
|
Loading…
Reference in New Issue