VRFRecovery, VRFRecoveryAdditional and VRFSafetyProperties all pass on the current code because they only document the current behaviour; none of them fails if a token can never be revealed.

Missing coverage: (a) a fulfilment where the callback fails (the Chainlink mock emits success=false and deletes the request) and the resulting permanent AwaitingRandomness state; (b) a request older than Chainlink's 24h window and any recovery path; (c) a fulfilment delivered at exactly the minimum callbackGasLimit of 150,000 through VRFCoordinatorV2_5Mock (only the project's own mock at 95k is tested); (d) ordering fuzz over more than three concurrently pending tokens showing outcomes are independent of delivery order; (e) BMCMainnetFork.t.sol, which needs an RPC and was not run here, so burnFrom against the real BMC token remains unverified; (f) nothing checks the Chainlink mock's unfunded-at-request path together with later funding beyond one token.

Read in full: src/BMCIlluminatiPass.sol, src/StandardArt.sol, src/interfaces/IBMCToken.sol, src/experimental/BMCPassVRFPrototype.sol, script/Deploy.s.sol, all eight test files and both mocks, the vendored VRFCoordinatorV2_5Mock and SubscriptionAPI, README and docs.

Not reached: the live BMC token and the live VRF coordinator (no fork RPC; BMCMainnetFork.t.sol not run), and the full VRFCoordinatorV2_5 source (not vendored; behaviour taken from Chainlink docs and the vendored mock). All 55 existing tests pass; runtime size 23,488 bytes; no DELEGATECALL/CALLCODE/SELFDESTRUCT, no proxy, no receive/fallback.

Entry points: setGenesisImagesCID (owner, once), setVRFConfig (owner, unbounded in time), mintGenesis/mintMystery (anyone paying), rawFulfillRandomWords (immutable coordinator only), finalizeReveals (anyone), standard ERC721/Ownable. No reroll, owner-assignment, selective-fulfilment or reentrancy path was found in A: the word is bound to the token before any reveal, re-delivery is ignored, in-order processing removes finalization-order choice, and _burnExact fails closed.

ARCHITECTURE A verdict: cannot safely handle an expired or permanently unavailable request. One lost request freezes that token and all later ones permanently with no recovery (high finding); the owner/subscription owner can cause it by inaction; odds drift with timing.

ARCHITECTURE B verdict: the prototype is not an architecture yet (pure function, no request, no mint, no state).

As a design it is fairer (flat 37/460 odds, one request to fund and monitor, no ordering question) but it is only safe if (1) the seed is requested after minting is closed and never before, (2) a permissionless timed re-request exists with first-delivered-word-wins, and (3) the sale close is itself rule-based (sell-out or a deadline), so neither of the two existing designs satisfies the requirements as written.

RECOMMENDATION: keep BMCIlluminatiPass's mint, burn, pricing, 40 sequential Genesis, storage and metadata, and replace the per-mint request with the single-seed model: mintMystery assigns position only; minting closes at 500 or at a fixed deadline (permissionless close); after close anyone may call requestSeed(); if no word has arrived after a timeout of at least 7 days anyone may call requestSeed() again; rawFulfillRandomWords stores the first word for any request id the contract issued and ignores later ones; outcomes come from the existing O(1) virtual Fisher-Yates driven by keccak256(seed, tokenId) in mint order via the existing permissionless finalizeReveals, so no 500k-gas permutation is needed and all existing invariant tests still apply.

If per-mint requests must stay, the minimal fix is: a permissionless re-request for a token whose request is older than a timeout, first-delivered-wins (never invalidate an open request, to block mempool rerolls), plus the same strict order.

RESIDUAL RISKS of the recommended design: Chainlink node operators know the word before submitting and, with two open requests, could choose which to deliver (mitigated by the long timeout); the reveal of all 460 happens only after close, so early minters wait longer; the subscription must stay funded for one request; the owner's setVRFConfig power remains a trust assumption.

