Skip to content

Commit

Permalink
Fail with explicit error when creating proxy for missing stdlib (zepp…
Browse files Browse the repository at this point in the history
…elinos#322)

* Fail with explicit error when creating proxy for missing stdlib

Creating a proxy for a contract that belongs to an stdlib that was
linked, but that link was not pushed to the network, would yield
a "VM Revert" error. Now it explains that the stdlib is linked but
a push is missing.

* Improve error handling in check methods of network base controller
  • Loading branch information
spalladino authored and bakaoh committed Jul 31, 2018
1 parent edef540 commit b1a4d0a
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 17 deletions.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"mock-stdlib-invalid": "file:./test/mocks/mock-stdlib-invalid",
"mock-stdlib-undeployed": "file:./test/mocks/mock-stdlib-undeployed",
"react": "^16.3.2",
"react-dom": "^16.3.2"
"react-dom": "^16.3.2",
"sinon": "^6.1.4"
}
}
9 changes: 9 additions & 0 deletions src/models/network/NetworkAppController.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ export default class NetworkAppController extends NetworkBaseController {
return super.isContractDefined(contractAlias) || this.isStdlibContract(contractAlias);
}

_errorForLocalContractDeployed(contractAlias) {
const baseErr = super._errorForLocalContractDeployed(contractAlias);
if (baseErr) {
return baseErr;
} else if (this.isStdlibContract(contractAlias) && !this.networkFile.hasStdlib()) {
return `Contract ${contractAlias} is provided by ${this.packageFile.stdlibName} but it was not deployed to the network, consider running \`zos push\``;
}
}

