_transferOwnership() is an effective bypass of tx.origin

[defido]

To bypass the security or 'bad practice' of using tx.origin, _transferOwnership() is effective when using OZ Ownable. You can use it in the constructor setup effectively, before adding other logic, such as transaction approvals etc.

constructor( string memory name, string memory symbol, address payable setup_wallet, ) { _name = name_; _symbol = symbol_; _transferOwnership(setup_wallet) }

[defido]

Just a minor notice, feel free to ignore, I've tested it and it's far simpler than changing the Ownable, and safer than tx.origin (I think).

[pcaversaccio]

@defido thanks for raising this point. The reason why this can be done is due to the linearization pattern of multiple inheritance in Solidity (see here). Your pattern will emit two events since the first _transferOwnership call within the constructor of the Ownable contract will be triggered in any case.

The reason I don't want to have specific workarounds in the README is that this can pile up really fast due to the various libraries used out there (OZ is just one of multiple libraries). What I however would like to do is that you open a discussion here on this topic and share your input regarding OZ (also link to this PR please). Like that other people can share other approaches. Also, FYI, your recommendation could become obsolete with OZ contracts v5 where they plan to remove this first _transferOwnership call within the constructor of the Ownable contract.

I will link to the discussion thread in the README.

[basememara]

@defido your instinct of tx.origin not being safe was correct. Under the context of Account Abstraction, smart contracts have the ability to issue transactions on behalf of a user. See the section Security Considerations for Developers in this OpenZepplin post regarding how tx.origin may not be what's intended for that scenario.

If you're running xdeployer from the terminal, you should be ok (I think) since it's using the private key in the config. However, note that if you deploy using EIP-4337, then assumptions change.

[basememara]

OpenZepplin v5 was just released and can now pass in initialOwner 🎉

https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4267/files#diff-12d832c90ed93f8f3de2bdb5d70c7f18a00a7ee6ab685510c2f4c88510d32180