MAINNET READINESS: not ready.

Blocking: the high finding, an expiry/re-request path, constructor-time configuration for the launch, a corrected README, and tests that fail on the freeze.

Also pending: the real-BMC burnFrom fork check (O-3) and an external gas measurement under the real coordinator.

Area: Numerical Gap (boundary x invariant) and Boundary. The virtual Fisher-Yates in _finalize is arithmetically correct (uniform permutation, no modulo bias beyond 460/2^256, no over/underflow: remaining is in [1,460] because tokenId = 41 + revealedCount <= 500). The fairness invariant the brief asks about ('no rarity bias') holds only ex ante.

Because (a) every token's draw is taken from the pool that remains after all earlier positions and (b) every delivered word is emitted in RandomnessStored and every reveal in Revealed, the conditional Genesis probability of the NEXT mint position is a public quantity that drifts with state and reaches 0 or 1 at the pool boundary, while mintMystery charges the same 350,000 BMC.

Concrete numbers: ex-ante 37/460 = 8.04%; at revealedCount=300 with 10 Genesis revealed 27/160 = 16.9%; with 30 revealed 7/160 = 4.4%; at revealedCount=423 with 0 Genesis revealed every one of positions 464..500 is a certain Genesis; at revealedCount=k with 37 Genesis revealed every later position is a certain Standard at full price.

Sharper version: a RandomnessStored token whose predecessor is still AwaitingRandomness (lost request) already has a publicly computable outcome.

With no reveals yet, token 42 with stored word w42 draws index idx42 = keccak256(abi.encode(w42,42)) % 459 and receives value idx42 unless token 41 draws exactly slot idx42 (probability 1/460), so its tier is known with probability 459/460 before 41 has any randomness, while remainingMysteryGenesis() still reports 37 ('includes outcomes owed to pending tokens').

A buyer deciding whether to mint position 43 therefore knows the effective pool better than the view advertises, and buyers in general can time purchases to favourable states; the project cannot. This is inherent to architecture A (per-mint words are public events, so deferring finalization hides nothing), is disclosed in README 'Odds', and was rated Low by the previous audit (aa16c116).

It is reported here because the assignment asks explicitly whether either architecture is free of rarity bias: A is not; only a single seed requested after sale close (architecture B, with the timed re-request from the test consumer) gives flat 37/460 odds for every position.

Recommended: keep A only with this disclosure in the mint UI (show the live conditional odds, not 37/460), or adopt B with a rule-based close. No code defect in the arithmetic itself.

Area: Boundary / Numerical Gap (boundary x precision). The first reveal in a callback is most expensive when its outcome is Genesis: mysteryGenesisRevealed goes 0 -> 1 (cold SLOAD 2,100 + SSTORE 20,000) on top of revealedCount 0 -> 1, _tier and _artworkId 0 -> nonzero and the cold _slotSwap write.

