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

Feature Request: flexible asset min and max amounts #322

Open
yuriescl opened this issue Oct 2, 2020 · 5 comments
Open

Feature Request: flexible asset min and max amounts #322

yuriescl opened this issue Oct 2, 2020 · 5 comments
Labels
blocked Cannot continue until something else happens discussion

Comments

@yuriescl
Copy link
Contributor

yuriescl commented Oct 2, 2020

Some Anchors might want to have different minimum and maximum deposit/withdrawal amounts for their assets, depending on the type of deposit/withdrawal.
For example, SEPA transfers might have a different deposit_min_amount than Cash deposits.

@yuriescl yuriescl changed the title Feature Request: custom asset min and max amounts Feature Request: flexible asset min and max amounts Oct 2, 2020
@yuriescl
Copy link
Contributor Author

yuriescl commented Oct 4, 2020

Asset.significant_decimals (and possibly more fields) would also require some flexibility. Depending on the transfer type, the Anchor might wanna do rounding differently.
Now I'm not sure if it's a good idea to change Polaris in order to allow those different configurations for assets.
Maybe it's better to leave Polaris as it is and let Anchors implement custom code for different transfer types.
Would Polaris be able to behave properly if the Anchor decides to use custom code for min/max amounts and also for significant decimals? Maybe there are core Polaris functions that would need refactoring to avoid conflicts related to this.

@JakeUrban
Copy link
Contributor

Polaris has an assumption that there is one minimum and maximum limit for an anchored asset no matter the rails used or fee's incurred by the anchor, and I agree this is an assumption we should remove. Without thinking too hard about it, it seems like it would be a decent amount of work and involve breaking changes, at least to the data model.

This means that full support for multiple min/max pairs per asset would likely be slated for 2.0 unless we could make the change strictly additive. And unfortunately, Polaris would likely break or conflict in some way if the anchor tried forcing a different min/max pair.

Depending on the transfer type, the Anchor might wanna do rounding differently.

That seems like a red flag. The asset should always be counted in the same units and no precision should be lost.

@yuriescl
Copy link
Contributor Author

yuriescl commented Oct 7, 2020

That seems like a red flag. The asset should always be counted in the same units and no precision should be lost

You're right. The scenario I imagined doesn't really makes sense. Nevermind that.

@yuriescl
Copy link
Contributor Author

yuriescl commented Oct 9, 2020

Polaris has an assumption that there is one minimum and maximum limit for an anchored asset no matter the rails used or fee's incurred by the anchor, and I agree this is an assumption we should remove. Without thinking too hard about it, it seems like it would be a decent amount of work and involve breaking changes, at least to the data model.

The min/max amounts could be properties and call integration functions, passing both the asset and the transfer type (type parameter in deposit/withdraw calls) as parameters:

def asset_min_amount(asset, transfer_type)

This would allow the anchor to customize the amounts and not require much refactor in Polaris code.

But, Polaris is currently compliant with the SEPs, since both SEP-6 /info and SEP-24 /info return a single min/max amount for the asset no matter the type.
This discussion seems to target the SEPs, so it might be more appropriate to transfer the issue to stellar/stellar-protocol.

@JakeUrban
Copy link
Contributor

I agree, reference this discussion there and we'll adjust Polaris according to the results of that discussion.

@JakeUrban JakeUrban added the blocked Cannot continue until something else happens label Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Cannot continue until something else happens discussion
Projects
None yet
Development

No branches or pull requests

2 participants