Skip to content

Custom fee detector - #1284

Closed
littletarzan wants to merge 10 commits into
hiero-ledger:mainfrom
littletarzan:custom-fee-detector
Closed

Custom fee detector#1284
littletarzan wants to merge 10 commits into
hiero-ledger:mainfrom
littletarzan:custom-fee-detector

Conversation

@littletarzan

Copy link
Copy Markdown
Contributor

Description:

Related issue(s):

Fixes #

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@littletarzan
littletarzan requested a review from a team as a code owner September 15, 2025 15:41
Signed-off-by: Michael Garber <michael.garber@hashgraph.com>
@Neurone

Neurone commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

The system contract seems to provide info about the token keys already (see getTokenKey helper).

Is that enough for the use case you mention?

Comment thread hip-1284.md
Comment on lines +50 to +51
bool hasCustomFeeSchedule,
bool hasCustomFeeScheduleKey

@mgh14 mgh14 Sep 17, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using booleans here, it seems like a contract might eagerly reject a token with a reasonable fee schedule. Better safe than sorry, but this might cause other issues for legitimate tokens.

That said, how much information is it appropriate to expose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mgh14 A major concern I have is that a legitimate token with a reasonable custom fee schedule can be changed by the fee schedule key to become arbitrarily unreasonable, hence why I am looking for a boolean hasCustomFeeScheduleKey.

IHederaTokenService.sol does return the custom fee schedule, but the boolean would be helpful if the goal is to reject all custom fee tokens (https://github.com/hashgraph/hedera-smart-contracts/blob/2e7c67fe3e0306e660a767028dc2f61379033580/contracts/system-contracts/hedera-token-service/IHederaTokenService.sol#L651)

@littletarzan

Copy link
Copy Markdown
Contributor Author

The system contract seems to provide info about the token keys already (see getTokenKey helper).

Is that enough for the use case you mention?

@Neurone Unfortunately, here in the services codebase, contracts only "see" simple key types inside the EVM. This means if key is a threshold key or list, it would appear empty. I definitely should have included this in the PR and will update accordingly.

@Neurone

Neurone commented Sep 22, 2025

Copy link
Copy Markdown
Contributor

The system contract seems to provide info about the token keys already (see getTokenKey helper).
Is that enough for the use case you mention?

@Neurone Unfortunately, here in the services codebase, contracts only "see" simple key types inside the EVM. This means if key is a threshold key or list, it would appear empty. I definitely should have included this in the PR and will update accordingly.

I see, right.

What if we propose extending getTokenKey to support complex keys? I assume there's no need to go into detail. Smart contracts can hardly manage those anyway. We can think about adding COMPLEX to the KeyValueType enum. The actual raw value can be 1 (true).

Also, I think you can get all the info about the current custom fees via the getTokenInfo method.
Does the hasCustomFeeSchedule method add anything in this regard?

@littletarzan

Copy link
Copy Markdown
Contributor Author

The system contract seems to provide info about the token keys already (see getTokenKey helper).
Is that enough for the use case you mention?

@Neurone Unfortunately, here in the services codebase, contracts only "see" simple key types inside the EVM. This means if key is a threshold key or list, it would appear empty. I definitely should have included this in the PR and will update accordingly.

I see, right.

What if we propose extending getTokenKey to support complex keys? I assume there's no need to go into detail. Smart contracts can hardly manage those anyway. We can think about adding COMPLEX to the KeyValueType enum. The actual raw value can be 1 (true).

Also, I think you can get all the info about the current custom fees via the getTokenInfo method. Does the hasCustomFeeSchedule method add anything in this regard?

@Neurone Yes, I think extending getTokenKey would be sufficient, that way other keys like wipe/freeze/etc can be understood better in the EVM. For my use case, there is no need to go into detail wrt keys. If hasCustomFeeSchedule can be replaced by asking if FixedFee[] memory fixedFees, FractionalFee[] memory fractionalFees, RoyaltyFee[] memory royaltyFees all have length = 0, then I don't think it adds anything special.

@mgarbs

mgarbs commented Sep 29, 2025

Copy link
Copy Markdown
Contributor

@littletarzan may you DCO and GPG verify your commits? These checks are failing

@littletarzan

Copy link
Copy Markdown
Contributor Author

@littletarzan may you DCO and GPG verify your commits? These checks are failing

It looks like we are going in a different direction than I anticipated so Ii will just close the PR and try again with this new idea of exposing visibility into comple key types

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.

4 participants