Measured through finalizeReveals(1) on this tree: Genesis first reveal 151,309 gas, Genesis second reveal 117,109 gas (the project's test_GasPerRevealBudgetCoversMeasuredCost measures 129,539 / 112,439 for its Standard words and asserts < 140,000, so the assertion would fail for a Genesis first word). The constant's own comment ('worst case ... about 134k') is therefore inaccurate by about 17k.

Impact check at every batch threshold: batch = (gasAvailable - 30,000) / 140,000 and the inner call receives about 63/64 of gasAvailable, so forwarded gas at the thresholds is about 166k / 304k / 442k / 580k for batches 1..4 while the worst-case cost is about 151k / 268k / 386k / 503k; a scan of callback gas 190k..240k in 2k steps shows the inline Genesis reveal first attempted at 204k and succeeding there (used 188,583), so no window exists where a batch is attempted and reverts, and even a revert would be caught by try/catch with the word kept.

Nothing is lost; this is a documentation and test-coverage gap only.

Recommended: raise GAS_PER_REVEAL to about 155,000 (or correct the comment to state the real margin source: CALLBACK_GAS_RESERVE plus the 63/64 rule) and add a Genesis-first word to test_GasPerRevealBudgetCoversMeasuredCost.

Architecture A recovers a lost request only through rerequestRandomness, which is gated by block.timestamp < dueAt (src/BMCIlluminatiPass.sol:295-296) with dueAt = lastRequestedAt + 7 days. Because finalizeReveals (line 370-379) processes tokens strictly in mint order, one token without a word holds every later token in RandomnessStored for the whole window.

The window is sized for the case the chain cannot observe (Chainlink dropping an under-funded request after ~24h). But the second loss mode the contract itself documents (lines 38-40), a callback that ran out of gas, is final the moment the coordinator marks the request fulfilled: the request can never deliver, yet the contract still forces a 7-day wait on the affected token and, through strict order, on every token minted after it.

In a 460-mint sale that reveals 'minutes after each mint' (README), one such event converts the reveal of every later holder into a >=7-day stall, and a second loss while the queue is stalled compounds it. This is a liveness degradation of a stated guarantee, not a loss of funds or a reroll, so it is reported as low.

Recommended: (1) shorten REREQUEST_DELAY to about 48 hours, still 2x Chainlink's 24h drop window, so the probability that two requests are simultaneously live stays negligible while the worst-case stall drops from 7 days to 2; and/or (2) treat a request the coordinator has already consumed as immediately replaceable (the v2.5 coordinator deletes the request commitment on fulfilment; if the deployed coordinator exposes s_requestCommitments(requestId), a zero value proves the old request can never deliver and rerequestRandomness can skip the timer for that token without weakening first-delivered-word-wins).

Either change preserves all required constants, prices and burning.

The contract inherits OpenZeppelin Ownable (single-step transferOwnership(address) and renounceOwnership(); lib/openzeppelin-contracts/contracts/access/Ownable.sol:76-84).

The owner's only power is setVRFConfig (line 218-226), but that power is the project's sole response to a permanently unavailable VRF path: if Chainlink retires the configured key hash or the subscription becomes unusable, every pending mystery token depends on the owner pointing setVRFConfig at a working lane before rerequestRandomness can succeed (README 'Unavailable randomness', docs/VRF-ARCHITECTURE-REVIEW.md section 4). Ownable2Step (also vendored in lib/openzeppelin-contracts) would require the new owner to accept before control moves, removing the lost-key failure mode at no cost to the agreed design.

The README discloses that renouncing freezes the config; the mistaken-transfer case is not disclosed.

Reported as informational: it requires an owner mistake and has no unprivileged amplifier, but it sits on the only recovery path for the 'permanently unavailable VRF' question the brief asks about.

draw(uint256 seed) is a pure function of a single 256-bit word, so it can produce at most 2^256 distinct permutations, while 460! is about 2^3300; the docstring overstates the reachable permutation space by a factor of roughly 2^3000.

Each position's marginal distribution is still uniform (each keccak256(seed, i) % remaining index is uniform up to a 2^-247 modulo bias), so this is a documentation error rather than a bias, but a reviewer reading the claim as a fairness argument would be misled.

Separately, the measured cost of draw is 497,723 gas and it returns 460 outcomes in memory; no production path can persist them (460 cold SSTOREs alone exceed 9M gas), so the only viable B design is the per-token derivation the review document already describes, and the only B consumer in the repository (ExperimentalVRFConsumer in test/BMCPassVRFChainlinkIntegration.t.sol) is test-only with a permissionless closeMinting().

Conclusion for the brief's question: B can handle a lost seed request only with the same timed, first-delivered-wins re-request as A, and it additionally needs a rule-based sale close that the brief does not define; it is not a safer design to adopt as it stands.

The suites assert the right behaviour wherever they look, but three edges of the recovery design are unexercised. (a) rerequestRandomness for a token that is RandomnessStored (word delivered, blocked behind an earlier token) is never asserted to revert NotAwaitingRandomness; the existing test (test/VRFRecovery.t.sol:186-203) only covers a token that was finalized inline.

(b) test_GasPerRevealBudgetCoversMeasuredCost measures finalizeReveals(1) from an EOA call after the words were stored with 95k gas, so the measured 129,539 gas does not include the state the first inline reveal actually sees inside rawFulfillRandomWords (the 63/64 forwarding rule on this.finalizeReveals, and the one-time 0->1 transitions of revealedCount and mysteryGenesisRevealed on a Genesis first outcome).

The design is safe either way because a failed batch is caught and the word kept (lines 355-363), but the README's claim that a 500k callback finalizes three reveals is asserted only through the project's own mock. (c) No test takes a token through two consecutive losses (original and replacement both fail their callback) to a third request that is fulfilled; testRerequestRateLimitedPerToken opens three requests but never delivers.

The scratch probe in test/scratch/ReviewProbe.t.sol covers (a) and the mempool-relay interleaving (a fulfilment relayed from inside the minter's own onERC721Received), both of which behave correctly.

Seam execution x periphery x first principles. The contract's stated guarantee is that a wrong assumption "fails closed rather than mints" (README, pre-deployment checks) and that "a bad config blocks new mystery mints and re-requests until corrected" (README.md:91).

The burn-mismatch and decimals guards do fail closed, but the VRF leg does not: the real VRFCoordinatorV2_5.requestRandomWords (v1.4.0 lines 255-290) checks only consumer membership, confirmations, callback gas and numWords. It does not check the subscription balance and explicitly does not check the key hash.

Chainlink's own VRFCoordinatorV2_5Mock vendored in lib/ behaves the same way, and the project's test test_UnfundedSubscriptionKeepsTokenPendingUntilFunded already shows the unfunded case. So _burnExact destroys 350,000 BMC and then the request is accepted even when (a) the subscription balance is 0 (the node retries for about 24h and drops the request), or (b) the owner typed a key hash that no node serves.

In both cases the buyer's BMC is gone, the token sits in AwaitingRandomness, the earliest remedy is a permissionless re-request after 7 days, and that remedy only works if a third party funds the subscription or the owner fixes the key hash.

Economic consequence: the sale's liveness rests entirely on an off-chain funding duty with no on-chain guard; a buyer cannot tell from the contract whether their reveal is servable.

Fix that preserves the design: read VRF_COORDINATOR.getSubscription(cfg.subscriptionId) (IVRFSubscriptionV2Plus, vendored) in _requestRandomWord and revert mintMystery when the balance in the configured payment asset (balance for LINK, nativeBalance for ETH) is below a constructor-set floor, so the burn rolls back exactly as it does today when the pass is not a consumer.

The key-hash case cannot be checked on-chain; the README sentence must be corrected and the website should display the configured lane. Expose a subscriptionCanPay() view for the website either way.

The recovery design assumes requests are sometimes lost (24h unfunded expiry or a failed callback) and replaces them with rerequestRandomness.

On the real VRFCoordinatorV2_5 (v1.4.0) requestRandomWords does ++consumerConfig.pendingReqCount (line 286) and only fulfillRandomWords does --s_consumers[rc.sender][rc.subId].pendingReqCount (line 487). pendingRequestExists(subId) returns true while any consumer's count is above zero (lines 638-647), and both removeConsumer (line 652) and cancelSubscription (line 678) revert with PendingRequestExists while it is true.

A request that a node dropped is never fulfilled, so the counter never returns to zero: after the first lost request the subscription owner can never cancel the subscription or remove the pass, and whatever LINK or ETH remains after the sale is stranded (only Chainlink's coordinator owner can release it via ownerCancelSubscription).

The README tells the owner to keep the subscription funded for 460 requests plus re-requests, which encourages a large up-front balance that the design's own failure mode then locks. This is an economic consequence for the subscription owner, not the pass contract, and it is not a code defect in the pass; it belongs in the operational guidance and the mainnet readiness list.

Not a new defect: the prior audit reported it (aa16c116) and the project disclosed it in the README ("Odds") and chose architecture A anyway. Recorded here because the brief's fairness deliverable needs the numbers and because it is the main economic argument for architecture B.

In A the draw for the next finalized token is keccak256(word, tokenId) % remaining over the live pool, so the conditional Genesis probability at any moment is remainingMysteryGenesis() / (460 - revealedCount), public on-chain. Early buyers take the ex-ante 37/460 = 8.04% with no information; a late buyer holds a free option to buy only when the remainder is rich.

In the extreme state of 300 Standard reveals and 0 Genesis the figure is 37/160 = 23.1%, and a buyer who takes the whole remaining tail receives exactly all 37 Genesis for 160 x 350,000 = 56,000,000 BMC while the 300 early buyers burned 105,000,000 BMC for none.

The information is symmetric, so no buyer is deceived, but the ex-ante symmetry the README implies ("every mystery mint pays the same price") hides that the option value is not symmetric: it accrues to whoever buys last. The mirror state (Genesis-heavy early run) leaves a tail nobody rational buys, which is a sell-out risk for the project.

Architecture B (one seed requested after a rule-based close) has none of this: every position is exactly 37/460 and no state is observable before the close. If the requester wants A's instant reveals, the honest disclosure is the drift formula on the website at mint time (the website already shows provisional odds); if the requester wants fairness across positions, B with a close rule is the only option.

Deliverable 1 (security comparison) and the brief's "selective fulfilment" criterion. Both architectures already accept that the VRF node knows a word before submitting and can withhold. The review's section 4 correctly lists, for A, that after a 7-day wait two requests are open and the node "could choose which to deliver" for ONE token.

The same timed re-request is then recommended for B (docs/VRF-ARCHITECTURE-REVIEW.md:97-99: "after 7 days without a word anyone may call it again; the first word delivered for any issued request id is final") and the test consumer implements it (test/BMCPassVRFChainlinkIntegration.t.sol:35-52, 54-61).

In B the seed determines all 460 outcomes, so a node that withholds the first seed for 7 days and then sees the second one can pick whichever of two full permutations it prefers (for example the one that puts Genesis on positions it or a bribing holder owns), and testFirstDeliveredSeedWinsWhenOriginalArrivesLate shows the contract will accept either.

The comparison table row for B therefore understates the node trust required: the blast radius of node selectivity in B is the whole collection, in A one token per lost request. This is a design observation, not a code defect (B has no production consumer).

It should be weighed in deliverable 4 if B is ever adopted: a commit-reveal of the seed by the project is not a fix (the project would then hold the reroll), so B needs either no re-request at all (accepting a permanent freeze on a lost seed) or a re-request that becomes possible only after a much longer silence than Chainlink's 24h drop window and is announced on-chain so the delay itself is evidence of withholding.

Economics x asymmetry. Outcomes are drawn without replacement and finalized during the sale, so the conditional probability that the next mint is Genesis equals remainingMysteryGenesis() / (460 - revealedCount) (both public; remainingMysteryGenesis() at line 461).

Because every delivered word is also emitted (RandomnessStored, line 353) and finalization is strict mint-order, the pool state after all stored-but-unfinalized tokens is computable too, so the odds for the next position are known almost exactly even while the queue is blocked.

Every minter pays the same 350,000 BMC, but a minter who reads this state mints only when the conditional odds are above the ex-ante 37/460 = 8.0% and abstains when they are below; the uninformed minters absorb the low-odds positions. Concrete drift already acknowledged in docs/VRF-ARCHITECTURE-REVIEW.md section 3: at 300 revealed, 27 Genesis left gives 27/160 = 16.9%, 7 left gives 7/160 = 4.4%.

The bias is per position, not in the total (always exactly 37 Genesis), and the prior audit recorded it (aa16c116) and the README discloses it. It is reported here because the assignment asks for a rarity-bias determination: architecture A cannot remove it (the words are public), architecture B removes it only if the single seed is requested after a rule-based sale close.

Recommendation: keep the disclosure, expose the conditional odds on-chain (a view returning numerator/denominator) so the information is equally available to everyone, and show it on the mint site; or move to B with a fixed close rule if flat 37/460 odds per position are a product requirement.

Asymmetry between the writer and the reader of _revealStatus: rawFulfillRandomWords moves a token to RandomnessStored and emits its word (lines 351-353), but tokenURI (lines 499-511) branches only on _tier == Pending and serves the same 'awaiting Chainlink VRF randomness ... not yet determined on-chain' text for both AwaitingRandomness and RandomnessStored.

Under strict mint-order finalization a token can sit in RandomnessStored for at least 7 days whenever an earlier token's request is lost (the recovery path requires REREQUEST_DELAY), and for shorter windows whenever more than three deliveries land before the queue drains.

During that window the holder (or anyone reading events) computes keccak256(abi.encode(word, tokenId)) % remaining against the public virtual pool: if the draw index is >= 37 the token is Standard with certainty regardless of what the blocked predecessor later draws; if it is < 37 it is Genesis unless the predecessor collides on the same index (1/remaining).

The token still trades on marketplaces as a 'Pending Reveal' with the ex-ante odds in its metadata, so an informed seller can offload a known-Standard token to a buyer who trusts the on-chain description. This is an information asymmetry the contract itself creates by describing determined state as undetermined.

Fix (no design change): in tokenURI, when _revealStatus[tokenId] == RandomnessStored, say so (e.g. 'randomness delivered, awaiting in-order finalization') and/or add a view provisionalOutcome(tokenId) that returns the tier the stored word will produce given the current pool (with a flag when it depends on an unresolved predecessor), so buyers and sellers see the same thing.

Access-control hygiene. The contract inherits OpenZeppelin Ownable (single-step transferOwnership, immediate renounceOwnership, lib/openzeppelin-contracts/contracts/access/Ownable.sol:76-89).

The owner's only power, setVRFConfig, is also the only on-chain way to keep mintMystery and rerequestRandomness working when the subscription or gas lane changes: the subscription owner can cancel the subscription once no request is pending (Chainlink v2.5 cancelSubscription), Chainlink can deregister a key hash, or the project may need to move to a new funded subscription.

A typo in transferOwnership (ownership to an address nobody controls) or a renounceOwnership call (README line 94 records that it 'freezes the VRF config forever') therefore turns any later config problem into a permanent stall of the remaining mystery sale and of every token whose request is later lost.

OpenZeppelin Ownable2Step preserves the agreed design (same single owner, same power) and removes the mistake path; renounceOwnership can be overridden to revert if the requester wants the config lever to be permanent. This is a trust-assumption hardening, not a bypass.

Every mystery mint draws without replacement from one shared pool of 37 Genesis and 423 Standard outcomes, and reveals happen per mint. The chance that the next mint is Genesis therefore equals remainingMysteryGenesis() / (460 - revealedCount), a public value that drifts away from the ex-ante 37/460 as reveals land. In the extreme the pool runs out of one tier and a 350,000 BMC mint becomes a guaranteed Standard (or a guaranteed Genesis) while the price stays the same.

Informed buyers can time their mints; uninformed buyers bear the complement. This is structural to per-mint reveals over a finite pool and is already disclosed in the README and the previous review (prior audit id aa16c116); it cannot be removed inside architecture A without deferring all reveals to the end of the sale (which is architecture B) or giving up the exact 37/423 inventory.

Reported so the requester decides on disclosure or a design change rather than having a test bless it.