_updateTruffleDeployedInformation(contractAlias, implementation) {
const contractName = this.packageFile.contract(contractAlias)
if (contractName) {
Expand Down
42 changes: 28 additions & 14 deletions src/models/network/NetworkBaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,48 +111,62 @@ export default class NetworkBaseController {
}

checkLocalContractsDeployed(throwIfFail = false) {
let msg;
const err = this._errorForLocalContractsDeployed();
if (err) this._handleErrorMessage(err, throwIfFail);
}

_errorForLocalContractsDeployed() {
const [contractsDeployed, contractsMissing] = _.partition(this.packageFile.contractAliases, (alias) => this.isContractDeployed(alias));
const contractsChanged = _.filter(contractsDeployed, (alias) => this.hasContractChanged(alias));

if (!_.isEmpty(contractsMissing)) {
msg = `Contracts ${contractsMissing.join(', ')} are not deployed.`;
return `Contracts ${contractsMissing.join(', ')} are not deployed.`;
} else if (!_.isEmpty(contractsChanged)) {
msg = `Contracts ${contractsChanged.join(', ')} have changed since the last deploy.`;
return `Contracts ${contractsChanged.join(', ')} have changed since the last deploy.`;
}

if (msg && throwIfFail) throw Error(msg);
else if (msg) log.info(msg);
}

checkLocalContractDeployed(contractAlias, throwIfFail = false) {
let msg;
const err = this._errorForLocalContractDeployed(contractAlias);
if (err) this._handleErrorMessage(err, throwIfFail);
}

_errorForLocalContractDeployed(contractAlias) {
if (!this.isContractDefined(contractAlias)) {
msg = `Contract ${contractAlias} not found in application or stdlib`;
return `Contract ${contractAlias} not found in this project`;
} else if (!this.isContractDeployed(contractAlias)) {
msg = `Contract ${contractAlias} is not deployed to ${this.network}.`;
return `Contract ${contractAlias} is not deployed to ${this.network}.`;
} else if (this.hasContractChanged(contractAlias)) {
msg = `Contract ${contractAlias} has changed locally since the last deploy, consider running 'zos push'.`;
return `Contract ${contractAlias} has changed locally since the last deploy, consider running 'zos push'.`;
}
}

if (msg && throwIfFail) throw Error(msg);
else if (msg) log.info(msg);
_handleErrorMessage(msg, throwIfFail = false) {
if (throwIfFail) {
throw Error(msg);
} else {
log.info(msg);
}
}

hasContractChanged(contractAlias) {
if (!this.packageFile.hasContract(contractAlias)) return false;
if (!this.isLocalContract(contractAlias)) return false;
if (!this.isContractDeployed(contractAlias)) return true;
const contractName = this.packageFile.contract(contractAlias);
const contractClass = Contracts.getFromLocal(contractName);
return !this.networkFile.hasSameBytecode(contractAlias, contractClass)
}

isLocalContract(contractAlias) {
return this.packageFile.hasContract(contractAlias)
}

isContractDefined(contractAlias) {
return this.packageFile.hasContract(contractAlias)
}

isContractDeployed(contractAlias) {
return !this.packageFile.hasContract(contractAlias) || this.networkFile.hasContract(contractAlias);
return !this.isLocalContract(contractAlias) || this.networkFile.hasContract(contractAlias);
}

writeNetworkPackage() {
Expand Down
64 changes: 64 additions & 0 deletions test/commands/add.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
'use strict'
require('../setup')

import sinon from 'sinon';

import * as add from '../../src/scripts/add';
import * as addAll from '../../src/scripts/add-all';
import * as push from '../../src/scripts/push';
import * as runWithTruffle from '../../src/utils/runWithTruffle'
import Session from '../../src/models/network/Session'
import program from '../../src/bin/program';

contract('add command', function() {

it('should call add script with an alias and a filename', function(done) {
var addFake = sinon.fake(function () {
expect(addFake.calledOnce).to.equal(true)
expect(addFake.calledWithExactly({ contractsData: [ { name: 'ImplV1', alias: 'Impl' } ] })).to.equal(true)
addStub.restore()
done()
})

var addStub = sinon.stub(add, 'default').callsFake(addFake)
program.parse(['node', 'zos', 'add', 'ImplV1:Impl', '--skip-compile'])
});

it('should call add-all script if passed --all option', function(done) {
var addAllFake = sinon.fake(function () {
expect(addAllFake.calledOnce).to.equal(true)
expect(addAllFake.calledWithExactly( { } )).to.equal(true)
addAllStub.restore()
done()
})

var addAllStub = sinon.stub(addAll, 'default').callsFake(addAllFake)
program.parse(['node', 'zos', 'add', '--all', '--skip-compile'])
});

it('should call push script if passed --push option', function(done) {
var addAllFake = sinon.fake()
var runWithTruffleFake = function(script, options) {
const { network, from, timeout } = Session.getOptions(options)
const txParams = from ? { from } : {}

if (!network) throw Error('A network name must be provided to execute the requested action.')
script({ network, txParams })
}
var pushFake = sinon.fake(function () {
expect(pushFake.calledOnce).to.equal(true)
expect(pushFake.calledWithExactly({ deployStdlib: undefined, reupload: undefined, network: 'test', txParams: {} })).to.equal(true)
addAllStub.restore()
pushStub.restore()
runWithTruffleStub.restore()
done()
})

var addAllStub = sinon.stub(addAll, 'default').callsFake(addAllFake)
var pushStub = sinon.stub(push, 'default').callsFake(pushFake)
var runWithTruffleStub = sinon.stub(runWithTruffle, 'default').callsFake(runWithTruffleFake)

program.parse(['node', 'zos', 'add', '--all', '--push', 'test', '--skip-compile'])
});

});
19 changes: 17 additions & 2 deletions test/scripts/create.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ contract('create script', function([_, owner]) {

it('should refuse to create a proxy for an undefined contract', async function() {
await createProxy({ contractAlias: 'NotExists', network, txParams, networkFile: this.networkFile })
.should.be.rejectedWith('Contract NotExists not found in application or stdlib');
.should.be.rejectedWith(/Contract NotExists not found/);
});

it('should refuse to create a proxy for a lib project', async function() {
Expand Down Expand Up @@ -148,11 +148,26 @@ contract('create script', function([_, owner]) {

it('should create a proxy for a stdlib contract', async function () {
await createProxy({ contractAlias: 'Greeter', network, txParams, networkFile: this.networkFile });

await assertProxy(this.networkFile, 'Greeter', { version });
});
});

describe('with unpushed stdlib link', function () {
beforeEach('setting stdlib', async function () {
await linkStdlib({ stdlibNameVersion: '[email protected]', packageFile: this.packageFile });
});

it('should refuse create a proxy for a stdlib contract', async function () {
await createProxy({ contractAlias: 'Greeter', network, txParams, networkFile: this.networkFile })
.should.be.rejectedWith(/Contract Greeter is provided by mock-stdlib but it was not deployed to the network/)
});
});

it('should refuse to create a proxy for an undefined contract', async function() {
await createProxy({ contractAlias: 'NotExists', network, txParams, networkFile: this.networkFile })
.should.be.rejectedWith(/Contract NotExists not found/);
});

describe('with local modifications', function () {
beforeEach('changing local network file to have a different bytecode', async function () {
const contracts = this.networkFile.contracts
Expand Down

0 comments on commit b1a4d0a

Please sign in to comment.