Skip to content

Commit 940c6f6

Browse files
committed
test(token): fold M-04 tests into _approve
Review feedback: drop the `audit M-04` describe block and the before/after-fix narration. The three regression tests move into the existing `_approve` block with self-describing names, and the comments that survive describe current behaviour only. Traceability for the finding stays in the fix commit and the PR body.
1 parent b2816f4 commit 940c6f6

1 file changed

Lines changed: 7 additions & 31 deletions

File tree

contracts/src/token/test/nonFungibleToken.test.ts

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -816,22 +816,8 @@ describe('NonFungibleToken', () => {
816816
await token._approve(ZERO_ACCOUNT, TOKENID_1, OWNER.either);
817817
expect(await token.getApproved(TOKENID_1)).toEqual(ZERO_ACCOUNT);
818818
});
819-
});
820819

821-
// Audit finding M-04 (Midnight Foundation #02, release 0.3.0-alpha.1).
822-
//
823-
// Before the fix, `_approve` checked the approver only when `auth` was
824-
// non-zero. That guard held the `_requireOwned` existence check, and the
825-
// approval write sat outside it, so the zero-auth path recorded approvals
826-
// for unminted tokens. Other circuits read the broken invariant back: a
827-
// recorded approval implies the token exists.
828-
//
829-
// Fixed by splitting the circuit the way Solidity overloads it: `_approve`
830-
// always requires existence and delegates to `_unsafeApprove`, whose
831-
// `isExistenceRequired` flag mirrors Solidity's `emitEvent`. These tests
832-
// failed on the unfixed code and pin the intended behaviour.
833-
describe('audit M-04: approvals for nonexistent tokens', () => {
834-
it('should reject a zero-auth approval for a nonexistent token', async () => {
820+
it('should throw if token does not exist and auth is zero', async () => {
835821
await expect(
836822
token._approve(SPENDER.either, NON_EXISTENT_TOKEN, ZERO_ACCOUNT),
837823
).rejects.toThrow('NonFungibleToken: nonexistent token');
@@ -841,20 +827,13 @@ describe('NonFungibleToken', () => {
841827
);
842828
});
843829

844-
it('should not let a planted approval mint a nonexistent token', async () => {
845-
// Plant an approval on an id nobody minted. On the unfixed code this
846-
// call succeeded; it now reverts and the chain stops right here (the
847-
// first test pins that revert), so the transfer below is exercised
848-
// without a stale approval.
830+
it('should not approve a nonexistent token into existence', async () => {
831+
// Plant an approval on a nonexistent id
849832
await token
850833
._approve(SPENDER.either, NON_EXISTENT_TOKEN, ZERO_ACCOUNT)
851834
.catch(() => undefined);
852835

853-
// On the unfixed code the stale approval satisfied `_isAuthorized`, so
854-
// `_checkAuthorized` never reached its nonexistent-token assert:
855-
// `_update` read the owner as zero, skipped the balance decrement,
856-
// credited SPENDER and wrote the owner entry, leaving SPENDER holding a
857-
// token nobody minted and blocking the composer's own gated mint of it.
836+
// Attempt to mint the nonexistent token through a transfer
858837
await token.privateState.injectSecretKey(SPENDER.secretKey);
859838
await expect(
860839
token.transferFrom(ZERO_ACCOUNT, SPENDER.either, NON_EXISTENT_TOKEN),
@@ -864,20 +843,17 @@ describe('NonFungibleToken', () => {
864843
expect(await token.balanceOf(SPENDER.either)).toEqual(0n);
865844
});
866845

867-
it('should not let a planted approval survive a later mint', async () => {
868-
// Plant an approval on the id OWNER is about to mint. Reverts on the
869-
// fixed code; succeeded on the unfixed code.
846+
it('should not approve a token before it is minted', async () => {
847+
// Plant an approval on the id OWNER is about to mint
870848
await token
871849
._approve(SPENDER.either, TOKENID_1, ZERO_ACCOUNT)
872850
.catch(() => undefined);
873851

874852
// `_update` clears approvals only when the source is non-zero, so a
875-
// mint leaves any planted approval standing.
853+
// planted approval would survive the mint
876854
await token._mint(OWNER.either, TOKENID_1);
877855
expect(await token.getApproved(TOKENID_1)).toEqual(ZERO_ACCOUNT);
878856

879-
// On the unfixed code SPENDER then took OWNER's token on the strength
880-
// of the surviving approval.
881857
await token.privateState.injectSecretKey(SPENDER.secretKey);
882858
await expect(
883859
token.transferFrom(OWNER.either, SPENDER.either, TOKENID_1),

0 commit comments

Comments
 (0)