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: Move signature verification #51

Merged
merged 2 commits into from
Aug 12, 2022
Merged

feat: Move signature verification #51

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
ac1021c
feat: Move signature verification
fzavalia Aug 10, 2022
a3ff7fa
fix: Test
fzavalia Aug 10, 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
96 changes: 54 additions & 42 deletions contracts/Rentals.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,27 +192,7 @@ contract Rentals is
) external nonReentrant {
_verifyUnsafeTransfer(_listing.contractAddress, _listing.tokenId);

// Verify that the signer provided in the listing is the one that signed it.
bytes32 listingHash = _hashTypedDataV4(
keccak256(
abi.encode(
LISTING_TYPE_HASH,
_listing.signer,
_listing.contractAddress,
_listing.tokenId,
_listing.expiration,
keccak256(abi.encodePacked(_listing.nonces)),
keccak256(abi.encodePacked(_listing.pricePerDay)),
keccak256(abi.encodePacked(_listing.maxDays)),
keccak256(abi.encodePacked(_listing.minDays)),
_listing.target
)
)
);

address lessor = ECDSAUpgradeable.recover(listingHash, _listing.signature);

require(lessor == _listing.signer, "Rentals#acceptListing: SIGNATURE_MISMATCH");
address lessor = _listing.signer;

// Verify that the caller and the signer are not the same address.
address tenant = _msgSender();
Expand Down Expand Up @@ -249,6 +229,8 @@ contract Rentals is
// Verify that the provided rental days is between min and max days range.
require(_rentalDays >= minDays && _rentalDays <= maxDays, "Rentals#acceptListing: DAYS_NOT_IN_RANGE");

_verifyListingSigner(_listing);

_rent(
RentParams(
lessor,
Expand Down Expand Up @@ -423,27 +405,7 @@ contract Rentals is
}

function _acceptOffer(Offer memory _offer, address _lessor) private nonReentrant {
// Verify that the signer provided in the offer is the one that signed it.
bytes32 offerHash = _hashTypedDataV4(
keccak256(
abi.encode(
OFFER_TYPE_HASH,
_offer.signer,
_offer.contractAddress,
_offer.tokenId,
_offer.expiration,
keccak256(abi.encodePacked(_offer.nonces)),
_offer.pricePerDay,
_offer.rentalDays,
_offer.operator,
_offer.fingerprint
)
)
);

address tenant = ECDSAUpgradeable.recover(offerHash, _offer.signature);

require(tenant == _offer.signer, "Rentals#acceptOffer: SIGNATURE_MISMATCH");
address tenant = _offer.signer;

require(_lessor != tenant, "Rentals#acceptOffer: CALLER_CANNOT_BE_SIGNER");

Expand All @@ -458,6 +420,8 @@ contract Rentals is
// Verify that the rental days provided in the offer are valid.
require(_offer.rentalDays > 0, "Rentals#acceptOffer: RENTAL_DAYS_IS_ZERO");

_verifyOfferSigner(_offer);

_rent(
RentParams(
_lessor,
Expand All @@ -473,6 +437,54 @@ contract Rentals is
);
}

/// @dev Verify that the signer provided in the listing is the address that created the provided signature.
function _verifyListingSigner(Listing calldata _listing) private view {
bytes32 listingHash = _hashTypedDataV4(
keccak256(
abi.encode(
LISTING_TYPE_HASH,
_listing.signer,
_listing.contractAddress,
_listing.tokenId,
_listing.expiration,
keccak256(abi.encodePacked(_listing.nonces)),
keccak256(abi.encodePacked(_listing.pricePerDay)),
keccak256(abi.encodePacked(_listing.maxDays)),
keccak256(abi.encodePacked(_listing.minDays)),
_listing.target
)
)
);

address signer = ECDSAUpgradeable.recover(listingHash, _listing.signature);

require(signer == _listing.signer, "Rentals#_verifyListingSigner: SIGNER_MISMATCH");
}

/// @dev Verify that the signer provided in the offer is the address that created the provided signature.
function _verifyOfferSigner(Offer memory _offer) private view {
bytes32 offerHash = _hashTypedDataV4(
keccak256(
abi.encode(
OFFER_TYPE_HASH,
_offer.signer,
_offer.contractAddress,
_offer.tokenId,
_offer.expiration,
keccak256(abi.encodePacked(_offer.nonces)),
_offer.pricePerDay,
_offer.rentalDays,
_offer.operator,
_offer.fingerprint
)
)
);

address signer = ECDSAUpgradeable.recover(offerHash, _offer.signature);

require(signer == _offer.signer, "Rentals#_verifyOfferSigner: SIGNER_MISMATCH");
}

function _rent(RentParams memory _rentParams) private {
IERC721Rentable asset = IERC721Rentable(_rentParams.contractAddress);

Expand Down
33 changes: 10 additions & 23 deletions test/Rentals.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ describe('Rentals', () => {
acceptListingParams.rentalDays,
acceptListingParams.fingerprint
)
).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH')
).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH')
})

it('should revert when pricePerDay maxDays and minDays length is 0', async () => {
Expand Down Expand Up @@ -1518,9 +1518,8 @@ describe('Rentals', () => {

it('should revert when the listing was signed with arrays in a different order than the ones provided', async () => {
listingParams.pricePerDay = [ether('10'), ether('20'), ether('30')]
listingParams.maxDays = [10, 20, 30]
listingParams.minDays = [5, 15, 25]
listingParams.nonces = [1, 2, 3]
listingParams.maxDays = [98, 99, 100]
listingParams.minDays = [1, 2, 3]

const signature = await getListingSignature(lessor, rentals, listingParams)

Expand All @@ -1534,7 +1533,7 @@ describe('Rentals', () => {
acceptListingParams.rentalDays,
acceptListingParams.fingerprint
)
).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH')
).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH')

await expect(
rentals
Expand All @@ -1546,7 +1545,7 @@ describe('Rentals', () => {
acceptListingParams.rentalDays,
acceptListingParams.fingerprint
)
).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH')
).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH')

await expect(
rentals
Expand All @@ -1558,19 +1557,7 @@ describe('Rentals', () => {
acceptListingParams.rentalDays,
acceptListingParams.fingerprint
)
).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH')

await expect(
rentals
.connect(tenant)
.acceptListing(
{ ...listingParams, nonces: listingParams.nonces.reverse() as [BigNumberish, BigNumberish, BigNumberish], signature },
acceptListingParams.operator,
acceptListingParams.index,
acceptListingParams.rentalDays,
acceptListingParams.fingerprint
)
).to.be.revertedWith('Rentals#acceptListing: SIGNATURE_MISMATCH')
).to.be.revertedWith('Rentals#_verifyListingSigner: SIGNER_MISMATCH')
})
})

Expand Down Expand Up @@ -1936,8 +1923,8 @@ describe('Rentals', () => {
await expect(
rentals
.connect(lessor)
.acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, { ...offerParams, signer: lessor.address }) })
).to.be.revertedWith('Rentals#acceptOffer: SIGNATURE_MISMATCH')
.acceptOffer({ ...offerParams, signature: await getOfferSignature(tenant, rentals, { ...offerParams, signer: extra.address }) })
).to.be.revertedWith('Rentals#_verifyOfferSigner: SIGNER_MISMATCH')
})

it('should revert when lessor is same as tenant', async () => {
Expand Down Expand Up @@ -3206,13 +3193,13 @@ describe('Rentals', () => {
})

it('should revert when the offer signer does not match the signer provided in params', async () => {
offerEncodeValue[signerIndex] = lessor.address
offerEncodeValue[signerIndex] = extra.address

const bytes = ethers.utils.defaultAbiCoder.encode([offerEncodeType], [offerEncodeValue])

await expect(
land.connect(lessor)['safeTransferFrom(address,address,uint256,bytes)'](lessor.address, rentals.address, tokenId, bytes)
).to.be.revertedWith('Rentals#acceptOffer: SIGNATURE_MISMATCH')
).to.be.revertedWith('Rentals#_verifyOfferSigner: SIGNER_MISMATCH')
})

it('should revert when lessor is same as tenant', async () => {
Expand Down