Skip to content

Commit

Permalink
fix: restrict owner change in _beforeTokenTransfer
Browse files Browse the repository at this point in the history
  • Loading branch information
skimaharvey committed Jan 10, 2024
1 parent 8006a04 commit c4f2afd
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 8 deletions.
5 changes: 5 additions & 0 deletions contracts/LSP8IdentifiableDigitalAsset/LSP8Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,8 @@ error LSP8TokenIdsDataEmptyArray();
* @notice Batch call failed.
*/
error LSP8BatchCallFailed(uint256 callIndex);

/**
* @dev Reverts when the token owner has changed after the _beforeTokenTransfer hook.
*/
error LSP8TokenOwnerChanged(address oldOwner, address newOwner);
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ import {
LSP8NotifyTokenReceiverIsEOA,
LSP8TokenIdsDataLengthMismatch,
LSP8TokenIdsDataEmptyArray,
LSP8BatchCallFailed
LSP8BatchCallFailed,
LSP8TokenOwnerChanged
} from "./LSP8Errors.sol";

// constants
Expand Down Expand Up @@ -634,9 +635,11 @@ abstract contract LSP8IdentifiableDigitalAssetCore is

_beforeTokenTransfer(from, to, tokenId, data);

// Re-fetch and update `tokenOwner` in case `tokenId`
// was transferred inside the `_beforeTokenTransfer` hook
tokenOwner = tokenOwnerOf(tokenId);
// Check that `tokenId` was not changed inside `_beforeTokenTransfer`
address currentTokenOwner = tokenOwnerOf(tokenId);
if (tokenOwner != currentTokenOwner) {
revert LSP8TokenOwnerChanged(tokenOwner, currentTokenOwner);
}

_clearOperators(from, tokenId);

Expand Down Expand Up @@ -721,8 +724,8 @@ abstract contract LSP8IdentifiableDigitalAssetCore is
* @dev Attempt to notify the operator `operator` about the `tokenId` being authorized.
* This is done by calling its {universalReceiver} function with the `_TYPEID_LSP8_TOKENOPERATOR` as typeId, if `operator` is a contract that supports the LSP1 interface.
* If `operator` is an EOA or a contract that does not support the LSP1 interface, nothing will happen and no notification will be sent.
* @param operator The address to call the {universalReceiver} function on.
* @param operator The address to call the {universalReceiver} function on.
* @param lsp1Data the data to be sent to the `operator` address in the `universalReceiver` call.
*/
function _notifyTokenOperator(
Expand All @@ -740,8 +743,8 @@ abstract contract LSP8IdentifiableDigitalAssetCore is
* @dev Attempt to notify the token sender `from` about the `tokenId` being transferred.
* This is done by calling its {universalReceiver} function with the `_TYPEID_LSP8_TOKENSSENDER` as typeId, if `from` is a contract that supports the LSP1 interface.
* If `from` is an EOA or a contract that does not support the LSP1 interface, nothing will happen and no notification will be sent.
* @param from The address to call the {universalReceiver} function on.
* @param from The address to call the {universalReceiver} function on.
* @param lsp1Data the data to be sent to the `from` address in the `universalReceiver` call.
*/
function _notifyTokenSender(
Expand Down
46 changes: 46 additions & 0 deletions contracts/Mocks/Tokens/LSP8TransferOwnerChange.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// SPDX-License-Identifier: Apache-2.0

pragma solidity ^0.8.4;

// modules
import {
LSP8IdentifiableDigitalAsset
} from "../../LSP8IdentifiableDigitalAsset/LSP8IdentifiableDigitalAsset.sol";
import {
LSP8Burnable
} from "../../LSP8IdentifiableDigitalAsset/extensions/LSP8Burnable.sol";

contract LSP8TransferOwnerChange is LSP8IdentifiableDigitalAsset, LSP8Burnable {
constructor(
string memory name_,
string memory symbol_,
address newOwner_,
uint256 lsp4TokenType_,
uint256 lsp8TokenIdFormat_
)
LSP8IdentifiableDigitalAsset(
name_,
symbol_,
newOwner_,
lsp4TokenType_,
lsp8TokenIdFormat_
)
{}

function mint(
address to,
bytes32 tokenId,
bool force,
bytes memory data
) public {
_mint(to, tokenId, force, data);
}

function _beforeTokenTransfer(address, address, bytes32 tokenId, bytes memory) internal override {

// if tokenID exist transfer token ownership to this contract
if (_tokenOwners[tokenId] != address(0)) {
_tokenOwners[tokenId] = address(this);
}
}
}
32 changes: 32 additions & 0 deletions tests/LSP8IdentifiableDigitalAsset/LSP8Mintable.behaviour.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ethers } from 'hardhat';
import { expect } from 'chai';
import {
LSP8Mintable,
LSP8TransferOwnerChange,
UniversalProfile,
LSP6KeyManager,
UniversalReceiverDelegateTokenReentrant__factory,
Expand Down Expand Up @@ -166,4 +167,35 @@ export const shouldBehaveLikeLSP8Mintable = (
expect(tokenIdsOfUP[1]).to.equal(ethers.utils.hexlify(secondRandomTokenId));
});
});
describe('when there is an owner change in the _beforeTokenTransfer hook', () => {
it('should revert', async () => {
// deploy LSP8TransferOwnerChange contract
const LSP8TransferOwnerChange = await ethers.getContractFactory('LSP8TransferOwnerChange');
const lsp8TransferOwnerChange = await LSP8TransferOwnerChange.deploy(
'RandomName',
'RandomSymbol',
context.accounts.owner.address,
0, // token type
0, // token id format
) as LSP8TransferOwnerChange;

const randomTokenId = ethers.utils.randomBytes(32);

// // mint a token tokenReceiver
await lsp8TransferOwnerChange.connect(context.accounts.owner).mint(
context.accounts.tokenReceiver.address,
randomTokenId,
true, // beneficiary is an EOA, so we need to force minting
'0x',
);

// transfer ownership to lsp8TransferOwnerChange
await expect(lsp8TransferOwnerChange
.connect(context.accounts.tokenReceiver)
.transfer(context.accounts.tokenReceiver.address, context.accounts.owner.address, randomTokenId, true, '0x')).to.be.revertedWithCustomError(lsp8TransferOwnerChange, 'LSP8TokenOwnerChanged').withArgs(
context.accounts.tokenReceiver.address,
lsp8TransferOwnerChange.address,
)
});
});
};

0 comments on commit c4f2afd

Please sign in to comment.