Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Rename set operator #37

Merged
merged 2 commits into from
Aug 8, 2022
Merged

feat: Rename set operator #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e1dc3e
feat: Rename setOperator
fzavalia Aug 5, 2022
fecf688
feat: Rename event and add test
fzavalia Aug 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions contracts/Rentals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ contract Rentals is NonceVerifiable, NativeMetaTransaction, IERC721Receiver, Ree
event TokenUpdated(IERC20 _from, IERC20 _to, address _sender);
event FeeCollectorUpdated(address _from, address _to, address _sender);
event FeeUpdated(uint256 _from, uint256 _to, address _sender);
event OperatorUpdated(address _contractAddress, uint256 _tokenId, address _to, address _sender);
event UpdateOperatorUpdated(address _contractAddress, uint256 _tokenId, address _to, address _sender);
event AssetClaimed(address _contractAddress, uint256 _tokenId, address _sender);
event AssetRented(
address _contractAddress,
Expand Down Expand Up @@ -291,7 +291,7 @@ contract Rentals is NonceVerifiable, NativeMetaTransaction, IERC721Receiver, Ree
/// @param _contractAddress The contract address of the asset.
/// @param _tokenId The token id of the asset.
/// @param _operator The address that will have operator privileges over the asset.
function setOperator(
function setUpdateOperator(
address _contractAddress,
uint256 _tokenId,
address _operator
Expand All @@ -305,14 +305,14 @@ contract Rentals is NonceVerifiable, NativeMetaTransaction, IERC721Receiver, Ree
bool rented = isRented(_contractAddress, _tokenId);
// If rented, only the tenant can change the operator.
// If not, only the original owner can.
bool canSetOperator = (rental.tenant == sender && rented) || (rental.lessor == sender && !rented);
bool canSetUpdateOperator = (rental.tenant == sender && rented) || (rental.lessor == sender && !rented);

require(canSetOperator, "Rentals#setOperator: CANNOT_UPDATE_OPERATOR");
require(canSetUpdateOperator, "Rentals#setUpdateOperator: CANNOT_UPDATE_OPERATOR");

// Update the operator.
asset.setUpdateOperator(_tokenId, _operator);

emit OperatorUpdated(_contractAddress, _tokenId, _operator, sender);
emit UpdateOperatorUpdated(_contractAddress, _tokenId, _operator, sender);
}

/// @notice Standard function called by ERC721 contracts whenever a safe transfer occurs.
Expand Down
36 changes: 22 additions & 14 deletions test/Rentals.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2147,17 +2147,25 @@ describe('Rentals', () => {
})
})

describe('setOperator', () => {
describe('setUpdateOperator', () => {
const newOperator = zeroAddress

beforeEach(async () => {
await rentals.connect(deployer).initialize(owner.address, mana.address, collector.address, fee)
})

it('should emit an UpdateOperatorUpdated event', async () => {
await rentals.connect(lessor).acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, offerParams) })

await expect(rentals.connect(tenant).setUpdateOperator(land.address, tokenId, newOperator))
.to.emit(rentals, 'UpdateOperatorUpdated')
.withArgs(offerParams.contractAddress, offerParams.tokenId, newOperator, tenant.address)
})

it('should allow the tenant to update the asset operator', async () => {
await rentals.connect(lessor).acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, offerParams) })

await rentals.connect(tenant).setOperator(land.address, tokenId, newOperator)
await rentals.connect(tenant).setUpdateOperator(land.address, tokenId, newOperator)
})

it('should allow the lessor to update the asset operator after the rent is over', async () => {
Expand All @@ -2166,15 +2174,15 @@ describe('Rentals', () => {
await evmIncreaseTime(daysToSeconds(offerParams.rentalDays))
await evmMine()

await rentals.connect(lessor).setOperator(land.address, tokenId, newOperator)
await rentals.connect(lessor).setUpdateOperator(land.address, tokenId, newOperator)
})

it('should revert when the asset has never been rented', async () => {
const setOperatorByLessor = rentals.connect(lessor).setOperator(land.address, tokenId, newOperator)
const setOperatorByTenant = rentals.connect(tenant).setOperator(land.address, tokenId, newOperator)
const setUpdateOperatorByLessor = rentals.connect(lessor).setUpdateOperator(land.address, tokenId, newOperator)
const setUpdateOperatorByTenant = rentals.connect(tenant).setUpdateOperator(land.address, tokenId, newOperator)

await expect(setOperatorByLessor).to.be.revertedWith('Rentals#setOperator: CANNOT_UPDATE_OPERATOR')
await expect(setOperatorByTenant).to.be.revertedWith('Rentals#setOperator: CANNOT_UPDATE_OPERATOR')
await expect(setUpdateOperatorByLessor).to.be.revertedWith('Rentals#setUpdateOperator: CANNOT_UPDATE_OPERATOR')
await expect(setUpdateOperatorByTenant).to.be.revertedWith('Rentals#setUpdateOperator: CANNOT_UPDATE_OPERATOR')
})

it('should revert when the tenant tries to update the operator after the rent is over', async () => {
Expand All @@ -2183,17 +2191,17 @@ describe('Rentals', () => {
await evmIncreaseTime(daysToSeconds(offerParams.rentalDays))
await evmMine()

const setOperator = rentals.connect(tenant).setOperator(land.address, tokenId, newOperator)
const setUpdateOperator = rentals.connect(tenant).setUpdateOperator(land.address, tokenId, newOperator)

await expect(setOperator).to.be.revertedWith('Rentals#setOperator: CANNOT_UPDATE_OPERATOR')
await expect(setUpdateOperator).to.be.revertedWith('Rentals#setUpdateOperator: CANNOT_UPDATE_OPERATOR')
})

it('should revert when the lessor tries to update the operator before the rental ends', async () => {
await rentals.connect(lessor).acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, offerParams) })

const setOperator = rentals.connect(lessor).setOperator(land.address, tokenId, newOperator)
const setUpdateOperator = rentals.connect(lessor).setUpdateOperator(land.address, tokenId, newOperator)

await expect(setOperator).to.be.revertedWith('Rentals#setOperator: CANNOT_UPDATE_OPERATOR')
await expect(setUpdateOperator).to.be.revertedWith('Rentals#setUpdateOperator: CANNOT_UPDATE_OPERATOR')
})
})

Expand Down Expand Up @@ -2816,12 +2824,12 @@ describe('Rentals', () => {
).to.be.revertedWith('ReentrancyGuard: reentrant call')
})

it('should revert when the contract is reentered through the setOperator function', async () => {
it('should revert when the contract is reentered through the setUpdateOperator function', async () => {
offerParams = { ...offerParams, contractAddress: reentrantERC721.address }

const abi = ['function setOperator(address _contractAddress,uint256 _tokenId,address _operator)']
const abi = ['function setUpdateOperator(address _contractAddress,uint256 _tokenId,address _operator)']
const iface = new ethers.utils.Interface(abi)
const functionData = iface.encodeFunctionData('setOperator', [reentrantERC721.address, offerParams.tokenId, extra.address])
const functionData = iface.encodeFunctionData('setUpdateOperator', [reentrantERC721.address, offerParams.tokenId, extra.address])
await reentrantERC721.setData(functionData)

await expect(
Expand Down