Skip to content

Refactor royalty #440

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Refactor royalty #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd1c456
refactored royalty code
adhocmaster May 12, 2022
b17ca4f
removed tokenId from calculateRoyalty
adhocmaster May 12, 2022
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
2 changes: 2 additions & 0 deletions packages/contracts/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ node_modules
#Hardhat files
cache
artifacts
.openzeppelin/*
workspace.code-workspace
18 changes: 14 additions & 4 deletions packages/contracts/contracts/identity/NonFungibleRegistryEnumerableUpgradeable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ contract NonFungibleRegistryEnumerableUpgradeable is
address public burnAddress;

/// @notice The amount in basis points of registrationFee to send to the burnAddress account
/// @notice This variable must be less than or equal to 10000
/// @notice This variable must be less than or equal to 10000 / burnDenom
uint256 public burnFee;


/** @dev address of primary NFR registry required for participation
* if the primaryRegistry is address(0), then this variable is ignored
* if the primaryRegistry is an ERC721, then the recipient of an NFI must
Expand Down Expand Up @@ -125,6 +126,10 @@ contract NonFungibleRegistryEnumerableUpgradeable is
/// @dev The REGISTRAR_ROLE_ADMIN curates the address with REGISTRAR_ROLE permissions
bytes32 public constant REGISTRAR_ROLE_ADMIN = keccak256("REGISTRAR_ROLE_ADMIN");


/// @notice used to calculate burn % in calculating royaltyAmount
uint constant burnDenom = 10000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to uint256


/**
* @dev Emitted when updateLabel is called successfully
*/
Expand All @@ -143,7 +148,7 @@ contract NonFungibleRegistryEnumerableUpgradeable is
/// @custom:oz-upgrades-unsafe-allow constructor
constructor() initializer {}

/// @notice initialize is called once on the creation of an upgradable proxy
/// @notice initialize is called once on the creation of an upgradable proxy.
/// @dev can only be called once due to the initializer modifier
/// @param name_ name to be given to the Non Fungible Registry
/// @param symbol_ shorthand symbol to be given to the Non Fungible Registry
Expand Down Expand Up @@ -273,7 +278,8 @@ contract NonFungibleRegistryEnumerableUpgradeable is

_createLabeledToken(to, label, registrationData, tokenId);

uint256 burnAmount = registrationFee * burnFee / 10000;
uint256 burnAmount = calculateRoyalty(registrationFee);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the intended functionality; this function costs "registration token" (ie Hypertoken) to use, and isn't/shouldn't be related to the royalties at all.


IERC20Upgradeable(registrationToken).transfer(burnAddress, burnAmount);
// the fee stays with the token, not the token owner
identityStakes[tokenId] = Fee(registrationToken, registrationFee-burnAmount);
Expand Down Expand Up @@ -443,12 +449,16 @@ contract NonFungibleRegistryEnumerableUpgradeable is
}
}

function calculateRoyalty(uint256 salePrice) internal view returns(uint256 amount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can change this to pure

amount = salePrice * burnFee / burnDenom;
}

function royaltyInfo(uint256 tokenId, uint256 salePrice)
external
view
override(IERC2981Upgradeable)
returns (address receiver, uint256 royaltyAmount) {
royaltyAmount = salePrice * burnFee / 10000;
royaltyAmount = calculateRoyalty(salePrice);
receiver = burnAddress;
}

Expand Down
13 changes: 10 additions & 3 deletions packages/contracts/contracts/identity/NonFungibleRegistryUpgradeable.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ contract NonFungibleRegistryUpgradeable is
address public burnAddress;

/// @notice The amount in basis points of registrationFee to send to the burnAddress account
/// @notice This variable must be less than or equal to 10000
/// @notice This variable must be less than or equal to 10000 / burnDenom
uint256 public burnFee;

/** @dev address of primary NFR registry required for participation
Expand Down Expand Up @@ -123,6 +123,8 @@ contract NonFungibleRegistryUpgradeable is
/// @dev The REGISTRAR_ROLE_ADMIN curates the address with REGISTRAR_ROLE permissions
bytes32 public constant REGISTRAR_ROLE_ADMIN = keccak256("REGISTRAR_ROLE_ADMIN");

/// @notice used to calculate burn % in calculating royaltyAmount
uint constant burnDenom = 10000;
/**
* @dev Emitted when updateLabel is called successfully
*/
Expand Down Expand Up @@ -270,7 +272,7 @@ contract NonFungibleRegistryUpgradeable is

_createLabeledToken(to, label, registrationData, tokenId);

uint256 burnAmount = registrationFee * burnFee / 10000;
uint256 burnAmount = calculateRoyalty(registrationFee);
IERC20Upgradeable(registrationToken).transfer(burnAddress, burnAmount);
// the fee stays with the token, not the token owner
identityStakes[tokenId] = Fee(registrationToken, registrationFee-burnAmount);
Expand Down Expand Up @@ -440,12 +442,17 @@ contract NonFungibleRegistryUpgradeable is
}
}

function calculateRoyalty(uint256 salePrice) internal view returns(uint256 amount) {
amount = salePrice * burnFee / burnDenom;
}


function royaltyInfo(uint256 tokenId, uint256 salePrice)
external
view
override(IERC2981Upgradeable)
returns (address receiver, uint256 royaltyAmount) {
royaltyAmount = salePrice * burnFee / 10000;
royaltyAmount = calculateRoyalty(salePrice);
receiver = burnAddress;
}

Expand Down
8 changes: 8 additions & 0 deletions packages/contracts/hardhat.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ module.exports = {
chainId: 31337,
url: "http://127.0.0.1:8569",
},
dev_ganache: {
accounts: {
accountsBalance: "10000000000000000000000",
mnemonic,
},
chainId: 1337,
url: "http://127.0.0.1:7545"
},
DevNet: {
accounts: {
mnemonic,
Expand Down