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: configurable referral values #767

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

michael1011
Copy link
Member

@michael1011 michael1011 commented Jan 3, 2025

Closes #754
Closes #768

};

// TODO: direction of chain swaps
type Premiums = Partial<Record<SwapType, number>>;
Copy link
Member

@jackstar12 jackstar12 Jan 3, 2025

Choose a reason for hiding this comment

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

Think we should go for a seperate Premium type to accomodate chain swaps and to be more flexible in the future.

type Premium = {
    swapType: SwapType
    pair: Pair
    serviceFee: number
}

And use a Premium[] in the config instead of a Record

Copy link
Member Author

Choose a reason for hiding this comment

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

That would just turn the Records into lists. What is the advantage of that?

Copy link
Member

@jackstar12 jackstar12 Jan 3, 2025

Choose a reason for hiding this comment

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

No. With the records, we can only have one per swap type.
With this, we could have 2 entries for chain swaps since we also have the pair information, so you could change BTC->LBTC without affecting LBTC-BTC - same for reverse / submarine swaps.

Copy link
Member Author

@michael1011 michael1011 Jan 3, 2025

Choose a reason for hiding this comment

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

  // Pair configs beat the ones of the type
  pairs?: Record<string, ReferralPairConfig>;

There is a nested mapping for pair -> type -> values already.

so you could change BTC->LBTC without affecting LBTC-BTC

That is not the case either way. Those two directions are the same pair internally

@kilrau
Copy link
Member

kilrau commented Jan 3, 2025

Does this address all points of #754 including limiting concurrent swaps yet?

@maybeast
Copy link
Member

maybeast commented Jan 3, 2025

Does this address all points of #754 including limiting concurrent swaps yet?

Everything excluding concurrent limits.

@maybeast
Copy link
Member

maybeast commented Jan 3, 2025

concept ack

@michael1011 michael1011 marked this pull request as ready for review January 5, 2025 18:58
} else {
req.setConfig(argv.config);
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

why the double else?

@maybeast
Copy link
Member

maybeast commented Jan 5, 2025

utACK

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.

Pro: gRPC Pro: more customization for referrals
4 participants