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.

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

chore: fusdc optional maxVariable in FeeConfig #10859

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −10
Open

chore: fusdc optional maxVariable in FeeConfig #10859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 1 addition & 4 deletions packages/boot/test/fast-usdc/snapshots/fast-usdc.test.ts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,7 @@ Generated by [AVA](https://avajs.dev).
brand: Object @Alleged: USDC brand {},
value: 10000n,
},
maxVariable: {
brand: Object @Alleged: USDC brand {},
value: 5000000n,
},
maxVariable: undefined,
variableRate: {
denominator: {
brand: Object @Alleged: USDC brand {},
Expand Down
Binary file modified packages/boot/test/fast-usdc/snapshots/fast-usdc.test.ts.snap
View file
Open in desktop
Binary file not shown.
4 changes: 2 additions & 2 deletions packages/builders/scripts/fast-usdc/start-fast-usdc.build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const { keys } = Object;
const options = {
flatFee: { type: 'string', default: '0.01' },
variableRate: { type: 'string', default: '0.01' },
maxVariableFee: { type: 'string', default: '5' },
maxVariableFee: { type: 'string' },
contractRate: { type: 'string', default: '0.2' },
net: { type: 'string' },
oracle: { type: 'string', multiple: true },
Expand Down Expand Up @@ -165,7 +165,7 @@ export default async (homeP, endowments) => {
return {
flat: toAmount(flatFee),
variableRate: toRatio(variableRate),
maxVariable: toAmount(maxVariableFee),
maxVariable: maxVariableFee ? toAmount(maxVariableFee) : undefined,
contractRate: toRatio(contractRate),
};
};
Expand Down
2 changes: 1 addition & 1 deletion packages/fast-usdc/src/type-guards.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const NatAmountShape = { brand: BrandShape, value: M.nat() };
export const FeeConfigShape = {
flat: NatAmountShape,
variableRate: RatioShape,
maxVariable: NatAmountShape,
maxVariable: M.opt(NatAmountShape),
contractRate: RatioShape,
};
harden(FeeConfigShape);
Expand Down
6 changes: 5 additions & 1 deletion packages/fast-usdc/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,13 @@ export interface PendingTx extends CctpTxEvidence {
}

export type FeeConfig = {
/** flat fee charged for every advance */
flat: Amount<'nat'>;
/** percentage charged for every advance */
Copy link
Member

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.

Suggested change
/** percentage charged for every advance */
/** proportion of advance kept as a fee */

variableRate: Ratio;
maxVariable: Amount<'nat'>;
/** a cap on variable fees. no cap if undefined */
Copy link
Member

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.

maxVariable?: Amount<'nat'>;
/** percent of fees that go to contract (remaining goes to LPs) */
Copy link
Member

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)

contractRate: Ratio;
};

Expand Down
5 changes: 4 additions & 1 deletion packages/fast-usdc/src/utils/fees.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ export const makeFeeTools = feeConfig => {
* @throws {Error} if requested does not exceed fees
*/
calculateAdvanceFee(requested) {
const variable = min(multiplyBy(requested, variableRate), maxVariable);
const baseVariable = multiplyBy(requested, variableRate);
const variable = maxVariable
? min(baseVariable, maxVariable)
: baseVariable;
const fee = add(variable, flat);
!isGTE(fee, requested) || Fail`Request must exceed fees.`;
return fee;
Expand Down
19 changes: 18 additions & 1 deletion packages/fast-usdc/test/utils/fees.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ test(feeToolsScenario, {
});

test(feeToolsScenario, {
// TODO consider behavior where 0 or undefined means "no fee cap"
name: 'only flat is charged if `maxVariable: 0`',
requested: USDC(10000n),
config: {
Expand All @@ -166,6 +165,24 @@ test(feeToolsScenario, {
},
});

test(feeToolsScenario, {
name: 'no bound on variable fee if `maxVariable: undefined`',
requested: USDC(10000n),
config: {
...aFeeConfig,
variableRate: USDCRatio(20n),
maxVariable: undefined,
},
expected: {
totalFee: USDC(2010n),
advance: USDC(7990n),
split: {
ContractFee: USDC(402n),
PoolFee: USDC(1608n),
},
},
});

test(feeToolsScenario, {
name: 'AGORIC_PLUS_OSMO with commonPrivateArgs.feeConfig',
// 150_000_000n
Expand Down
Loading