Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Force royalties on ERC721 #55

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cookiecutter.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
"max_supply": "y",
"max_supply_amount": 10000,
"permitable": "y",
"royalty_percentage_in_decimals": 10.0,
"royalties": "y",
"royalty_percentage_in_decimals": 10.0
"force_royalties": "y",
"minRoyaltyAmount": 1000000000000000,
"thresholdPercentage": 1.0
}
5 changes: 4 additions & 1 deletion tests/Basic.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
"max_supply_amount": 3,
"permitable": "y",
"royalties": "y",
"royalty_percentage_in_decimals": 10.0
"royalty_percentage_in_decimals": 10.0,
"force_royalties": "y",
"minRoyaltyAmount": 1000000000000000,
"thresholdPercentage": 5.0
}
}
7 changes: 5 additions & 2 deletions tests/Mintable.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
"updatable_uri": "n",
"base_uri": "ipfs://QmaZm1rAkt6kHTKTFX8GwEhtPMVMeAGJYMBvoAcJWTddwb",
"metadata": "n",
"mintable": "y",
"mintable": "n",
"max_supply": "y",
"max_supply_amount": 3,
"permitable": "y",
"royalties": "y",
"royalty_percentage_in_decimals": 10.0
"royalty_percentage_in_decimals": 10.0,
"force_royalties": "n",
"minRoyaltyAmount": 0,
"thresholdPercentage": 0.0

}
}
3 changes: 3 additions & 0 deletions {{cookiecutter.project_name}}/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,15 @@ pip-log.txt
pip-delete-this-directory.txt

# Unit test / coverage reports
__local__.json
htmlcov/
.tox/
.nox/
charmap.json.gz
.coverage
.coverage.*
.cache
.DS_Store
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually files like this are best in your global gitignore
https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files

nosetests.xml
coverage.xml
*.cover
Expand Down
94 changes: 94 additions & 0 deletions {{cookiecutter.project_name}}/contracts/NFT.vy
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ event ApprovalForAll:
operator: indexed(address)
approved: bool

event PaymentReceived:
sender: indexed(address)
amount: uint256

event Withdrawal:
to: indexed(address)
amount: uint256


owner: public(address)
isMinter: public(HashMap[address, bool])

Expand All @@ -109,6 +118,22 @@ isApprovedForAll: public(HashMap[address, HashMap[address, bool]])
# @dev Mapping from NFT ID to approved address.
idToApprovals: public(HashMap[uint256, address])

{%- if cookiecutter.force_royalties == 'y' %}
# @dev The minimum amount of royalties that should be paid to transfer a token successfully
# NOTE: this can be used to track the floor price for enforcing a minimum royalty payment to the creator
# NOTE: current implementation requires paying directly to the smart contract, which forwards to the creator
minRoyaltyAmount: public(uint256)

# Percentage threshold to decide whether the minRoyaltyAmount should be increased or decreased.
thresholdPercentage: public(decimal)

lastBalance: public(uint256)

totalReceived: public(uint256)

payments: HashMap[address, uint256]
{%- endif %}

{%- if cookiecutter.permitable == 'y' %}
############ ERC-4494 ############

Expand Down Expand Up @@ -165,6 +190,13 @@ def __init__():
self.baseURI = "{{cookiecutter.base_uri}}"
{%- endif %}

# @dev the default value for the minRoyaltyAmount is 0.01 ETH = 10**16
{%- if cookiecutter.force_royalties == 'y' %}
self.minRoyaltyAmount = {{ cookiecutter.minRoyaltyAmount }}
self.thresholdPercentage = {{ cookiecutter.thresholdPercentage }}
self.lastBalance = self.balance
{%- endif %}


{%- if cookiecutter.permitable == 'y' %}
# ERC712 domain separator for ERC4494
Expand All @@ -179,6 +211,32 @@ def __init__():
)
{%- endif %}


{%- if cookiecutter.force_royalties == 'y' %}
@external
@payable
def __default__():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, when smart contracts built by compilers like solidity or vyper send ether to a contract, it only gives a limited amount of gas to that contract to execute. So, basically I think this won't work since there's too much logic going on inside it

# Check if the incoming payment is less than the minimum royalty amount
if msg.value < self.minRoyaltyAmount:
raise "Royalties not paid correctly."

# Calculate the expected minimum based on the current minimum royalty amount and the threshold
expectedMin: uint256 = self.minRoyaltyAmount * convert(1.0 + self.thresholdPercentage, uint256)

# Convert balance to uint256 for comparison
balanceInWei: uint256 = self.balance

# Check if balance has increased by more than the expected minimum
if balanceInWei > self.lastBalance + expectedMin:
# Increase minRoyaltyAmount by a certain percentage (for example, by 5%)
self.minRoyaltyAmount = convert(convert(self.minRoyaltyAmount, decimal) * 1.05, uint256)
elif balanceInWei < self.lastBalance + expectedMin:
# Decrease minRoyaltyAmount by a certain percentage (for example, by 5%)
self.minRoyaltyAmount = convert(convert(self.minRoyaltyAmount, decimal) * 0.95, uint256)

