Skip to content

Commit 5ee26b4

Browse files
GeObtsclaude
andcommitted
feat(manager): add _initialOwner constructor param for CREATE2 deploys
VRFConsumerBaseV2Plus hard-wires Chainlink's ConfirmedOwner to set s_owner = msg.sender at construction. With Arachnid's deterministic CREATE2 deployer (msg.sender = 0x4e59...), the Manager would be permanently un-owned — every administrative function (setTreasury, allowlistToken, pause, etc.) uncallable. This blocks any cross-chain deterministic deploy strategy. Fix: add _initialOwner as the last constructor param. After the parent constructors run (VRFConsumerBaseV2Plus sets s_owner = msg.sender), call transferOwnership(_initialOwner) to propose ownership to the intended EOA. Two-step transfer (the only available path — Chainlink's _transferOwnership is private, not internal). Deploy scripts finalize via acceptOwnership() from the EOA. Skipped when _initialOwner == msg.sender (Chainlink reverts on self-transfer); preserves in-test ownership semantics where the test contract deploys and owns directly. Test base updated to pass address(this) as _initialOwner. All 85 tests remain green; zero behavioral changes for non-CREATE2 deploy paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 71b4410 commit 5ee26b4

2 files changed

Lines changed: 32 additions & 6 deletions

File tree

src/TandaManager.sol

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,18 @@ contract TandaManager is VRFConsumerBaseV2Plus, Pausable {
249249
// ─────────────────────────────────────────────────────────────────────
250250

251251
/// @notice Deploys the Manager as the singleton orchestrator.
252-
/// @dev `VRFConsumerBaseV2Plus` hard-wires the owner to `msg.sender`
253-
/// via `ConfirmedOwner(msg.sender)`. To run with a different
254-
/// initial owner, call `transferOwnership` immediately
255-
/// post-deploy (Chainlink's two-step transfer).
252+
/// @dev `VRFConsumerBaseV2Plus` hard-wires `s_owner = msg.sender`
253+
/// via `ConfirmedOwner(msg.sender)`. For CREATE2 deploys
254+
/// (where `msg.sender` is the deterministic deployer
255+
/// contract rather than the operator EOA) we propose
256+
/// ownership to `_initialOwner` here so the deploy script
257+
/// can finalize via `acceptOwnership()` from that wallet.
258+
/// Chainlink's two-step transfer is the only path
259+
/// available — `_transferOwnership` in
260+
/// `ConfirmedOwnerWithProposal` is `private`. The transfer
261+
/// is skipped when `_initialOwner == msg.sender` (e.g. tests
262+
/// deploying directly), so the in-test ownership semantics
263+
/// are unchanged.
256264
/// @param _tandaImplementation EIP-1167 implementation that
257265
/// `createTanda` will clone. Must already
258266
/// be deployed and verified on this chain.
@@ -269,6 +277,10 @@ contract TandaManager is VRFConsumerBaseV2Plus, Pausable {
269277
/// address. Immutable.
270278
/// @param _completionNFT Soulbound Completion NFT contract
271279
/// address. Immutable.
280+
/// @param _initialOwner Wallet that will own the Manager after
281+
/// `acceptOwnership()`. Required nonzero.
282+
/// When equal to `msg.sender` the transfer
283+
/// is skipped (no pending-owner window).
272284
constructor(
273285
address _tandaImplementation,
274286
address _vrfCoordinator,
@@ -278,14 +290,16 @@ contract TandaManager is VRFConsumerBaseV2Plus, Pausable {
278290
address _treasury,
279291
address _passNFT,
280292
address _receiptNFT,
281-
address _completionNFT
293+
address _completionNFT,
294+
address _initialOwner
282295
) VRFConsumerBaseV2Plus(_vrfCoordinator) {
283296
if (_tandaImplementation == address(0)) revert ZeroAddress();
284297
if (_treasury == address(0)) revert ZeroAddress();
285298
if (_callbackGasLimit == 0) revert ZeroAmount();
286299
if (_passNFT == address(0)) revert ZeroAddress();
287300
if (_receiptNFT == address(0)) revert ZeroAddress();
288301
if (_completionNFT == address(0)) revert ZeroAddress();
302+
if (_initialOwner == address(0)) revert ZeroAddress();
289303
// _vrfCoordinator zero-check is handled by VRFConsumerBaseV2Plus.
290304

291305
tandaImplementation = _tandaImplementation;
@@ -303,6 +317,13 @@ contract TandaManager is VRFConsumerBaseV2Plus, Pausable {
303317

304318
nextTandaId = 1;
305319
nextCollectionId = 1;
320+
321+
// Two-step ownership handoff for CREATE2 deploys. Skipped when
322+
// the deployer wallet is also the intended owner — Chainlink's
323+
// `_transferOwnership` reverts on self-transfer.
324+
if (_initialOwner != msg.sender) {
325+
transferOwnership(_initialOwner);
326+
}
306327
}
307328

308329
// ─────────────────────────────────────────────────────────────────────

test/helpers/MitandaTestBase.sol

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ abstract contract MitandaTestBase is Test {
9292

9393
// 7. Deploy Manager. Its actual address MUST match the prediction
9494
// or every NFT call from any tanda will fail forever.
95+
// _initialOwner = address(this): test contract is both deployer
96+
// and intended owner, so the constructor skips transferOwnership
97+
// (Chainlink reverts on self-transfer). Same effective behavior
98+
// as the pre-_initialOwner version.
9599
manager = new TandaManager(
96100
address(tandaImpl),
97101
address(vrfCoordinator),
@@ -101,7 +105,8 @@ abstract contract MitandaTestBase is Test {
101105
TREASURY,
102106
address(passNFT),
103107
address(receiptNFT),
104-
address(completionNFT)
108+
address(completionNFT),
109+
address(this)
105110
);
106111
require(address(manager) == predictedManager, "MitandaTestBase: Manager address prediction failed");
107112

0 commit comments

Comments
 (0)