From 019f5729144d23e0f8bce00b28891a48652deccc Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Tue, 25 Jan 2022 16:54:49 +0000 Subject: [PATCH 01/18] add first option for staking --- .../ERC1238/extensions/ERC1238Stakable.sol | 61 +++++++ contracts/mocks/ERC1238StakableMock.sol | 55 ++++++ test/ERC1238/extensions/ERC1238Stakable.ts | 172 ++++++++++++++++++ 3 files changed, 288 insertions(+) create mode 100644 contracts/ERC1238/extensions/ERC1238Stakable.sol create mode 100644 contracts/mocks/ERC1238StakableMock.sol create mode 100644 test/ERC1238/extensions/ERC1238Stakable.ts diff --git a/contracts/ERC1238/extensions/ERC1238Stakable.sol b/contracts/ERC1238/extensions/ERC1238Stakable.sol new file mode 100644 index 0000000..0a94374 --- /dev/null +++ b/contracts/ERC1238/extensions/ERC1238Stakable.sol @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238.sol"; + +/** + * @dev Proposal for ERC1238 tokens extension that make them 'stakable' + */ +abstract contract ERC1238Stakable is ERC1238 { + // Mapping owner => tokenId => stakeholder => stake size + mapping(address => mapping(uint256 => mapping(address => uint256))) private _stakes; + + function _beforeBurn( + address burner, + address from, + uint256 id, + uint256 amount + ) internal view virtual override { + require(burner == from || _stakes[from][id][burner] >= amount, "ERC1238Stakable: Unauthorized to burn tokens"); + } + + /** + * @dev Allows token owners to put their tokens at stake + * + * Calling this function again with the same stakeholder and id + * overrides the previous given allowance + * + * Requirements: + * + * + */ + function _increaseStake( + address stakeholder, + uint256 id, + uint256 amount + ) internal { + _stakes[msg.sender][id][stakeholder] += amount; + } + + function _decreaseStake( + address owner, + uint256 id, + uint256 amount + ) internal { + uint256 authorization = _stakes[owner][id][msg.sender]; + + require(authorization >= amount, "ERC1238Stakable: cannot decrease more than current stake"); + unchecked { + _stakes[owner][id][msg.sender] = authorization - amount; + } + } + + function stakeOf( + address owner, + uint256 id, + address stakeholder + ) public view returns (uint256) { + return _stakes[owner][id][stakeholder]; + } +} diff --git a/contracts/mocks/ERC1238StakableMock.sol b/contracts/mocks/ERC1238StakableMock.sol new file mode 100644 index 0000000..78aeda3 --- /dev/null +++ b/contracts/mocks/ERC1238StakableMock.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238/ERC1238.sol"; +import "../ERC1238/extensions/ERC1238Stakable.sol"; + +/** + * @dev Mock contract for ERC1238 tokens using ERC1238Stakable extension rendering them 'stakable' + */ +contract ERC1238StakableMock is ERC1238, ERC1238Stakable { + constructor(string memory uri) ERC1238(uri) {} + + function _beforeBurn( + address burner, + address from, + uint256 id, + uint256 amount + ) internal view override(ERC1238, ERC1238Stakable) { + super._beforeBurn(burner, from, id, amount); + } + + function mint( + address to, + uint256 id, + uint256 amount, + bytes memory data + ) public { + _mint(to, id, amount, data); + } + + function burn( + address owner, + uint256 id, + uint256 amount + ) public { + _burn(owner, id, amount); + } + + function increaseStake( + address stakeholder, + uint256 id, + uint256 amount + ) external { + _increaseStake(stakeholder, id, amount); + } + + function decreaseStake( + address owner, + uint256 id, + uint256 amount + ) external { + _decreaseStake(owner, id, amount); + } +} diff --git a/test/ERC1238/extensions/ERC1238Stakable.ts b/test/ERC1238/extensions/ERC1238Stakable.ts new file mode 100644 index 0000000..aae6978 --- /dev/null +++ b/test/ERC1238/extensions/ERC1238Stakable.ts @@ -0,0 +1,172 @@ +import { artifacts, ethers, waffle } from "hardhat"; +import type { Artifact } from "hardhat/types"; +import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/dist/src/signer-with-address"; +import { expect } from "chai"; + +import type { ERC1238StakableMock } from "../../../src/types/ERC1238StakableMock"; +import { toBN } from "../../utils/test-utils"; + +const BASE_URI = "https://token-cdn-domain/{id}.json"; + +describe("ERC1238URIStakable", function () { + let erc1238Stakable: ERC1238StakableMock; + let admin: SignerWithAddress; + let tokenHolder: SignerWithAddress; + let stakeholder: SignerWithAddress; + + before(async function () { + const signers: SignerWithAddress[] = await ethers.getSigners(); + admin = signers[0]; + tokenHolder = signers[1]; + stakeholder = signers[2]; + }); + + beforeEach(async function () { + const ERC1238StakableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238StakableMock"); + erc1238Stakable = await waffle.deployContract(admin, ERC1238StakableMockArtifact, [BASE_URI]); + }); + + describe("Staking", () => { + const fungibleTokenId = 1; + const amountMintedFungible = toBN("1000"); + const nftID = 2; + const amountMintedNonFungible = 1; + + beforeEach(async () => { + // mint fungible tokens + await erc1238Stakable.connect(admin).mint(tokenHolder.address, fungibleTokenId, amountMintedFungible, []); + // mint an NFT + await erc1238Stakable.connect(admin).mint(tokenHolder.address, nftID, amountMintedNonFungible, []); + }); + + it("should let a token owner increase a stake", async () => { + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, nftID, 1); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, nftID, stakeholder.address)).to.eq(1); + }); + + it("should let a stakeholder burn a staked NFT", async () => { + // Given + expect(await erc1238Stakable.balanceOf(tokenHolder.address, nftID)).to.eq(amountMintedNonFungible); + + // When + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, nftID, 1); + + await erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, nftID, 1); + + // Expect + expect(await erc1238Stakable.balanceOf(tokenHolder.address, nftID)).to.eq(0); + }); + + it("should let a stakeholder burn up to the amount of fungible tokens staked", async () => { + // Given + const stakedAmount = toBN("500"); + + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + + // When + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + await erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, fungibleTokenId, stakedAmount); + + // Expect + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq( + amountMintedFungible.sub(stakedAmount), + ); + }); + + it("should not let a stakeholder burn more that the staked amount", async () => { + const stakedAmount = toBN("500"); + const burnAmount = stakedAmount.add(toBN("1")); + + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + await expect( + erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, fungibleTokenId, burnAmount), + ).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); + }); + + it("should let a token owner burn tokens before staking", async () => { + // Given + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + + // When + await erc1238Stakable.connect(tokenHolder).burn(tokenHolder.address, fungibleTokenId, amountMintedFungible); + + // Expect + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(0); + }); + + it("should let a token owner burn tokens after staking", async () => { + // Given + const stakedAmount = toBN("800"); + + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + + // When + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + await erc1238Stakable.connect(tokenHolder).burn(tokenHolder.address, fungibleTokenId, stakedAmount); + + // Expect + expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq( + amountMintedFungible.sub(stakedAmount), + ); + // "Burn" allowance is the same + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount, + ); + }); + + context("Decrease Stake", () => { + it("should let a stakeholder decrease a stake", async () => { + const stakedAmount = toBN("800"); + const amountToUnstake = toBN("300"); + + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount, + ); + + await erc1238Stakable.connect(stakeholder).decreaseStake(tokenHolder.address, fungibleTokenId, amountToUnstake); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount.sub(amountToUnstake), + ); + }); + + it("should let a stakeholder fully unstake", async () => { + const stakedAmount = toBN("800"); + const amountToUnstake = stakedAmount; + + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount, + ); + + await erc1238Stakable.connect(stakeholder).decreaseStake(tokenHolder.address, fungibleTokenId, amountToUnstake); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq(0); + }); + + it("should not let a token owner unstake", async () => { + const stakedAmount = toBN("600"); + await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount, + ); + + await expect( + erc1238Stakable.connect(tokenHolder).decreaseStake(tokenHolder.address, fungibleTokenId, stakedAmount), + ).to.be.revertedWith("ERC1238Stakable: cannot decrease more than current stake"); + + expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + stakedAmount, + ); + }); + }); + }); +}); From ca96311d8fd720674248aa53bbe2b21786fa9dbc Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Sat, 29 Jan 2022 11:18:41 +0000 Subject: [PATCH 02/18] rename tokenHolder to tokenOwner --- test/ERC1238/extensions/ERC1238Stakable.ts | 72 +++++++++++----------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/test/ERC1238/extensions/ERC1238Stakable.ts b/test/ERC1238/extensions/ERC1238Stakable.ts index aae6978..b93a8c6 100644 --- a/test/ERC1238/extensions/ERC1238Stakable.ts +++ b/test/ERC1238/extensions/ERC1238Stakable.ts @@ -11,13 +11,13 @@ const BASE_URI = "https://token-cdn-domain/{id}.json"; describe("ERC1238URIStakable", function () { let erc1238Stakable: ERC1238StakableMock; let admin: SignerWithAddress; - let tokenHolder: SignerWithAddress; + let tokenOwner: SignerWithAddress; let stakeholder: SignerWithAddress; before(async function () { const signers: SignerWithAddress[] = await ethers.getSigners(); admin = signers[0]; - tokenHolder = signers[1]; + tokenOwner = signers[1]; stakeholder = signers[2]; }); @@ -34,43 +34,43 @@ describe("ERC1238URIStakable", function () { beforeEach(async () => { // mint fungible tokens - await erc1238Stakable.connect(admin).mint(tokenHolder.address, fungibleTokenId, amountMintedFungible, []); + await erc1238Stakable.connect(admin).mint(tokenOwner.address, fungibleTokenId, amountMintedFungible, []); // mint an NFT - await erc1238Stakable.connect(admin).mint(tokenHolder.address, nftID, amountMintedNonFungible, []); + await erc1238Stakable.connect(admin).mint(tokenOwner.address, nftID, amountMintedNonFungible, []); }); it("should let a token owner increase a stake", async () => { - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, nftID, 1); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, nftID, 1); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, nftID, stakeholder.address)).to.eq(1); + expect(await erc1238Stakable.stakeOf(tokenOwner.address, nftID, stakeholder.address)).to.eq(1); }); it("should let a stakeholder burn a staked NFT", async () => { // Given - expect(await erc1238Stakable.balanceOf(tokenHolder.address, nftID)).to.eq(amountMintedNonFungible); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, nftID)).to.eq(amountMintedNonFungible); // When - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, nftID, 1); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, nftID, 1); - await erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, nftID, 1); + await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, nftID, 1); // Expect - expect(await erc1238Stakable.balanceOf(tokenHolder.address, nftID)).to.eq(0); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, nftID)).to.eq(0); }); it("should let a stakeholder burn up to the amount of fungible tokens staked", async () => { // Given const stakedAmount = toBN("500"); - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); // When - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - await erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, stakedAmount); // Expect - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq( + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq( amountMintedFungible.sub(stakedAmount), ); }); @@ -79,41 +79,41 @@ describe("ERC1238URIStakable", function () { const stakedAmount = toBN("500"); const burnAmount = stakedAmount.add(toBN("1")); - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); await expect( - erc1238Stakable.connect(stakeholder).burn(tokenHolder.address, fungibleTokenId, burnAmount), + erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount), ).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); }); it("should let a token owner burn tokens before staking", async () => { // Given - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); // When - await erc1238Stakable.connect(tokenHolder).burn(tokenHolder.address, fungibleTokenId, amountMintedFungible); + await erc1238Stakable.connect(tokenOwner).burn(tokenOwner.address, fungibleTokenId, amountMintedFungible); // Expect - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(0); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(0); }); it("should let a token owner burn tokens after staking", async () => { // Given const stakedAmount = toBN("800"); - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq(amountMintedFungible); + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); // When - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - await erc1238Stakable.connect(tokenHolder).burn(tokenHolder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).burn(tokenOwner.address, fungibleTokenId, stakedAmount); // Expect - expect(await erc1238Stakable.balanceOf(tokenHolder.address, fungibleTokenId)).to.eq( + expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq( amountMintedFungible.sub(stakedAmount), ); // "Burn" allowance is the same - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount, ); }); @@ -123,15 +123,15 @@ describe("ERC1238URIStakable", function () { const stakedAmount = toBN("800"); const amountToUnstake = toBN("300"); - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount, ); - await erc1238Stakable.connect(stakeholder).decreaseStake(tokenHolder.address, fungibleTokenId, amountToUnstake); + await erc1238Stakable.connect(stakeholder).decreaseStake(tokenOwner.address, fungibleTokenId, amountToUnstake); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount.sub(amountToUnstake), ); }); @@ -140,30 +140,30 @@ describe("ERC1238URIStakable", function () { const stakedAmount = toBN("800"); const amountToUnstake = stakedAmount; - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount, ); - await erc1238Stakable.connect(stakeholder).decreaseStake(tokenHolder.address, fungibleTokenId, amountToUnstake); + await erc1238Stakable.connect(stakeholder).decreaseStake(tokenOwner.address, fungibleTokenId, amountToUnstake); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq(0); + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq(0); }); it("should not let a token owner unstake", async () => { const stakedAmount = toBN("600"); - await erc1238Stakable.connect(tokenHolder).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount, ); await expect( - erc1238Stakable.connect(tokenHolder).decreaseStake(tokenHolder.address, fungibleTokenId, stakedAmount), + erc1238Stakable.connect(tokenOwner).decreaseStake(tokenOwner.address, fungibleTokenId, stakedAmount), ).to.be.revertedWith("ERC1238Stakable: cannot decrease more than current stake"); - expect(await erc1238Stakable.stakeOf(tokenHolder.address, fungibleTokenId, stakeholder.address)).to.eq( + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( stakedAmount, ); }); From ec50633f0b83a83503633c1ab02a4e62be6151c4 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Sat, 29 Jan 2022 15:07:20 +0000 Subject: [PATCH 03/18] introduce second option for staking --- .../ERC1238/extensions/ERC1238Holdable.sol | 60 ++++++ .../ERC1238/extensions/IERC1238Holdable.sol | 26 +++ contracts/mocks/ERC1238HoldableMock.sol | 74 +++++++ test/ERC1238/extensions/ERC1238Holdable.ts | 190 ++++++++++++++++++ 4 files changed, 350 insertions(+) create mode 100644 contracts/ERC1238/extensions/ERC1238Holdable.sol create mode 100644 contracts/ERC1238/extensions/IERC1238Holdable.sol create mode 100644 contracts/mocks/ERC1238HoldableMock.sol create mode 100644 test/ERC1238/extensions/ERC1238Holdable.ts diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol new file mode 100644 index 0000000..f96d4aa --- /dev/null +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238.sol"; +import "./IERC1238Holdable.sol"; + +/** + * @dev Proposal for ERC1238 tokens extension that allow addresses + * to hold tokens on behalf of others (escrow) + */ +abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { + // Mapping holder => id => balance + mapping(address => mapping(uint256 => uint256)) private _escrowedBalances; + + function escrowedBalance(address holder, uint256 id) public view override returns (uint256) { + return _escrowedBalances[holder][id]; + } + + function _beforeMint( + address minter, + address to, + uint256 id, + uint256 amount, + bytes memory data + ) internal virtual override { + // set the token recipient as first holder by default when tokens are minted + _escrowedBalances[to][id] += amount; + } + + function _beforeBurn( + address burner, + address from, + uint256 id, + uint256 amount + ) internal virtual override { + require(burner == from, "ERC1238Holdable: Unauthorized to burn tokens"); + require(_escrowedBalances[burner][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); + + _escrowedBalances[burner][id] -= amount; + } + + function _entrust( + address to, + uint256 id, + uint256 amount + ) internal virtual { + require(to != address(0), "ERC1238Holdable: transfer to the zero address"); + + address from = msg.sender; + + uint256 fromBalance = _escrowedBalances[from][id]; + require(fromBalance >= amount, "ERC1238Holdable: amount exceeds balance held"); + + _escrowedBalances[from][id] -= amount; + _escrowedBalances[to][id] += amount; + + emit Entrust(from, to, id, amount); + } +} diff --git a/contracts/ERC1238/extensions/IERC1238Holdable.sol b/contracts/ERC1238/extensions/IERC1238Holdable.sol new file mode 100644 index 0000000..172bfad --- /dev/null +++ b/contracts/ERC1238/extensions/IERC1238Holdable.sol @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../IERC1238.sol"; + +/** + * @dev Proposal of an interface for ERC1238 token with storage based token URI management. + */ +interface IERC1238Holdable is IERC1238 { + /** + * @dev Event emitted when `from` entrusts `to` with `amount` of tokens with token `id` + */ + event Entrust(address from, address to, uint256 indexed id, uint256 amount); + + /** + * @dev Returns the balance of a token holder for a given `id` + */ + function escrowedBalance(address holder, uint256 id) external view returns (uint256); + + function entrust( + address to, + uint256 id, + uint256 amount + ) external; +} diff --git a/contracts/mocks/ERC1238HoldableMock.sol b/contracts/mocks/ERC1238HoldableMock.sol new file mode 100644 index 0000000..c167ba2 --- /dev/null +++ b/contracts/mocks/ERC1238HoldableMock.sol @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238/ERC1238.sol"; +import "../ERC1238/extensions/ERC1238Holdable.sol"; + +/** + * @dev Mock contract for ERC1238 tokens using ERC1238Holdable extension + */ +contract ERC1238HoldableMock is ERC1238, ERC1238Holdable { + constructor(string memory uri) ERC1238(uri) {} + + function _beforeMint( + address minter, + address to, + uint256 id, + uint256 amount, + bytes memory data + ) internal override(ERC1238, ERC1238Holdable) { + super._beforeMint(minter, to, id, amount, data); + } + + function _beforeBurn( + address burner, + address from, + uint256 id, + uint256 amount + ) internal override(ERC1238, ERC1238Holdable) { + super._beforeBurn(burner, from, id, amount); + } + + function mint( + address to, + uint256 id, + uint256 amount, + bytes memory data + ) public { + _mint(to, id, amount, data); + } + + function mintBatch( + address to, + uint256[] memory ids, + uint256[] memory amounts, + bytes memory data + ) public { + _mintBatch(to, ids, amounts, data); + } + + function burn( + address owner, + uint256 id, + uint256 amount + ) public { + _burn(owner, id, amount); + } + + function burnBatch( + address owner, + uint256[] memory ids, + uint256[] memory amounts + ) public { + _burnBatch(owner, ids, amounts); + } + + function entrust( + address to, + uint256 id, + uint256 amount + ) public override { + _entrust(to, id, amount); + } +} diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts new file mode 100644 index 0000000..2b46a1a --- /dev/null +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -0,0 +1,190 @@ +import { artifacts, ethers, waffle } from "hardhat"; +import type { Artifact } from "hardhat/types"; +import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/dist/src/signer-with-address"; +import { expect } from "chai"; + +import type { ERC1238HoldableMock } from "../../../src/types/ERC1238HoldableMock"; +import { ZERO_ADDRESS } from "../../utils/test-utils"; + +const BASE_URI = "https://token-cdn-domain/{id}.json"; + +describe("ERC1238URIHoldable", function () { + let erc1238Holdable: ERC1238HoldableMock; + let admin: SignerWithAddress; + let tokenOwner: SignerWithAddress; + let tokenHolder1: SignerWithAddress; + let tokenHolder2: SignerWithAddress; + + const tokenId = 888888; + const mintAmount = 98765432; + const data = "0x12345678"; + + before(async function () { + const signers: SignerWithAddress[] = await ethers.getSigners(); + admin = signers[0]; + tokenOwner = signers[1]; + tokenHolder1 = signers[2]; + tokenHolder2 = signers[3]; + }); + + beforeEach(async function () { + const ERC1238HoldableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238HoldableMock"); + erc1238Holdable = await waffle.deployContract(admin, ERC1238HoldableMockArtifact, [BASE_URI]); + }); + + describe("Minting", () => { + it("should set the the token recipient as first holder", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + + it("should update the held balance when minting multiple times", async () => { + const firstAmount = 1000; + const secondAmount = 200; + await erc1238Holdable.mint(tokenOwner.address, tokenId, firstAmount, data); + + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(firstAmount); + + await erc1238Holdable.mint(tokenOwner.address, tokenId, secondAmount, data); + + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(firstAmount + secondAmount); + }); + }); + + describe("Escrow", () => { + it("should not allow an escrow to the zero address", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + const tx = erc1238Holdable.connect(tokenOwner).entrust(ZERO_ADDRESS, tokenId, mintAmount); + + await expect(tx).to.be.revertedWith("ERC1238Holdable: transfer to the zero address"); + }); + + context("Full Escrow", () => { + it("should let a token owner put all their token in escrow", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + // tokenOwner entrusts tokenHolder1 with their tokens + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + + // tokenOwner does not hold the tokens anymore + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); + // tokenHolder1 does hold them + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(mintAmount); + // tokenOwner is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + + it("should let a holder transfer tokens to another holder", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + // tokenOwner entrusts tokenHolder1 with their tokens + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + // tokenHolder1 entrusts tokenHolder2 with these same tokens belonging to tokenOwner + await erc1238Holdable.connect(tokenHolder1).entrust(tokenHolder2.address, tokenId, mintAmount); + + // tokenOwner does not hold the tokens anymore + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); + // tokenHolder1 does not hold them + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); + // tokenHolder2 does hold them + expect(await erc1238Holdable.escrowedBalance(tokenHolder2.address, tokenId)).to.eq(mintAmount); + // tokenOwner is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + + it("should let a holder transfer tokens back to their owner", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + // tokenOwner entrusts tokenHolder1 with their tokens + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + // tokenHolder1 transfers them back to tokenOwner + await erc1238Holdable.connect(tokenHolder1).entrust(tokenOwner.address, tokenId, mintAmount); + + // tokenHolder1 does not hold the tokens anymore + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); + // tokenOwner does hold them + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount); + // tokenOwner is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + }); + + context("Partial Escrow", () => { + const escrowedAmount = mintAmount - 1000; + it("should let a token owner put all their token in escrow", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + // tokenOwner entrusts tokenHolder1 with some of their tokens + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + + // tokenOwner holds the remaining amount of tokens + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - escrowedAmount); + // tokenHolder1 holds the escrowed amount + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(escrowedAmount); + // tokenOwner is still the owner of all the tokens + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + + it("should let a token holder transfer the escrowed amount", async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + + // tokenOwner entrusts tokenHolder1 with some their tokens + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + // tokenHolder1 entrusts tokenHolder2 with these tokens + await erc1238Holdable.connect(tokenHolder1).entrust(tokenHolder2.address, tokenId, escrowedAmount); + + // tokenOwner holds the remaining amount of tokens + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - escrowedAmount); + // tokenHolder1 does not hold any tokens + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); + // tokenHolder2 does hold some of them + expect(await erc1238Holdable.escrowedBalance(tokenHolder2.address, tokenId)).to.eq(escrowedAmount); + // tokenOwner is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + }); + }); + }); + + describe("Burning", () => { + beforeEach(async () => { + await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + }); + + it("should let a token owner burn all of their tokens", async () => { + await erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, mintAmount); + + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(0); + }); + + it("should let a token owner burn some of their tokens", async () => { + const amountToBurn = mintAmount - 1000; + await erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, amountToBurn); + + expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); + }); + + it("should not give a token holder the right to burn tokens", async () => { + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + + await expect( + erc1238Holdable.connect(tokenHolder1).burn(tokenOwner.address, tokenId, mintAmount), + ).to.be.revertedWith("ERC1238Holdable: Unauthorized to burn tokens"); + }); + + it("should not let a token owner burn tokens they do not hold", async () => { + const escrowedAmount = 2000; + + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + + const amountHeldByOwner = mintAmount - escrowedAmount; + + await expect( + erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, amountHeldByOwner + 1), + ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); + }); + }); +}); From 9d99752e6651950b4e3c64e4ddfebdfe2b53e176 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Sat, 29 Jan 2022 15:07:58 +0000 Subject: [PATCH 04/18] small change --- contracts/ERC1238/extensions/ERC1238Stakable.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ERC1238/extensions/ERC1238Stakable.sol b/contracts/ERC1238/extensions/ERC1238Stakable.sol index 0a94374..0f011db 100644 --- a/contracts/ERC1238/extensions/ERC1238Stakable.sol +++ b/contracts/ERC1238/extensions/ERC1238Stakable.sol @@ -8,7 +8,7 @@ import "../ERC1238.sol"; * @dev Proposal for ERC1238 tokens extension that make them 'stakable' */ abstract contract ERC1238Stakable is ERC1238 { - // Mapping owner => tokenId => stakeholder => stake size + // Mapping owner => token id => stakeholder => stake size mapping(address => mapping(uint256 => mapping(address => uint256))) private _stakes; function _beforeBurn( From 737f3dc1969b41c9b6fcb0dc2e08be71db8968e5 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Wed, 2 Feb 2022 20:07:15 +0000 Subject: [PATCH 05/18] fix typos --- test/ERC1238/extensions/ERC1238Holdable.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts index 2b46a1a..71d30bd 100644 --- a/test/ERC1238/extensions/ERC1238Holdable.ts +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -1,8 +1,7 @@ -import { artifacts, ethers, waffle } from "hardhat"; -import type { Artifact } from "hardhat/types"; import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/dist/src/signer-with-address"; import { expect } from "chai"; - +import { artifacts, ethers, waffle } from "hardhat"; +import type { Artifact } from "hardhat/types"; import type { ERC1238HoldableMock } from "../../../src/types/ERC1238HoldableMock"; import { ZERO_ADDRESS } from "../../utils/test-utils"; @@ -33,13 +32,13 @@ describe("ERC1238URIHoldable", function () { }); describe("Minting", () => { - it("should set the the token recipient as first holder", async () => { + it("should set the token recipient as first holder", async () => { await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount); }); - it("should update the held balance when minting multiple times", async () => { + it("should update the balance held when minting multiple times", async () => { const firstAmount = 1000; const secondAmount = 200; await erc1238Holdable.mint(tokenOwner.address, tokenId, firstAmount, data); @@ -113,7 +112,7 @@ describe("ERC1238URIHoldable", function () { context("Partial Escrow", () => { const escrowedAmount = mintAmount - 1000; - it("should let a token owner put all their token in escrow", async () => { + it("should let a token owner put some of their token in escrow", async () => { await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); // tokenOwner entrusts tokenHolder1 with some of their tokens From 5848a760831e49dd506dbcd777a041fa235579cb Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Wed, 2 Feb 2022 21:55:42 +0000 Subject: [PATCH 06/18] add comment --- contracts/ERC1238/extensions/ERC1238Holdable.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index f96d4aa..ea6cdb1 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -40,6 +40,11 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { _escrowedBalances[burner][id] -= amount; } + /** + * @dev Lets sender entrusts `to` with `amount` + * of tokens which gets transferred between their respective escrowedBalances + * + */ function _entrust( address to, uint256 id, From 87119b0026fb5b4d81c192116d716b08075e8d5e Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Wed, 2 Feb 2022 21:56:32 +0000 Subject: [PATCH 07/18] decrease stakeholder stake when burning --- .../ERC1238/extensions/ERC1238Stakable.sol | 56 +++++++++++++------ contracts/mocks/ERC1238StakableMock.sol | 2 +- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/contracts/ERC1238/extensions/ERC1238Stakable.sol b/contracts/ERC1238/extensions/ERC1238Stakable.sol index 0f011db..cba3265 100644 --- a/contracts/ERC1238/extensions/ERC1238Stakable.sol +++ b/contracts/ERC1238/extensions/ERC1238Stakable.sol @@ -11,23 +11,38 @@ abstract contract ERC1238Stakable is ERC1238 { // Mapping owner => token id => stakeholder => stake size mapping(address => mapping(uint256 => mapping(address => uint256))) private _stakes; + function stakeOf( + address owner, + uint256 id, + address stakeholder + ) public view returns (uint256) { + return _stakes[owner][id][stakeholder]; + } + + /** + * @dev Called before tokens are burned + * + * Requirements: + * - `burner` and `from` are the same account OR + * - `from` entrusted `burner` with at least `amount` of tokens with id `id` + */ function _beforeBurn( address burner, address from, uint256 id, uint256 amount - ) internal view virtual override { - require(burner == from || _stakes[from][id][burner] >= amount, "ERC1238Stakable: Unauthorized to burn tokens"); + ) internal virtual override { + if (burner != from) { + require(_stakes[from][id][burner] >= amount, "ERC1238Stakable: Unauthorized to burn tokens"); + _decreaseStakeFrom(burner, from, id, amount); + } } /** * @dev Allows token owners to put their tokens at stake * * Calling this function again with the same stakeholder and id - * overrides the previous given allowance - * - * Requirements: - * + * adds to the previous staked amount * */ function _increaseStake( @@ -38,24 +53,33 @@ abstract contract ERC1238Stakable is ERC1238 { _stakes[msg.sender][id][stakeholder] += amount; } + /** + * @dev Lets sender (stakeholder) decrease a staked `amount` of + * tokens with id `id` belonging to `owner` + * + * Requirements: + * + * - `amount` must be less that the current staked amount + */ function _decreaseStake( address owner, uint256 id, uint256 amount ) internal { - uint256 authorization = _stakes[owner][id][msg.sender]; - - require(authorization >= amount, "ERC1238Stakable: cannot decrease more than current stake"); - unchecked { - _stakes[owner][id][msg.sender] = authorization - amount; - } + _decreaseStakeFrom(msg.sender, owner, id, amount); } - function stakeOf( + function _decreaseStakeFrom( + address stakeholder, address owner, uint256 id, - address stakeholder - ) public view returns (uint256) { - return _stakes[owner][id][stakeholder]; + uint256 amount + ) private { + uint256 authorization = _stakes[owner][id][stakeholder]; + + require(authorization >= amount, "ERC1238Stakable: cannot decrease more than current stake"); + unchecked { + _stakes[owner][id][stakeholder] = authorization - amount; + } } } diff --git a/contracts/mocks/ERC1238StakableMock.sol b/contracts/mocks/ERC1238StakableMock.sol index 78aeda3..fb2b244 100644 --- a/contracts/mocks/ERC1238StakableMock.sol +++ b/contracts/mocks/ERC1238StakableMock.sol @@ -16,7 +16,7 @@ contract ERC1238StakableMock is ERC1238, ERC1238Stakable { address from, uint256 id, uint256 amount - ) internal view override(ERC1238, ERC1238Stakable) { + ) internal override(ERC1238, ERC1238Stakable) { super._beforeBurn(burner, from, id, amount); } From 7bb7e084adf9bb729b96f0aa07342b1e4f08aca7 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Wed, 2 Feb 2022 21:56:48 +0000 Subject: [PATCH 08/18] add test --- test/ERC1238/extensions/ERC1238Stakable.ts | 27 +++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/test/ERC1238/extensions/ERC1238Stakable.ts b/test/ERC1238/extensions/ERC1238Stakable.ts index b93a8c6..bff4db8 100644 --- a/test/ERC1238/extensions/ERC1238Stakable.ts +++ b/test/ERC1238/extensions/ERC1238Stakable.ts @@ -1,8 +1,7 @@ -import { artifacts, ethers, waffle } from "hardhat"; -import type { Artifact } from "hardhat/types"; import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/dist/src/signer-with-address"; import { expect } from "chai"; - +import { artifacts, ethers, waffle } from "hardhat"; +import type { Artifact } from "hardhat/types"; import type { ERC1238StakableMock } from "../../../src/types/ERC1238StakableMock"; import { toBN } from "../../utils/test-utils"; @@ -86,6 +85,28 @@ describe("ERC1238URIStakable", function () { ).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); }); + it("should not let a stakeholder burn more than its stake in several transactions", async () => { + // Given + const stakedAmount = toBN("500"); + // burnAmount is half the staked amount + const burnAmount = stakedAmount.div(2); + + await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); + // burn half + await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); + // burn the other half + await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); + + expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq(0); + + // When + // Tries to burn more + const tx = erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); + + // Expect + await expect(tx).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); + }); + it("should let a token owner burn tokens before staking", async () => { // Given expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); From 6146807258e64513d17a892d43c099687d352ada Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Sun, 13 Feb 2022 11:52:30 -0700 Subject: [PATCH 09/18] update rules for burning --- .../ERC1238/extensions/ERC1238Holdable.sol | 1 - test/ERC1238/extensions/ERC1238Holdable.ts | 19 ++++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index ea6cdb1..d6f743c 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -34,7 +34,6 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { uint256 id, uint256 amount ) internal virtual override { - require(burner == from, "ERC1238Holdable: Unauthorized to burn tokens"); require(_escrowedBalances[burner][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); _escrowedBalances[burner][id] -= amount; diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts index 71d30bd..783791b 100644 --- a/test/ERC1238/extensions/ERC1238Holdable.ts +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -166,12 +166,25 @@ describe("ERC1238URIHoldable", function () { expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); }); - it("should not give a token holder the right to burn tokens", async () => { + it("should give a token holder the right to burn tokens", async () => { + const amountToBurn = mintAmount - 100; + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + await erc1238Holdable.connect(tokenHolder1).burn(tokenOwner.address, tokenId, amountToBurn); + + expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(mintAmount - amountToBurn); + expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); + }); + + it("should not let a token holder burn more tokens than they hodl", async () => { + const escrowedAmount = mintAmount; + + await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + await expect( - erc1238Holdable.connect(tokenHolder1).burn(tokenOwner.address, tokenId, mintAmount), - ).to.be.revertedWith("ERC1238Holdable: Unauthorized to burn tokens"); + erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, escrowedAmount + 1), + ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); }); it("should not let a token owner burn tokens they do not hold", async () => { From 800f273d982e088a6304bf126b4cdcd709563948 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Wed, 13 Apr 2022 18:38:39 +0100 Subject: [PATCH 10/18] remove stakable option --- .../ERC1238/extensions/ERC1238Stakable.sol | 85 -------- contracts/mocks/ERC1238StakableMock.sol | 55 ----- test/ERC1238/extensions/ERC1238Stakable.ts | 193 ------------------ 3 files changed, 333 deletions(-) delete mode 100644 contracts/ERC1238/extensions/ERC1238Stakable.sol delete mode 100644 contracts/mocks/ERC1238StakableMock.sol delete mode 100644 test/ERC1238/extensions/ERC1238Stakable.ts diff --git a/contracts/ERC1238/extensions/ERC1238Stakable.sol b/contracts/ERC1238/extensions/ERC1238Stakable.sol deleted file mode 100644 index cba3265..0000000 --- a/contracts/ERC1238/extensions/ERC1238Stakable.sol +++ /dev/null @@ -1,85 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "../ERC1238.sol"; - -/** - * @dev Proposal for ERC1238 tokens extension that make them 'stakable' - */ -abstract contract ERC1238Stakable is ERC1238 { - // Mapping owner => token id => stakeholder => stake size - mapping(address => mapping(uint256 => mapping(address => uint256))) private _stakes; - - function stakeOf( - address owner, - uint256 id, - address stakeholder - ) public view returns (uint256) { - return _stakes[owner][id][stakeholder]; - } - - /** - * @dev Called before tokens are burned - * - * Requirements: - * - `burner` and `from` are the same account OR - * - `from` entrusted `burner` with at least `amount` of tokens with id `id` - */ - function _beforeBurn( - address burner, - address from, - uint256 id, - uint256 amount - ) internal virtual override { - if (burner != from) { - require(_stakes[from][id][burner] >= amount, "ERC1238Stakable: Unauthorized to burn tokens"); - _decreaseStakeFrom(burner, from, id, amount); - } - } - - /** - * @dev Allows token owners to put their tokens at stake - * - * Calling this function again with the same stakeholder and id - * adds to the previous staked amount - * - */ - function _increaseStake( - address stakeholder, - uint256 id, - uint256 amount - ) internal { - _stakes[msg.sender][id][stakeholder] += amount; - } - - /** - * @dev Lets sender (stakeholder) decrease a staked `amount` of - * tokens with id `id` belonging to `owner` - * - * Requirements: - * - * - `amount` must be less that the current staked amount - */ - function _decreaseStake( - address owner, - uint256 id, - uint256 amount - ) internal { - _decreaseStakeFrom(msg.sender, owner, id, amount); - } - - function _decreaseStakeFrom( - address stakeholder, - address owner, - uint256 id, - uint256 amount - ) private { - uint256 authorization = _stakes[owner][id][stakeholder]; - - require(authorization >= amount, "ERC1238Stakable: cannot decrease more than current stake"); - unchecked { - _stakes[owner][id][stakeholder] = authorization - amount; - } - } -} diff --git a/contracts/mocks/ERC1238StakableMock.sol b/contracts/mocks/ERC1238StakableMock.sol deleted file mode 100644 index fb2b244..0000000 --- a/contracts/mocks/ERC1238StakableMock.sol +++ /dev/null @@ -1,55 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; - -import "../ERC1238/ERC1238.sol"; -import "../ERC1238/extensions/ERC1238Stakable.sol"; - -/** - * @dev Mock contract for ERC1238 tokens using ERC1238Stakable extension rendering them 'stakable' - */ -contract ERC1238StakableMock is ERC1238, ERC1238Stakable { - constructor(string memory uri) ERC1238(uri) {} - - function _beforeBurn( - address burner, - address from, - uint256 id, - uint256 amount - ) internal override(ERC1238, ERC1238Stakable) { - super._beforeBurn(burner, from, id, amount); - } - - function mint( - address to, - uint256 id, - uint256 amount, - bytes memory data - ) public { - _mint(to, id, amount, data); - } - - function burn( - address owner, - uint256 id, - uint256 amount - ) public { - _burn(owner, id, amount); - } - - function increaseStake( - address stakeholder, - uint256 id, - uint256 amount - ) external { - _increaseStake(stakeholder, id, amount); - } - - function decreaseStake( - address owner, - uint256 id, - uint256 amount - ) external { - _decreaseStake(owner, id, amount); - } -} diff --git a/test/ERC1238/extensions/ERC1238Stakable.ts b/test/ERC1238/extensions/ERC1238Stakable.ts deleted file mode 100644 index bff4db8..0000000 --- a/test/ERC1238/extensions/ERC1238Stakable.ts +++ /dev/null @@ -1,193 +0,0 @@ -import type { SignerWithAddress } from "@nomiclabs/hardhat-ethers/dist/src/signer-with-address"; -import { expect } from "chai"; -import { artifacts, ethers, waffle } from "hardhat"; -import type { Artifact } from "hardhat/types"; -import type { ERC1238StakableMock } from "../../../src/types/ERC1238StakableMock"; -import { toBN } from "../../utils/test-utils"; - -const BASE_URI = "https://token-cdn-domain/{id}.json"; - -describe("ERC1238URIStakable", function () { - let erc1238Stakable: ERC1238StakableMock; - let admin: SignerWithAddress; - let tokenOwner: SignerWithAddress; - let stakeholder: SignerWithAddress; - - before(async function () { - const signers: SignerWithAddress[] = await ethers.getSigners(); - admin = signers[0]; - tokenOwner = signers[1]; - stakeholder = signers[2]; - }); - - beforeEach(async function () { - const ERC1238StakableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238StakableMock"); - erc1238Stakable = await waffle.deployContract(admin, ERC1238StakableMockArtifact, [BASE_URI]); - }); - - describe("Staking", () => { - const fungibleTokenId = 1; - const amountMintedFungible = toBN("1000"); - const nftID = 2; - const amountMintedNonFungible = 1; - - beforeEach(async () => { - // mint fungible tokens - await erc1238Stakable.connect(admin).mint(tokenOwner.address, fungibleTokenId, amountMintedFungible, []); - // mint an NFT - await erc1238Stakable.connect(admin).mint(tokenOwner.address, nftID, amountMintedNonFungible, []); - }); - - it("should let a token owner increase a stake", async () => { - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, nftID, 1); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, nftID, stakeholder.address)).to.eq(1); - }); - - it("should let a stakeholder burn a staked NFT", async () => { - // Given - expect(await erc1238Stakable.balanceOf(tokenOwner.address, nftID)).to.eq(amountMintedNonFungible); - - // When - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, nftID, 1); - - await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, nftID, 1); - - // Expect - expect(await erc1238Stakable.balanceOf(tokenOwner.address, nftID)).to.eq(0); - }); - - it("should let a stakeholder burn up to the amount of fungible tokens staked", async () => { - // Given - const stakedAmount = toBN("500"); - - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); - - // When - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, stakedAmount); - - // Expect - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq( - amountMintedFungible.sub(stakedAmount), - ); - }); - - it("should not let a stakeholder burn more that the staked amount", async () => { - const stakedAmount = toBN("500"); - const burnAmount = stakedAmount.add(toBN("1")); - - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - await expect( - erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount), - ).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); - }); - - it("should not let a stakeholder burn more than its stake in several transactions", async () => { - // Given - const stakedAmount = toBN("500"); - // burnAmount is half the staked amount - const burnAmount = stakedAmount.div(2); - - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - // burn half - await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); - // burn the other half - await erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq(0); - - // When - // Tries to burn more - const tx = erc1238Stakable.connect(stakeholder).burn(tokenOwner.address, fungibleTokenId, burnAmount); - - // Expect - await expect(tx).to.be.revertedWith("ERC1238Stakable: Unauthorized to burn tokens"); - }); - - it("should let a token owner burn tokens before staking", async () => { - // Given - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); - - // When - await erc1238Stakable.connect(tokenOwner).burn(tokenOwner.address, fungibleTokenId, amountMintedFungible); - - // Expect - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(0); - }); - - it("should let a token owner burn tokens after staking", async () => { - // Given - const stakedAmount = toBN("800"); - - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq(amountMintedFungible); - - // When - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - await erc1238Stakable.connect(tokenOwner).burn(tokenOwner.address, fungibleTokenId, stakedAmount); - - // Expect - expect(await erc1238Stakable.balanceOf(tokenOwner.address, fungibleTokenId)).to.eq( - amountMintedFungible.sub(stakedAmount), - ); - // "Burn" allowance is the same - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount, - ); - }); - - context("Decrease Stake", () => { - it("should let a stakeholder decrease a stake", async () => { - const stakedAmount = toBN("800"); - const amountToUnstake = toBN("300"); - - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount, - ); - - await erc1238Stakable.connect(stakeholder).decreaseStake(tokenOwner.address, fungibleTokenId, amountToUnstake); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount.sub(amountToUnstake), - ); - }); - - it("should let a stakeholder fully unstake", async () => { - const stakedAmount = toBN("800"); - const amountToUnstake = stakedAmount; - - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount, - ); - - await erc1238Stakable.connect(stakeholder).decreaseStake(tokenOwner.address, fungibleTokenId, amountToUnstake); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq(0); - }); - - it("should not let a token owner unstake", async () => { - const stakedAmount = toBN("600"); - await erc1238Stakable.connect(tokenOwner).increaseStake(stakeholder.address, fungibleTokenId, stakedAmount); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount, - ); - - await expect( - erc1238Stakable.connect(tokenOwner).decreaseStake(tokenOwner.address, fungibleTokenId, stakedAmount), - ).to.be.revertedWith("ERC1238Stakable: cannot decrease more than current stake"); - - expect(await erc1238Stakable.stakeOf(tokenOwner.address, fungibleTokenId, stakeholder.address)).to.eq( - stakedAmount, - ); - }); - }); - }); -}); From 7dd90d5c32b4298d50093967ea39b6ed65c60fa9 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 10:12:19 +0100 Subject: [PATCH 11/18] update holdable --- .../ERC1238/extensions/ERC1238Holdable.sol | 42 ++++++++++++------- .../ERC1238/extensions/IERC1238Holdable.sol | 15 +++---- .../ERC1238/extensions/IERC1238Holder.sol | 15 +++++++ 3 files changed, 46 insertions(+), 26 deletions(-) create mode 100644 contracts/ERC1238/extensions/IERC1238Holder.sol diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index d6f743c..4d2795c 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -4,44 +4,56 @@ pragma solidity ^0.8.0; import "../ERC1238.sol"; import "./IERC1238Holdable.sol"; +import "./IERC1238Holder.sol"; /** * @dev Proposal for ERC1238 tokens extension that allow addresses - * to hold tokens on behalf of others (escrow) + * to hold tokens on behalf of others. */ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { // Mapping holder => id => balance - mapping(address => mapping(uint256 => uint256)) private _escrowedBalances; + mapping(address => mapping(uint256 => uint256)) private _heldBalances; - function escrowedBalance(address holder, uint256 id) public view override returns (uint256) { - return _escrowedBalances[holder][id]; + event BurnAcknowledgmentFailed(address holder, address burner, address from, uint256 indexed id, uint256 amount); + + function heldBalance(address holder, uint256 id) public view override returns (uint256) { + return _heldBalances[holder][id]; } function _beforeMint( - address minter, + address, address to, uint256 id, uint256 amount, - bytes memory data + bytes memory ) internal virtual override { // set the token recipient as first holder by default when tokens are minted - _escrowedBalances[to][id] += amount; + _heldBalances[to][id] += amount; } function _beforeBurn( + address holder, address burner, address from, uint256 id, uint256 amount - ) internal virtual override { - require(_escrowedBalances[burner][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); + ) internal virtual { + require(_heldBalances[holder][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); - _escrowedBalances[burner][id] -= amount; + super._beforeBurn(burner, from, id, amount); + + try IERC1238Holder(holder).onBurnAcknowledged(id, amount) returns (bool isBurnAcknowledged) { + if (!isBurnAcknowledged) emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); + } catch { + emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); + } + + _heldBalances[burner][id] -= amount; } /** * @dev Lets sender entrusts `to` with `amount` - * of tokens which gets transferred between their respective escrowedBalances + * of tokens which gets transferred between their respective heldBalances * */ function _entrust( @@ -49,15 +61,13 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { uint256 id, uint256 amount ) internal virtual { - require(to != address(0), "ERC1238Holdable: transfer to the zero address"); - address from = msg.sender; - uint256 fromBalance = _escrowedBalances[from][id]; + uint256 fromBalance = _heldBalances[from][id]; require(fromBalance >= amount, "ERC1238Holdable: amount exceeds balance held"); - _escrowedBalances[from][id] -= amount; - _escrowedBalances[to][id] += amount; + _heldBalances[from][id] -= amount; + _heldBalances[to][id] += amount; emit Entrust(from, to, id, amount); } diff --git a/contracts/ERC1238/extensions/IERC1238Holdable.sol b/contracts/ERC1238/extensions/IERC1238Holdable.sol index 172bfad..65f5c25 100644 --- a/contracts/ERC1238/extensions/IERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/IERC1238Holdable.sol @@ -5,22 +5,17 @@ pragma solidity ^0.8.0; import "../IERC1238.sol"; /** - * @dev Proposal of an interface for ERC1238 token with storage based token URI management. + * @dev Proposal of an interface for ERC1238 tokens that can be held by another address than + * than their owner or staked in a smart contract. */ interface IERC1238Holdable is IERC1238 { /** - * @dev Event emitted when `from` entrusts `to` with `amount` of tokens with token `id` + * @dev Event emitted when `from` entrusts `to` with `amount` of tokens with token `id`. */ event Entrust(address from, address to, uint256 indexed id, uint256 amount); /** - * @dev Returns the balance of a token holder for a given `id` + * @dev Returns the balance of a token holder for a given `id`. */ - function escrowedBalance(address holder, uint256 id) external view returns (uint256); - - function entrust( - address to, - uint256 id, - uint256 amount - ) external; + function heldBalance(address holder, uint256 id) external view returns (uint256); } diff --git a/contracts/ERC1238/extensions/IERC1238Holder.sol b/contracts/ERC1238/extensions/IERC1238Holder.sol new file mode 100644 index 0000000..9d7643d --- /dev/null +++ b/contracts/ERC1238/extensions/IERC1238Holder.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "../IERC1238.sol"; +import "../IERC1238Receiver.sol"; + +/** + * @dev Interface proposal for contracts that need to hold ERC1238 tokens. + */ +interface IERC1238Holder is IERC1238Receiver { + /** + * @dev This function is called when tokens with id `id` are burnt. + */ + function onBurnAcknowledged(uint256 id, uint256 amount) external returns (bool); +} From 747ddee2f28509f62dde56ec24255c26e17629cd Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 11:16:05 +0100 Subject: [PATCH 12/18] update with burnHeldTokens --- contracts/ERC1238/extensions/ERC1238Holdable.sol | 10 +++++----- contracts/ERC1238/extensions/IERC1238Holdable.sol | 6 ++++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index 4d2795c..b011b1f 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -31,24 +31,24 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { _heldBalances[to][id] += amount; } - function _beforeBurn( - address holder, + function _burnHeldTokens( address burner, + address holder, address from, uint256 id, uint256 amount ) internal virtual { require(_heldBalances[holder][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); - super._beforeBurn(burner, from, id, amount); - try IERC1238Holder(holder).onBurnAcknowledged(id, amount) returns (bool isBurnAcknowledged) { if (!isBurnAcknowledged) emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); } catch { emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); } - _heldBalances[burner][id] -= amount; + super._burn(from, id, amount); + + _heldBalances[holder][id] -= amount; } /** diff --git a/contracts/ERC1238/extensions/IERC1238Holdable.sol b/contracts/ERC1238/extensions/IERC1238Holdable.sol index 65f5c25..8d239c5 100644 --- a/contracts/ERC1238/extensions/IERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/IERC1238Holdable.sol @@ -18,4 +18,10 @@ interface IERC1238Holdable is IERC1238 { * @dev Returns the balance of a token holder for a given `id`. */ function heldBalance(address holder, uint256 id) external view returns (uint256); + + function entrust( + address to, + uint256 id, + uint256 amount + ) external; } From fe9c54b8471860e6aced2677fc0787c1975fb1a5 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 11:16:28 +0100 Subject: [PATCH 13/18] update holdable mocks --- contracts/mocks/ERC1238HoldableMock.sol | 26 ++++------ .../mocks/ERC1238ReceiverHoldableMock.sol | 52 +++++++++++++++++++ 2 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 contracts/mocks/ERC1238ReceiverHoldableMock.sol diff --git a/contracts/mocks/ERC1238HoldableMock.sol b/contracts/mocks/ERC1238HoldableMock.sol index c167ba2..9dab524 100644 --- a/contracts/mocks/ERC1238HoldableMock.sol +++ b/contracts/mocks/ERC1238HoldableMock.sol @@ -21,39 +21,31 @@ contract ERC1238HoldableMock is ERC1238, ERC1238Holdable { super._beforeMint(minter, to, id, amount, data); } - function _beforeBurn( - address burner, - address from, - uint256 id, - uint256 amount - ) internal override(ERC1238, ERC1238Holdable) { - super._beforeBurn(burner, from, id, amount); - } - - function mint( + function mintToContract( address to, uint256 id, uint256 amount, bytes memory data ) public { - _mint(to, id, amount, data); + _mintToContract(to, id, amount, data); } - function mintBatch( + function mintBatchToContract( address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data ) public { - _mintBatch(to, ids, amounts, data); + _mintBatchToContract(to, ids, amounts, data); } - function burn( - address owner, + function burnHeldTokens( + address holder, + address from, uint256 id, uint256 amount ) public { - _burn(owner, id, amount); + _burnHeldTokens(msg.sender, holder, from, id, amount); } function burnBatch( @@ -68,7 +60,7 @@ contract ERC1238HoldableMock is ERC1238, ERC1238Holdable { address to, uint256 id, uint256 amount - ) public override { + ) public { _entrust(to, id, amount); } } diff --git a/contracts/mocks/ERC1238ReceiverHoldableMock.sol b/contracts/mocks/ERC1238ReceiverHoldableMock.sol new file mode 100644 index 0000000..c7cb2b5 --- /dev/null +++ b/contracts/mocks/ERC1238ReceiverHoldableMock.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238/IERC1238Receiver.sol"; +import "../ERC1238/extensions/IERC1238Holdable.sol"; + +// This is a dummy example of a ERC1238Receiver with arbitrary rules. +// It will reject non-transferable tokens if the token id is 0 in the case of a single token mint +// or if the first token id is 0 for a batch mint. +contract ERC1238ReceiverHoldableMock is IERC1238Receiver { + // bytes4(keccak256("onERC1238Mint(address,uint256,uint256,bytes)")) + bytes4 public constant ERC1238_ON_MINT = 0x45ed75d5; + + // bytes4(keccak256("onERC1238BatchMint(address,uint256[],uint256[],bytes)")) + bytes4 public constant ERC1238_ON_BATCH_MINT = 0xc0bfec68; + + function onERC1238Mint( + address, + uint256 id, + uint256, + bytes calldata + ) external pure override returns (bytes4) { + if (id == 0) { + return bytes4(0); + } + + return ERC1238_ON_MINT; + } + + function onERC1238BatchMint( + address, + uint256[] calldata ids, + uint256[] calldata, + bytes calldata + ) external pure override returns (bytes4) { + if (ids[0] == 0) { + return bytes4(0); + } + + return ERC1238_ON_BATCH_MINT; + } + + function entrust( + address targetContract, + address to, + uint256 id, + uint256 amount + ) external { + IERC1238Holdable(targetContract).entrust(to, id, amount); + } +} From 45ebeced93288210a26dea13b894b909e6b71e42 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 11:16:41 +0100 Subject: [PATCH 14/18] update tests --- test/ERC1238/extensions/ERC1238Holdable.ts | 250 +++++++++++---------- 1 file changed, 128 insertions(+), 122 deletions(-) diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts index 783791b..b84a945 100644 --- a/test/ERC1238/extensions/ERC1238Holdable.ts +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -3,6 +3,7 @@ import { expect } from "chai"; import { artifacts, ethers, waffle } from "hardhat"; import type { Artifact } from "hardhat/types"; import type { ERC1238HoldableMock } from "../../../src/types/ERC1238HoldableMock"; +import type { ERC1238ReceiverHoldableMock } from "../../../src/types/ERC1238ReceiverHoldableMock"; import { ZERO_ADDRESS } from "../../utils/test-utils"; const BASE_URI = "https://token-cdn-domain/{id}.json"; @@ -10,9 +11,9 @@ const BASE_URI = "https://token-cdn-domain/{id}.json"; describe("ERC1238URIHoldable", function () { let erc1238Holdable: ERC1238HoldableMock; let admin: SignerWithAddress; - let tokenOwner: SignerWithAddress; - let tokenHolder1: SignerWithAddress; - let tokenHolder2: SignerWithAddress; + let tokenOwnerContract: ERC1238ReceiverHoldableMock; + let eoa1: SignerWithAddress; + let eoa2: SignerWithAddress; const tokenId = 888888; const mintAmount = 98765432; @@ -21,182 +22,187 @@ describe("ERC1238URIHoldable", function () { before(async function () { const signers: SignerWithAddress[] = await ethers.getSigners(); admin = signers[0]; - tokenOwner = signers[1]; - tokenHolder1 = signers[2]; - tokenHolder2 = signers[3]; + eoa1 = signers[1]; + eoa2 = signers[2]; }); beforeEach(async function () { const ERC1238HoldableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238HoldableMock"); erc1238Holdable = await waffle.deployContract(admin, ERC1238HoldableMockArtifact, [BASE_URI]); + + const ERC1238ReceiverHoldableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238ReceiverHoldableMock"); + tokenOwnerContract = ( + await waffle.deployContract(admin, ERC1238ReceiverHoldableMockArtifact) + ); }); describe("Minting", () => { it("should set the token recipient as first holder", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount); + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); it("should update the balance held when minting multiple times", async () => { const firstAmount = 1000; const secondAmount = 200; - await erc1238Holdable.mint(tokenOwner.address, tokenId, firstAmount, data); + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, firstAmount, data); - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(firstAmount); + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(firstAmount); - await erc1238Holdable.mint(tokenOwner.address, tokenId, secondAmount, data); + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, secondAmount, data); - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(firstAmount + secondAmount); + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(firstAmount + secondAmount); }); }); - describe("Escrow", () => { - it("should not allow an escrow to the zero address", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + describe("Holding", () => { + it("should allow locking tokens forever at the zero address", async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); - const tx = erc1238Holdable.connect(tokenOwner).entrust(ZERO_ADDRESS, tokenId, mintAmount); + await tokenOwnerContract.entrust(erc1238Holdable.address, ZERO_ADDRESS, tokenId, mintAmount); - await expect(tx).to.be.revertedWith("ERC1238Holdable: transfer to the zero address"); + expect(await erc1238Holdable.heldBalance(ZERO_ADDRESS, tokenId)).to.eq(mintAmount); }); - context("Full Escrow", () => { - it("should let a token owner put all their token in escrow", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + context("Staking all tokens", () => { + it("should let a token owner stake all of their tokens", async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + + // tokenOwnerContract entrusts eoa1 with their tokens + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, mintAmount); - // tokenOwner entrusts tokenHolder1 with their tokens - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + // tokenOwnerContract does not hold the tokens anymore + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(0); + // eoa1 does hold them + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(mintAmount); - // tokenOwner does not hold the tokens anymore - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); - // tokenHolder1 does hold them - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(mintAmount); - // tokenOwner is still the owner of these tokens - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + // tokenOwnerContract is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); it("should let a holder transfer tokens to another holder", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); - - // tokenOwner entrusts tokenHolder1 with their tokens - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); - // tokenHolder1 entrusts tokenHolder2 with these same tokens belonging to tokenOwner - await erc1238Holdable.connect(tokenHolder1).entrust(tokenHolder2.address, tokenId, mintAmount); - - // tokenOwner does not hold the tokens anymore - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); - // tokenHolder1 does not hold them - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); - // tokenHolder2 does hold them - expect(await erc1238Holdable.escrowedBalance(tokenHolder2.address, tokenId)).to.eq(mintAmount); - // tokenOwner is still the owner of these tokens - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + + // tokenOwnerContract entrusts eoa1 with their tokens + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, mintAmount); + // eoa1 entrusts eoa2 with these same tokens belonging to tokenOwnerContract + await erc1238Holdable.connect(eoa1).entrust(eoa2.address, tokenId, mintAmount); + + // tokenOwnerContract does not hold the tokens anymore + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(0); + // eoa1 does not hold them + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(0); + // eoa2 does hold them + expect(await erc1238Holdable.heldBalance(eoa2.address, tokenId)).to.eq(mintAmount); + // tokenOwnerContract is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); it("should let a holder transfer tokens back to their owner", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); - - // tokenOwner entrusts tokenHolder1 with their tokens - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); - // tokenHolder1 transfers them back to tokenOwner - await erc1238Holdable.connect(tokenHolder1).entrust(tokenOwner.address, tokenId, mintAmount); - - // tokenHolder1 does not hold the tokens anymore - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); - // tokenOwner does hold them - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount); - // tokenOwner is still the owner of these tokens - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + + // tokenOwnerContract entrusts eoa1 with their tokens + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, mintAmount); + // eoa1 transfers them back to tokenOwnerContract + await erc1238Holdable.connect(eoa1).entrust(tokenOwnerContract.address, tokenId, mintAmount); + + // eoa1 does not hold the tokens anymore + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(0); + // tokenOwnerContract does hold them + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); + // tokenOwnerContract is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); }); - context("Partial Escrow", () => { - const escrowedAmount = mintAmount - 1000; - it("should let a token owner put some of their token in escrow", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); + context("Partial Staking", () => { + const stakedAmount = mintAmount - 1000; + it("should let a token owner put some of their token at stake", async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); - // tokenOwner entrusts tokenHolder1 with some of their tokens - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + // tokenOwnerContract entrusts eoa1 with some of their tokens + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, stakedAmount); - // tokenOwner holds the remaining amount of tokens - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - escrowedAmount); - // tokenHolder1 holds the escrowed amount - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(escrowedAmount); - // tokenOwner is still the owner of all the tokens - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + // tokenOwnerContract holds the remaining amount of tokens + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - stakedAmount); + // eoa1 holds the staked amount + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(stakedAmount); + // tokenOwnerContract is still the owner of all the tokens + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); - it("should let a token holder transfer the escrowed amount", async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); - - // tokenOwner entrusts tokenHolder1 with some their tokens - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); - // tokenHolder1 entrusts tokenHolder2 with these tokens - await erc1238Holdable.connect(tokenHolder1).entrust(tokenHolder2.address, tokenId, escrowedAmount); - - // tokenOwner holds the remaining amount of tokens - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - escrowedAmount); - // tokenHolder1 does not hold any tokens - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(0); - // tokenHolder2 does hold some of them - expect(await erc1238Holdable.escrowedBalance(tokenHolder2.address, tokenId)).to.eq(escrowedAmount); - // tokenOwner is still the owner of these tokens - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount); + it("should let a token holder transfer the staked amount", async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + + // tokenOwnerContract entrusts eoa1 with some their tokens + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, stakedAmount); + // eoa1 entrusts eoa2 with these tokens + await erc1238Holdable.connect(eoa1).entrust(eoa2.address, tokenId, stakedAmount); + + // tokenOwnerContract holds the remaining amount of tokens + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - stakedAmount); + // eoa1 does not hold any tokens + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(0); + // eoa2 does hold some of them + expect(await erc1238Holdable.heldBalance(eoa2.address, tokenId)).to.eq(stakedAmount); + // tokenOwnerContract is still the owner of these tokens + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount); }); }); }); - describe("Burning", () => { - beforeEach(async () => { - await erc1238Holdable.mint(tokenOwner.address, tokenId, mintAmount, data); - }); + // describe("Burning", () => { + // beforeEach(async () => { + // await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + // }); - it("should let a token owner burn all of their tokens", async () => { - await erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, mintAmount); + // it("should let a token owner burn all of their tokens", async () => { + // await erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, mintAmount); - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(0); - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(0); - }); + // expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(0); + // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(0); + // }); - it("should let a token owner burn some of their tokens", async () => { - const amountToBurn = mintAmount - 1000; - await erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, amountToBurn); + // it("should let a token owner burn some of their tokens", async () => { + // const amountToBurn = mintAmount - 1000; + // await erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, amountToBurn); - expect(await erc1238Holdable.escrowedBalance(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); - }); + // expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + // }); - it("should give a token holder the right to burn tokens", async () => { - const amountToBurn = mintAmount - 100; + // it("should give a token holder the right to burn tokens", async () => { + // const amountToBurn = mintAmount - 100; - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, mintAmount); + // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, mintAmount); - await erc1238Holdable.connect(tokenHolder1).burn(tokenOwner.address, tokenId, amountToBurn); + // await erc1238Holdable.connect(eoa1).burn(tokenOwnerContract.address, tokenId, amountToBurn); - expect(await erc1238Holdable.escrowedBalance(tokenHolder1.address, tokenId)).to.eq(mintAmount - amountToBurn); - expect(await erc1238Holdable.balanceOf(tokenOwner.address, tokenId)).to.eq(mintAmount - amountToBurn); - }); + // expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(mintAmount - amountToBurn); + // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + // }); - it("should not let a token holder burn more tokens than they hodl", async () => { - const escrowedAmount = mintAmount; + // it("should not let a token holder burn more tokens than they hodl", async () => { + // const stakedAmount = mintAmount; - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, escrowedAmount); - await expect( - erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, escrowedAmount + 1), - ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); - }); + // await expect( + // erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, escrowedAmount + 1), + // ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); + // }); - it("should not let a token owner burn tokens they do not hold", async () => { - const escrowedAmount = 2000; + // it("should not let a token owner burn tokens they do not hold", async () => { + // const escrowedAmount = 2000; - await erc1238Holdable.connect(tokenOwner).entrust(tokenHolder1.address, tokenId, escrowedAmount); + // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, escrowedAmount); - const amountHeldByOwner = mintAmount - escrowedAmount; + // const amountHeldByOwner = mintAmount - escrowedAmount; - await expect( - erc1238Holdable.connect(tokenOwner).burn(tokenOwner.address, tokenId, amountHeldByOwner + 1), - ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); - }); - }); + // await expect( + // erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, amountHeldByOwner + 1), + // ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); + // }); + // }); }); From c55b58306f0d720aba6134f54bc4c5660bfeec95 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:51:44 +0100 Subject: [PATCH 15/18] move event burn acknowledgment event --- .../ERC1238/extensions/ERC1238Holdable.sol | 17 +++++++++++------ .../ERC1238/extensions/IERC1238Holdable.sol | 5 +++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index b011b1f..9fb96b2 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -11,11 +11,11 @@ import "./IERC1238Holder.sol"; * to hold tokens on behalf of others. */ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { + using Address for address; + // Mapping holder => id => balance mapping(address => mapping(uint256 => uint256)) private _heldBalances; - event BurnAcknowledgmentFailed(address holder, address burner, address from, uint256 indexed id, uint256 amount); - function heldBalance(address holder, uint256 id) public view override returns (uint256) { return _heldBalances[holder][id]; } @@ -40,10 +40,12 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { ) internal virtual { require(_heldBalances[holder][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); - try IERC1238Holder(holder).onBurnAcknowledged(id, amount) returns (bool isBurnAcknowledged) { - if (!isBurnAcknowledged) emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); - } catch { - emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); + if (holder.isContract()) { + try IERC1238Holder(holder).onBurnAcknowledged(id, amount) returns (bool isBurnAcknowledged) { + if (!isBurnAcknowledged) emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); + } catch { + emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); + } } super._burn(from, id, amount); @@ -71,4 +73,7 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { emit Entrust(from, to, id, amount); } + + // TODO: entrustHolderContract to provide safer alternative which + // makes sure the recipient is a IERC1238Holder contract } diff --git a/contracts/ERC1238/extensions/IERC1238Holdable.sol b/contracts/ERC1238/extensions/IERC1238Holdable.sol index 8d239c5..2aba77e 100644 --- a/contracts/ERC1238/extensions/IERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/IERC1238Holdable.sol @@ -14,6 +14,11 @@ interface IERC1238Holdable is IERC1238 { */ event Entrust(address from, address to, uint256 indexed id, uint256 amount); + /** + * @dev Event emitted when tokens are burnt and the holder fails to acknowledge the burn. + */ + event BurnAcknowledgmentFailed(address holder, address burner, address from, uint256 indexed id, uint256 amount); + /** * @dev Returns the balance of a token holder for a given `id`. */ From d1590c6c51462b9d1f7ad90daeac5c517d15f47d Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 14:52:06 +0100 Subject: [PATCH 16/18] update tests around burning --- .../mocks/ERC1238ReceiverHoldableMock.sol | 11 +++ test/ERC1238/extensions/ERC1238Holdable.ts | 77 +++++++++---------- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/contracts/mocks/ERC1238ReceiverHoldableMock.sol b/contracts/mocks/ERC1238ReceiverHoldableMock.sol index c7cb2b5..a46e30e 100644 --- a/contracts/mocks/ERC1238ReceiverHoldableMock.sol +++ b/contracts/mocks/ERC1238ReceiverHoldableMock.sol @@ -5,6 +5,8 @@ pragma solidity ^0.8.0; import "../ERC1238/IERC1238Receiver.sol"; import "../ERC1238/extensions/IERC1238Holdable.sol"; +import "./ERC1238HoldableMock.sol"; + // This is a dummy example of a ERC1238Receiver with arbitrary rules. // It will reject non-transferable tokens if the token id is 0 in the case of a single token mint // or if the first token id is 0 for a batch mint. @@ -49,4 +51,13 @@ contract ERC1238ReceiverHoldableMock is IERC1238Receiver { ) external { IERC1238Holdable(targetContract).entrust(to, id, amount); } + + function burnHeldTokens( + address targetContract, + address holder, + uint256 id, + uint256 amount + ) external { + ERC1238HoldableMock(targetContract).burnHeldTokens(holder, address(this), id, amount); + } } diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts index b84a945..cc0594c 100644 --- a/test/ERC1238/extensions/ERC1238Holdable.ts +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -12,6 +12,7 @@ describe("ERC1238URIHoldable", function () { let erc1238Holdable: ERC1238HoldableMock; let admin: SignerWithAddress; let tokenOwnerContract: ERC1238ReceiverHoldableMock; + // let tokenOwnerContract2: ERC1238ReceiverHoldableMock; let eoa1: SignerWithAddress; let eoa2: SignerWithAddress; @@ -152,57 +153,51 @@ describe("ERC1238URIHoldable", function () { }); }); - // describe("Burning", () => { - // beforeEach(async () => { - // await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); - // }); - - // it("should let a token owner burn all of their tokens", async () => { - // await erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, mintAmount); - - // expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(0); - // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(0); - // }); - - // it("should let a token owner burn some of their tokens", async () => { - // const amountToBurn = mintAmount - 1000; - // await erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, amountToBurn); + describe("Burning", () => { + beforeEach(async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + }); - // expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); - // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); - // }); + it("should let a token owner burn all of their tokens", async () => { + await tokenOwnerContract.burnHeldTokens(erc1238Holdable.address, tokenOwnerContract.address, tokenId, mintAmount); - // it("should give a token holder the right to burn tokens", async () => { - // const amountToBurn = mintAmount - 100; + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(0); + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(0); + }); - // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, mintAmount); + it("should let a token owner burn some of their tokens", async () => { + const amountToBurn = mintAmount - 1000; - // await erc1238Holdable.connect(eoa1).burn(tokenOwnerContract.address, tokenId, amountToBurn); + await tokenOwnerContract.burnHeldTokens( + erc1238Holdable.address, + tokenOwnerContract.address, + tokenId, + amountToBurn, + ); - // expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(mintAmount - amountToBurn); - // expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); - // }); + expect(await erc1238Holdable.heldBalance(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + }); - // it("should not let a token holder burn more tokens than they hodl", async () => { - // const stakedAmount = mintAmount; + it("should let tokens held by an EOA be burnt", async () => { + const amountToBurn = mintAmount - 100; - // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, escrowedAmount); + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, amountToBurn); - // await expect( - // erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, escrowedAmount + 1), - // ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); - // }); + await erc1238Holdable.burnHeldTokens(eoa1.address, tokenOwnerContract.address, tokenId, amountToBurn); - // it("should not let a token owner burn tokens they do not hold", async () => { - // const escrowedAmount = 2000; + expect(await erc1238Holdable.heldBalance(eoa1.address, tokenId)).to.eq(0); + expect(await erc1238Holdable.balanceOf(tokenOwnerContract.address, tokenId)).to.eq(mintAmount - amountToBurn); + }); - // await erc1238Holdable.connect(tokenOwnerContract).entrust(eoa1.address, tokenId, escrowedAmount); + it("should revert when trying to burn more tokens that what the holder passed holds", async () => { + const stakedAmount = mintAmount; - // const amountHeldByOwner = mintAmount - escrowedAmount; + await tokenOwnerContract.entrust(erc1238Holdable.address, eoa1.address, tokenId, stakedAmount); - // await expect( - // erc1238Holdable.connect(tokenOwnerContract).burn(tokenOwnerContract.address, tokenId, amountHeldByOwner + 1), - // ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); - // }); - // }); + await expect( + erc1238Holdable.burnHeldTokens(eoa1.address, tokenOwnerContract.address, tokenId, stakedAmount + 1), + ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); + }); + }); }); From 6915a1b6821063ab67057f92683e85d1a3464446 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 15:56:36 +0100 Subject: [PATCH 17/18] add tests for burn notification and minor improvements --- .../ERC1238/extensions/ERC1238Holdable.sol | 2 +- .../ERC1238/extensions/IERC1238Holdable.sol | 5 ++ .../ERC1238/extensions/IERC1238Holder.sol | 2 +- contracts/mocks/ERC1238HoldableMock.sol | 2 +- contracts/mocks/ERC1238HolderMock.sol | 65 +++++++++++++++++++ test/ERC1238/extensions/ERC1238Holdable.ts | 53 ++++++++++++++- 6 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 contracts/mocks/ERC1238HolderMock.sol diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index 9fb96b2..45f73dc 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -41,7 +41,7 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { require(_heldBalances[holder][id] >= amount, "ERC1238Holdable: Amount to burn exceeds amount held"); if (holder.isContract()) { - try IERC1238Holder(holder).onBurnAcknowledged(id, amount) returns (bool isBurnAcknowledged) { + try IERC1238Holder(holder).onBurn(id, amount) returns (bool isBurnAcknowledged) { if (!isBurnAcknowledged) emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); } catch { emit BurnAcknowledgmentFailed(holder, burner, from, id, amount); diff --git a/contracts/ERC1238/extensions/IERC1238Holdable.sol b/contracts/ERC1238/extensions/IERC1238Holdable.sol index 2aba77e..ba00afa 100644 --- a/contracts/ERC1238/extensions/IERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/IERC1238Holdable.sol @@ -24,6 +24,11 @@ interface IERC1238Holdable is IERC1238 { */ function heldBalance(address holder, uint256 id) external view returns (uint256); + /** + * @dev Lets sender entrusts `to` with `amount` + * of tokens which gets transferred between their respective balances + * of tokens held. + */ function entrust( address to, uint256 id, diff --git a/contracts/ERC1238/extensions/IERC1238Holder.sol b/contracts/ERC1238/extensions/IERC1238Holder.sol index 9d7643d..3e8ca40 100644 --- a/contracts/ERC1238/extensions/IERC1238Holder.sol +++ b/contracts/ERC1238/extensions/IERC1238Holder.sol @@ -11,5 +11,5 @@ interface IERC1238Holder is IERC1238Receiver { /** * @dev This function is called when tokens with id `id` are burnt. */ - function onBurnAcknowledged(uint256 id, uint256 amount) external returns (bool); + function onBurn(uint256 id, uint256 amount) external returns (bool); } diff --git a/contracts/mocks/ERC1238HoldableMock.sol b/contracts/mocks/ERC1238HoldableMock.sol index 9dab524..84ce71c 100644 --- a/contracts/mocks/ERC1238HoldableMock.sol +++ b/contracts/mocks/ERC1238HoldableMock.sol @@ -60,7 +60,7 @@ contract ERC1238HoldableMock is ERC1238, ERC1238Holdable { address to, uint256 id, uint256 amount - ) public { + ) public override { _entrust(to, id, amount); } } diff --git a/contracts/mocks/ERC1238HolderMock.sol b/contracts/mocks/ERC1238HolderMock.sol new file mode 100644 index 0000000..a720fe9 --- /dev/null +++ b/contracts/mocks/ERC1238HolderMock.sol @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC1238/IERC1238Receiver.sol"; +import "../ERC1238/extensions/IERC1238Holdable.sol"; +import "../ERC1238/extensions/IERC1238Holder.sol"; + +import "./ERC1238HoldableMock.sol"; + +// This is a dummy example of a ERC1238Holder with arbitrary rules. +// It will reject non-transferable tokens if the token id is 0 in the case of a single token mint +// or if the first token id is 0 for a batch mint. +// +// It will also acknowledge and emit an event when tokens are burnt. +contract ERC1238HolderMock is IERC1238Receiver, IERC1238Holder { + // bytes4(keccak256("onERC1238Mint(address,uint256,uint256,bytes)")) + bytes4 public constant ERC1238_ON_MINT = 0x45ed75d5; + + // bytes4(keccak256("onERC1238BatchMint(address,uint256[],uint256[],bytes)")) + bytes4 public constant ERC1238_ON_BATCH_MINT = 0xc0bfec68; + + event TokenBurnt(uint256 id, uint256 amount); + + function onERC1238Mint( + address, + uint256 id, + uint256, + bytes calldata + ) external pure override returns (bytes4) { + if (id == 0) { + return bytes4(0); + } + + return ERC1238_ON_MINT; + } + + function onERC1238BatchMint( + address, + uint256[] calldata ids, + uint256[] calldata, + bytes calldata + ) external pure override returns (bytes4) { + if (ids[0] == 0) { + return bytes4(0); + } + + return ERC1238_ON_BATCH_MINT; + } + + function entrust( + address targetContract, + address to, + uint256 id, + uint256 amount + ) external { + IERC1238Holdable(targetContract).entrust(to, id, amount); + } + + function onBurn(uint256 id, uint256 amount) public override returns (bool) { + emit TokenBurnt(id, amount); + + return true; + } +} diff --git a/test/ERC1238/extensions/ERC1238Holdable.ts b/test/ERC1238/extensions/ERC1238Holdable.ts index cc0594c..94db8e3 100644 --- a/test/ERC1238/extensions/ERC1238Holdable.ts +++ b/test/ERC1238/extensions/ERC1238Holdable.ts @@ -3,6 +3,7 @@ import { expect } from "chai"; import { artifacts, ethers, waffle } from "hardhat"; import type { Artifact } from "hardhat/types"; import type { ERC1238HoldableMock } from "../../../src/types/ERC1238HoldableMock"; +import type { ERC1238HolderMock } from "../../../src/types/ERC1238HolderMock"; import type { ERC1238ReceiverHoldableMock } from "../../../src/types/ERC1238ReceiverHoldableMock"; import { ZERO_ADDRESS } from "../../utils/test-utils"; @@ -12,7 +13,6 @@ describe("ERC1238URIHoldable", function () { let erc1238Holdable: ERC1238HoldableMock; let admin: SignerWithAddress; let tokenOwnerContract: ERC1238ReceiverHoldableMock; - // let tokenOwnerContract2: ERC1238ReceiverHoldableMock; let eoa1: SignerWithAddress; let eoa2: SignerWithAddress; @@ -200,4 +200,55 @@ describe("ERC1238URIHoldable", function () { ).to.be.revertedWith("ERC1238Holdable: Amount to burn exceeds amount held"); }); }); + + describe("Burning notification", () => { + const amountToBurn = mintAmount - 1000; + let tokenOwnerContract2: ERC1238ReceiverHoldableMock; + + beforeEach(async () => { + await erc1238Holdable.mintToContract(tokenOwnerContract.address, tokenId, mintAmount, data); + + const ERC1238ReceiverHoldableMockArtifact: Artifact = await artifacts.readArtifact("ERC1238ReceiverHoldableMock"); + + tokenOwnerContract2 = ( + await waffle.deployContract(admin, ERC1238ReceiverHoldableMockArtifact) + ); + }); + + it("should emit an event if burning acknowledgment failed", async () => { + await tokenOwnerContract.entrust(erc1238Holdable.address, tokenOwnerContract2.address, tokenId, amountToBurn); + + const burnTx = await erc1238Holdable.burnHeldTokens( + tokenOwnerContract2.address, + tokenOwnerContract.address, + tokenId, + amountToBurn, + ); + + const receipt = await burnTx.wait(); + + // Checks that a BurnAcknowledgmentFailed event was emitted + expect(receipt.events?.findIndex(eventObject => eventObject.event === "BurnAcknowledgmentFailed")).to.not.eq(-1); + }); + + it("should not emit an event if a burn was acknowledged", async () => { + const ERC1238HolderMockArtifact: Artifact = await artifacts.readArtifact("ERC1238HolderMock"); + + const holderContract = await waffle.deployContract(admin, ERC1238HolderMockArtifact); + + await tokenOwnerContract.entrust(erc1238Holdable.address, holderContract.address, tokenId, amountToBurn); + + const burnTx = await erc1238Holdable.burnHeldTokens( + holderContract.address, + tokenOwnerContract.address, + tokenId, + amountToBurn, + ); + + const receipt = await burnTx.wait(); + + // Checks that a BurnAcknowledgmentFailed event was NOT emitted + expect(receipt.events?.findIndex(eventObject => eventObject.event === "BurnAcknowledgmentFailed")).to.eq(-1); + }); + }); }); From 8f7b1db6398fd35488bdf44c3db7b239f0920371 Mon Sep 17 00:00:00 2001 From: ra-phael <10075759+ra-phael@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:09:52 +0100 Subject: [PATCH 18/18] add comments --- .../ERC1238/extensions/ERC1238Holdable.sol | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/contracts/ERC1238/extensions/ERC1238Holdable.sol b/contracts/ERC1238/extensions/ERC1238Holdable.sol index 45f73dc..31b2819 100644 --- a/contracts/ERC1238/extensions/ERC1238Holdable.sol +++ b/contracts/ERC1238/extensions/ERC1238Holdable.sol @@ -20,6 +20,10 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { return _heldBalances[holder][id]; } + /** + * @dev Hooks into the minting flow to set the token recipient as first holder + * by default when tokens are minted. + */ function _beforeMint( address, address to, @@ -27,10 +31,17 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { uint256 amount, bytes memory ) internal virtual override { - // set the token recipient as first holder by default when tokens are minted _heldBalances[to][id] += amount; } + /** + * @dev Burns `amount` of tokens that are held by `holder` and owned by `from`. + * If `holder` is a smart contract and inherits {IERC1238Holder}, it notifies it to give it a chance to + * react to the burn and handle the operation how it sees fit. + * + * Requirements: + * - `holder` should hold at least the `amount` of tokens with the `id` passed + */ function _burnHeldTokens( address burner, address holder, @@ -56,7 +67,6 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { /** * @dev Lets sender entrusts `to` with `amount` * of tokens which gets transferred between their respective heldBalances - * */ function _entrust( address to, @@ -74,6 +84,6 @@ abstract contract ERC1238Holdable is IERC1238Holdable, ERC1238 { emit Entrust(from, to, id, amount); } - // TODO: entrustHolderContract to provide safer alternative which - // makes sure the recipient is a IERC1238Holder contract + // TODO: Add a function to provide a safer alternative which + // makes sure the recipient is a IERC1238Holder contract (same as idea as in IERC1238Receiver) }