self.lastBalance = balanceInWei
{%- endif %}

{%- if cookiecutter.metadata == 'y' %}
# ERC721 Metadata Extension
@pure
Expand Down Expand Up @@ -298,7 +356,21 @@ def royaltyInfo(_tokenId: uint256, _salePrice: uint256) -> (address, uint256):
"""

royalty: uint256 = convert(convert(_salePrice, decimal) * ROYALTY_TO_APPLY_TO_PRICE, uint256) # Percentage that accepts decimals
{%- if cookiecutter.force_royalties == 'y' %}
return self, max(self.minRoyaltyAmount, royalty)
{%- else %}
return self.owner, royalty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually needs to be update to integrate well

Suggested change
return self.owner, royalty
return self, max(self.minRoyaltyAmount, royalty)

{%- endif %}
{%- endif %}

{%- if cookiecutter.force_royalties == 'y' %}
@external
def withdraw():
assert msg.sender == self.owner
amount: uint256 = self.balance
send(self.owner, amount)
log Withdrawal(self.owner, amount)
self.lastBalance = self.balance # This should be 0 after the withdrawal
{%- endif %}

@view
Expand All @@ -325,6 +397,22 @@ def _isApprovedOrOwner(spender: address, tokenId: uint256) -> bool:
return False


# Royality Functions
{%- if cookiecutter.force_royalties == 'y' %}
@internal
def _enforceRoyalties():
# Calculate the payment amount from the most recent transaction
payment: uint256 = self.balance - self.lastBalance

# Ensure the payment is not less than the minimum royalty amount
if payment < self.minRoyaltyAmount:
raise "Insufficient payment."

# Update the last balance
self.lastBalance = self.balance
{%- endif %}


@internal
def _transferFrom(owner: address, receiver: address, tokenId: uint256, sender: address):
"""
Expand Down Expand Up @@ -378,6 +466,9 @@ def transferFrom(owner: address, receiver: address, tokenId: uint256):
@param receiver The new owner.
@param tokenId The NFT to transfer.
"""
{%- if cookiecutter.force_royalties == 'y' %}
self._enforceRoyalties()
{%- endif %}
self._transferFrom(owner, receiver, tokenId, msg.sender)


Expand All @@ -403,6 +494,9 @@ def safeTransferFrom(
@param tokenId The NFT to transfer.
@param data Additional data with no specified format, sent in call to `receiver`.
"""
{%- if cookiecutter.force_royalties == 'y' %}
self._enforceRoyalties()
{%- endif %}
self._transferFrom(owner, receiver, tokenId, msg.sender)
if receiver.is_contract: # check if `receiver` is a contract address
returnValue: bytes4 = ERC721Receiver(receiver).onERC721Received(msg.sender, owner, tokenId, data)
Expand Down
10 changes: 5 additions & 5 deletions {{cookiecutter.project_name}}/tests/test_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ def test_transfer(nft, owner, receiver):
nft.mint(owner, sender=owner)
assert nft.balanceOf(owner) == 1
assert nft.ownerOf(0) == owner.address
nft.transferFrom(owner, receiver, 0, sender=owner)
nft.transferFrom(owner, receiver, 0, value=nft.minRoyaltyAmount(), sender=owner)
assert nft.ownerOf(0) == receiver.address
assert nft.balanceOf(owner) == 0
assert nft.balanceOf(receiver) == 1
nft.transferFrom(receiver, owner, 0, sender=receiver)
nft.transferFrom(receiver, owner, 0, value=nft.minRoyaltyAmount(), sender=receiver)
assert nft.balanceOf(receiver) == 0
assert nft.balanceOf(owner) == 1
assert nft.ownerOf(0) == owner.address
Expand All @@ -74,7 +74,7 @@ def test_incorrect_signer_transfer(nft, owner, receiver):
assert nft.balanceOf(receiver) == 0
nft.mint(owner, sender=owner)
with ape.reverts():
nft.transferFrom(owner,receiver,0,sender=receiver)
nft.transferFrom(owner, receiver, 0, value=nft.minRoyaltyAmount(), sender=receiver)
assert nft.balanceOf(receiver) == 0
assert nft.balanceOf(owner) == 1
assert nft.ownerOf(0) == owner.address
Expand All @@ -100,14 +100,14 @@ def test_approve_transfer(nft, owner, receiver):

with ape.reverts():
nft.approve(receiver, 0, sender=receiver)
nft.transferFrom(owner, receiver, 0, sender=receiver)
nft.transferFrom(owner, receiver, 0, value=nft.minRoyaltyAmount(), sender=receiver)
assert nft.balanceOf(receiver) == 0
assert nft.balanceOf(owner) == 1
assert nft.ownerOf(0) == owner.address

nft.approve(receiver, 0, sender=owner)
assert nft.getApproved(0) == receiver
nft.transferFrom(owner, receiver, 0, sender=receiver)
nft.transferFrom(owner, receiver, 0, value=nft.minRoyaltyAmount(), sender=receiver)
assert nft.balanceOf(receiver) == 1
assert nft.balanceOf(owner) == 0
assert nft.ownerOf(0) == receiver.address
Expand Down