diff --git a/contracts/apps/AppProxyPinned.sol b/contracts/apps/AppProxyPinned.sol index 4b5c6fdd7..8dfff325b 100644 --- a/contracts/apps/AppProxyPinned.sol +++ b/contracts/apps/AppProxyPinned.sol @@ -1,10 +1,11 @@ pragma solidity 0.4.18; import "../common/UnstructuredStorage.sol"; +import "../common/IsContract.sol"; import "./AppProxyBase.sol"; -contract AppProxyPinned is AppProxyBase { +contract AppProxyPinned is IsContract, AppProxyBase { using UnstructuredStorage for bytes32; // keccak256("aragonOS.appStorage.pinnedCode"), used by Proxy Pinned @@ -21,7 +22,7 @@ contract AppProxyPinned is AppProxyBase { public // solium-disable-line visibility-first { setPinnedCode(getAppBase(_appId)); - require(pinnedCode() != address(0)); + require(isContract(pinnedCode())); } /** diff --git a/test/app_proxy.js b/test/app_proxy.js index 7bde904dd..81317d87b 100644 --- a/test/app_proxy.js +++ b/test/app_proxy.js @@ -13,6 +13,7 @@ const AppProxyPinned = artifacts.require('AppProxyPinned') const AppStub = artifacts.require('AppStub') const AppStub2 = artifacts.require('AppStub2') const ERCProxyMock = artifacts.require('ERCProxyMock') +const KernelSetAppMock = artifacts.require('KernelSetAppMock') const APP_ID = hash('stub.aragonpm.test') const ZERO_ADDR = '0x0000000000000000000000000000000000000000' @@ -109,19 +110,25 @@ contract('App proxy', accounts => { }) onlyAppProxyPinned(() => { + const FAKE_APP_ID = hash('fake.aragonpm.test') + it("fails if code hasn't been set yet", async () => { - const fakeAppId = hash('fake.aragonpm.test') - return assertRevert(async () => { - await AppProxyPinned.new(kernel.address, fakeAppId, EMPTY_BYTES) + await assertRevert(async () => { + await AppProxyPinned.new(kernel.address, FAKE_APP_ID, EMPTY_BYTES) }) }) - }) - context('> Fails on bad kernel', () => { - beforeEach(async () => { - await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase1.address) + it("fails if code set isn't a contract", async () => { + const kernelMock = await KernelSetAppMock.new() + await kernelMock.setApp(APP_BASES_NAMESPACE, FAKE_APP_ID, '0x1234') + + await assertRevert(async () => { + await AppProxyPinned.new(kernelMock.address, FAKE_APP_ID, EMPTY_BYTES) + }) }) + }) + context('> Fails on bad kernel', () => { it('fails if kernel address is 0', async () => { return assertRevert(async () => { await appProxyContract.new(ZERO_ADDR, APP_ID, EMPTY_BYTES) diff --git a/test/keccak_constants.js b/test/keccak_constants.js index ddead34f4..e5720532e 100644 --- a/test/keccak_constants.js +++ b/test/keccak_constants.js @@ -93,7 +93,10 @@ contract('Constants', accounts => { }) it('checks AppProxyPinned unstructured storage constants', async () => { - const app = await getContract('AppStubPinnedStorage').new() + // Set up AppStubPinnedStorage + const fakeApp = await getContract('AppStub').new() + const kernelMock = await getContract('KernelPinnedStorageMock').new(fakeApp.address) + const app = await getContract('AppStubPinnedStorage').new(kernelMock.address) assert.equal(await app.getPinnedCodePosition(), await keccakConstants.pinnedCodePosition(), "pinnedCodePosition doesn't match") }) diff --git a/test/mocks/AppStubPinnedStorage.sol b/test/mocks/AppStubPinnedStorage.sol index 6dde0f81d..0126adf0c 100644 --- a/test/mocks/AppStubPinnedStorage.sol +++ b/test/mocks/AppStubPinnedStorage.sol @@ -2,21 +2,29 @@ pragma solidity 0.4.18; import "../../contracts/apps/AppProxyPinned.sol"; import "../../contracts/kernel/IKernel.sol"; +import "../../contracts/kernel/Kernel.sol"; -contract AppStubPinnedStorage is AppProxyPinned { +contract FakeAppConstants { bytes32 constant FAKE_APP_ID = keccak256('FAKE_APP_ID'); - address constant FAKE_APP_ADDR = address(1); +} + +contract KernelPinnedStorageMock is Kernel, FakeAppConstants { + bytes32 constant FAKE_APP_ID = keccak256('FAKE_APP_ID'); + function KernelPinnedStorageMock(address _fakeApp) Kernel(false) public { + _setApp(APP_BASES_NAMESPACE, FAKE_APP_ID, _fakeApp); + } +} - // Allow the mock to be created without any arguments - function AppStubPinnedStorage() - AppProxyPinned(IKernel(0), FAKE_APP_ID, new bytes(0)) - public // solium-disable-line visibility-first - {} - // Overload base to return our own fake address - function getAppBase(bytes32 _appId) internal view returns (address) { - return FAKE_APP_ADDR; +// Testing this contract is a bit of a pain... we can't overload anything to make the contract check +// pass in the constructor, so we're forced to initialize this with a mocked Kernel that already +// sets a contract for the fake app. +contract AppStubPinnedStorage is AppProxyPinned, FakeAppConstants { + function AppStubPinnedStorage(KernelPinnedStorageMock _mockKernel) + AppProxyPinned(IKernel(_mockKernel), FAKE_APP_ID, new bytes(0)) + public // solium-disable-line visibility-first + { } function setPinnedCodeExt(address _pinnedCode) public { diff --git a/test/mocks/KernelSetAppMock.sol b/test/mocks/KernelSetAppMock.sol new file mode 100644 index 000000000..e2297837d --- /dev/null +++ b/test/mocks/KernelSetAppMock.sol @@ -0,0 +1,13 @@ +pragma solidity 0.4.18; + +import "../../contracts/kernel/Kernel.sol"; + +contract KernelSetAppMock is Kernel { + function KernelSetAppMock() Kernel(false) public { + } + + function setApp(bytes32 _namespace, bytes32 _name, address _app) public returns (bytes32 id) { + id = keccak256(_namespace, _name); + apps[id] = _app; + } +} diff --git a/test/unstructured_storage.js b/test/unstructured_storage.js index d760cbcea..823158469 100644 --- a/test/unstructured_storage.js +++ b/test/unstructured_storage.js @@ -1,14 +1,22 @@ +const AppStub = artifacts.require('AppStub') const AppStubStorage = artifacts.require('AppStubStorage') const AppStubPinnedStorage = artifacts.require('AppStubPinnedStorage') const Kernel = artifacts.require('Kernel') +// Mocks +const KernelPinnedStorageMock = artifacts.require('KernelPinnedStorageMock') + contract('Unstructured storage', accounts => { let app, kernel beforeEach(async () => { app = await AppStubStorage.new() - appPinned = await AppStubPinnedStorage.new() kernel = await Kernel.new(true) + + // Set up AppStubPinnedStorage + const fakeApp = await AppStub.new() + const kernelMock = await KernelPinnedStorageMock.new(fakeApp.address) + appPinned = await AppStubPinnedStorage.new(kernelMock.address) }) it('tests init block', async () => {