solidstate-network/solidstate-solidity

bug: SolidStateDiamond transferOwnership doesn't transfer the ownership

iam-dev opened this issue · 2 comments

SolidStateDiamond usage :

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.4;

import {SolidStateDiamond} from "@solidstate/contracts/proxy/diamond/SolidStateDiamond.sol";

contract SimplicyWalletDiamond is SolidStateDiamond {
}

Test file

describe("::SimplicyWalletDiamond", function () {
    it("should call functions through diamond address", async function () {
      expect(await walletDiamond.owner()).to.equal(this.deployer.address);
    });
    describe("#transferOwnership", function () {
      it("should transfer ownership", async function () {
        await walletDiamond.transferOwnership(newOwner.address);
        expect(await walletDiamond.owner()).to.equal(newOwner.address);
      });
    });
  });

Error:

1) WalletFactoryFacet
       ::SimplicyWalletDiamond
         #transferOwnership
           should transfer ownership:

      AssertionError: expected '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb…' to equal '0x70997970C51812dc3A010C7d01b50e0d17d…'
      + expected - actual

      -0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
      +0x70997970C51812dc3A010C7d01b50e0d17dc79C8

SafeOwnable uses transferOwnership like this:

function _transferOwnership(address account) internal virtual override {
SafeOwnableStorage.layout().setNomineeOwner(account);
}

The purpose of the SafeOwnable suite is to ensure a user accepts ownership before ownership is transferred.

Check nomineeOwner instead, or accept the transfer from the newOwner account.

Please update on whether this solves the issue.

You may also use @solidstate/spec; this will test all key functionality in the contracts which inherit from SolidStateDiamond thus ensuring no functionality is broken.