-
Notifications
You must be signed in to change notification settings - Fork 217
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
chore: fusdc optional maxVariable
in FeeConfig
#10859
base: master
Are you sure you want to change the base?
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
packages/fast-usdc/src/types.ts
Outdated
maxVariable: Amount<'nat'>; | ||
/** a cap on variable fees. no cap if undefined */ | ||
maxVariable?: Amount<'nat'>; | ||
/** percent of fees that go to contract (remaining goes to LPs) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proportion of fees ("percent" is a particular notation)
packages/fast-usdc/src/types.ts
Outdated
flat: Amount<'nat'>; | ||
/** percentage charged for every advance */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for these explanations. please be a bit more explicit.
/** percentage charged for every advance */ | |
/** proportion of advance kept as a fee */ |
packages/fast-usdc/src/types.ts
Outdated
variableRate: Ratio; | ||
maxVariable: Amount<'nat'>; | ||
/** a cap on variable fees. no cap if undefined */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read this at first as a limit on how the variableRate
could move. Now I understand it as a max on the variable component of the total fee.
Why have a max on that component instead of the fee overall? They're functionally equivalent offset by flat
, but having a maxFee
is much clearer to the user.
10aa465
to
fb901e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change. Remember to update the PR title before merging
closes: #10858
Description
If
maxVariable
isundefined
inFeeConfig
, do not cap fees.Security Considerations
None
Scaling Considerations
None
Documentation Considerations
None
Testing Considerations
Adds new unit test.
Upgrade Considerations
None, part of initial release for FUSDC.