bug: SolidStateDiamond transferOwnership doesn't transfer the ownership
iam-dev opened this issue · 2 comments
iam-dev commented
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
NouDaimon commented
SafeOwnable
uses transferOwnership
like this:
solidstate-solidity/contracts/access/ownable/SafeOwnableInternal.sol
Lines 42 to 44 in 55e7948
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.
NouDaimon commented
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.