search

stellar / django-polaris

An extendable Django app for building modular Stellar services
https://django-polaris.readthedocs.io
Apache License 2.0
94 stars 66 forks source link

SEP-6: `Asset.distribution_account` is always used as anchor's receiving address #645

Closed nikita-gorodeckij closed 1 year ago

nikita-gorodeckij commented 1 year ago

I want to use muxed accounts as anchor receiving account instead account+memo

Validation here doesn't allow me to do it https://github.com/stellar/django-polaris/blob/2420d6d97e32ee720957c5020f250618740d4371/polaris/sep6/withdraw.py#L114

Are there any reasons to not allow to use MuxedAccount here? It looks much comfortble for users than memo - many of them forget to attach it to the transaction

nikita-gorodeckij commented 1 year ago

Also there is no any sense in this line https://github.com/stellar/django-polaris/blob/v2.3.1/polaris/sep6/withdraw.py#L115

because receiving_account not overwrites account_id which sets here https://github.com/stellar/django-polaris/blob/v2.3.1/polaris/sep6/withdraw.py#L294

JakeUrban commented 1 year ago

Hi @nikita-gorodeckij,

The second line you point out is a bug, thanks for pointing that out, but the first is not.

The standard doesn't support using muxed accounts as receiving addresses for withdrawals. I think we'd need to discuss this at the protocol level before I change the code to support this.

SEP-6 withdraws require that the anchor specify a memo so that Polaris can match the payment made from the client to the anchor. These memos are per-transaction memos, they need to be unique per transaction so they disambiguate on-chain transactions.

Theoretically, you could just encode the per-transaction memo in the receiving address, creating a muxed account. This is something we should discuss on stellar-protocol. Note that wallets wouldn't have this functionality when you launch though, so it may be worth just using memos.

JakeUrban commented 1 year ago

I'll update the code to ensure the correct receiving address is used. This is the one that comes from get_receiving_account_and_memo(). It defaults to asset.distribution_account anyway.

JakeUrban commented 1 year ago

I'm going to change the title of the issue to highlight the bug that needs fixing here. But I'll open an issue on the protocol repo to discuss your use of muxed accounts.