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

feat/manage protocol fee list #464

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

r3v4s
Copy link
Member

@r3v4s r3v4s commented Jan 17, 2025

Description

  1. protocol_fee manages accumulated token path with its amount

to avoid iterating (entire) registered token list

  1. each contract which send certain amount to protocol_fee contract has to set which token and how much has been sent

@r3v4s r3v4s added enhancement New feature or request protocol_fee labels Jan 17, 2025
@r3v4s r3v4s requested review from notJoon, onlyhyde and mconcat January 17, 2025 06:27
@r3v4s r3v4s self-assigned this Jan 17, 2025
balance := common.BalanceOf(token, consts.PROTOCOL_FEE_ADDR)

// amount in `tokenListWithAmount` is corrupted, it should be equal to balance
if amount != balance {
Copy link
Member

Choose a reason for hiding this comment

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

Anyone can transfer assets to a PROTOCOL_FEE contract. The guard on that line is panicking if the assets on the books don't match the balance in the actual token contract. If someone maliciously sends assets to the PROTOCOL_FEE Contract, PROTOCOL_FEE will never receive them back. We recommend removing the guard on that line as it can cause problems.

Copy link
Member Author

Choose a reason for hiding this comment

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

panic(errInvalidAmount.Error())
}

if balance > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the condition should enter when the amount, not balance, is greater than or equal to 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

@r3v4s r3v4s requested a review from onlyhyde January 19, 2025 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request protocol_fee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants