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

Add RoyaltyFee custom fee type #71

Merged
merged 4 commits into from
Aug 12, 2021
Merged

Conversation

tinker-michaelj
Copy link
Contributor

@tinker-michaelj tinker-michaelj commented Aug 8, 2021

Description:

tinker-michaelj added 2 commits August 8, 2021 11:17
Signed-off-by: tinker-michaelj <[email protected]>
Signed-off-by: tinker-michaelj <[email protected]>
tinker-michaelj added 2 commits August 8, 2021 11:23
Signed-off-by: tinker-michaelj <[email protected]>
Signed-off-by: tinker-michaelj <[email protected]>
@xin-hedera
Copy link
Contributor

it's possible when the fungible value in exchange for the nft is low and multiplying it with the fraction will result in 0 royalty fee charged, is it desired or should we add a minimum?

@@ -251,7 +251,7 @@ enum ResponseCodeEnum {
CUSTOM_FEE_MUST_BE_POSITIVE = 239; // Only positive fees may be assessed at this time
TOKEN_HAS_NO_FEE_SCHEDULE_KEY = 240; // Fee schedule key is not set on token
CUSTOM_FEE_OUTSIDE_NUMERIC_RANGE = 241; // A fractional custom fee exceeded the range of a 64-bit signed integer
INVALID_CUSTOM_FRACTIONAL_FEES_SUM = 242; // The sum of all custom fractional fees must be strictly less than 1

Choose a reason for hiding this comment

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

If we still keep the FractionalFee, I think we should also keep this INVALID_CUSTOM_FRACTIONAL_FEES_SUM response code.

@tinker-michaelj
Copy link
Contributor Author

tinker-michaelj commented Aug 11, 2021

it's possible when the fungible value in exchange for the nft is low and multiplying it with the fraction will result in 0 royalty fee charged, is it desired or should we add a minimum?

Interesting question. 🤔 It seems we would need a separate minimum for each of the fungible tokens the fee collector accepts, which could make the feature somewhat elaborate.

@tinker-michaelj tinker-michaelj merged commit 3437c5c into develop Aug 12, 2021
@tinker-michaelj tinker-michaelj deleted the 01935-D-AddRoyaltyFees branch August 12, 2021 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants