Contract: Transfer Fee implementation for XRPL originated tokens that have them

[keyleu]

Description

• Implemented Transfer Fees for XRPL originated tokens that apply them.
• Modified truncation to be done after applying all fees (for all transfers) because XRPL doesn't support amounts like 1e17+1 so we must truncate in the last step.
• Update spec to latest changes of fee calculations.

Reviewers checklist:

• Try to write more meaningful comments with clear actions to be taken.
• Nit-picking should be unblocking. Focus on core issues.

Authors checklist

• Provide a concise and meaningful description
• Review the code yourself first, before making the PR.
• Annotate your PR in places that require explanation.
• Think and try to split the PR to smaller PR if it is big.

---

This change is 

[dzmitryhil]

Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

---

contract/src/contract.rs line 767 at r1 (raw file):

    let decimals;
    let amount_to_send;
    let truncated_portion;

Don't we call it remainder the same for all places?

---

contract/src/contract.rs line 769 at r1 (raw file):

            // We calculate the amount to send after applying the bridging fees for that token
            amount_to_send =
                amount_after_fees(amount_truncated, xrpl_token.bridging_fee, truncated_portion)?;

As I mentioned before we should burn only after the operation confirmation, since if the tx fails, full amount should be returned.

---

spec/spec.md line 143 at r1 (raw file):

The bridging fees are distributed across the relayer addresses after the execution of the sending, and locked until a relayer manually requests it. After such a request the
accumulated bridging fee will be distributed equally to the current relayer addresses.
The transfer fee (if applied) is burnt once we create the pending operation to send the tokens back to the user on XRPL.

Actually, we should burn it only if the transfer is successful. The same should be in the code.

[keyleu]

Reviewable status: 0 of 8 files reviewed, 3 unresolved discussions (waiting on @dzmitryhil, @miladz68, @wojtek-coreum, and @ysv)

---

contract/src/contract.rs line 767 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Don't we call it remainder the same for all places?

Fixed in all places.

---

contract/src/contract.rs line 769 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

As I mentioned before we should burn only after the operation confirmation, since if the tx fails, full amount should be returned.

Correct. I moved it there and added tests for it.

---

spec/spec.md line 143 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Actually, we should burn it only if the transfer is successful. The same should be in the code.

Correct. I moved it there and added tests for it. Updated spec too.

[ysv]

Reviewed 3 of 7 files at r1, 1 of 5 files at r2, all commit messages.
Reviewable status: 4 of 8 files reviewed, 8 unresolved discussions (waiting on @dzmitryhil, @keyleu, @miladz68, and @wojtek-coreum)

---

contract/src/contract.rs line 61 at r2 (raw file):

// If it is 1000000001 it means the fee will be 0.0000001%
// If it is 1500000000 it means the fee will be 50% and so on.
// We will use this value to calculate the fee to be applied to the amount being sent.

nit: I suggest to add link to this https://xrpl.org/transfer-fees.html#technical-details

---

contract/src/contract.rs line 62 at r2 (raw file):

// If it is 1500000000 it means the fee will be 50% and so on.
// We will use this value to calculate the fee to be applied to the amount being sent.
pub const XRPL_MIN_TRANSFER_RATE: Uint128 = Uint128::new(1000000000);

the name was confusing for me initially.
Maybe XRPL_ZERO_TRANSFER_RATE since this value defines 1 to 1 (no commission transfer rate)

---

contract/src/contract.rs line 149 at r2 (raw file):

        state: TokenState::Enabled,
        bridging_fee: XRP_DEFAULT_FEE,
        transfer_rate: None,

transfer rate is implemented only for XRPL tokens I guess
Could u remind me what we decided to do with Coreum tokens which have fees ?

---

contract/src/operation.rs line 117 at r2 (raw file):

            currency,
            amount,
            transfer_fee,

do I understand it correctly that transfer_rate & transfer_fee are 2 different things
transfer_rate is kinda percentage while transfer_fee is absolute value in denom ?

---

contract/src/tests.rs line 4586 at r2 (raw file):

                        issuer: bridge_xrpl_address.to_owned(),
                        currency: coreum_token.xrpl_currency.to_owned(),
                        amount: Uint128::new(650010000000000), // 650010000000000 will convert to 650010, which after charging fees and truncating will send 350000 to the receiver

does this test both bridging fee & transfer fee ?

if you can write number manipulations as they happen step by step would be nice:
smth like: 650010000000000 (convert to XRPL amount) -> 650010 (charge bridging fee) -> 620000 ...

[keyleu]

Reviewable status: 4 of 8 files reviewed, 8 unresolved discussions (waiting on @dzmitryhil, @miladz68, @wojtek-coreum, and @ysv)

---

contract/src/contract.rs line 61 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: I suggest to add link to this https://xrpl.org/transfer-fees.html#technical-details

Added it.

---

contract/src/contract.rs line 62 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

the name was confusing for me initially.
Maybe XRPL_ZERO_TRANSFER_RATE since this value defines 1 to 1 (no commission transfer rate)

OK, modified it.

---

contract/src/contract.rs line 149 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

transfer rate is implemented only for XRPL tokens I guess
Could u remind me what we decided to do with Coreum tokens which have fees ?

Yes. Transfer Rate is only applied for XRPL originated tokens.

For Coreum tokens everything will work out of the box because of two reasons:

1. When a user sends funds to the contract to be bridged, the fees (commission and burn) will be applied on top of that amount so the contract will already receive the amount deducted.

2. We recently disabled commission fees / burning fees when smart contracts are sending transactions so we don't need to worry about them because they don't affect our contract, and we can send full amounts without these fees messing with our values.

---

contract/src/operation.rs line 117 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

do I understand it correctly that transfer_rate & transfer_fee are 2 different things
transfer_rate is kinda percentage while transfer_fee is absolute value in denom ?

Correct. XRPL defines the % as transfer_rate using the notation specified here https://xrpl.org/transfer-fees.html#technical-details.

I calculate the transfer_fee using that rate and the formulas, so it's an absolute value.

---

contract/src/tests.rs line 4586 at r2 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

does this test both bridging fee & transfer fee ?

if you can write number manipulations as they happen step by step would be nice:
smth like: 650010000000000 (convert to XRPL amount) -> 650010 (charge bridging fee) -> 620000 ...

No, this one is only for bridging fees.

We have tests with network fees + bridging fees in network_fee_collection_and_claiming() tests and there everything is explained in detail including all number manipulations.

[dzmitryhil]

Reviewable status: 4 of 8 files reviewed, 5 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

[ysv]

Reviewed 1 of 5 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @keyleu, @miladz68, and @wojtek-coreum)

---

contract/src/contract.rs line 149 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

Yes. Transfer Rate is only applied for XRPL originated tokens.

For Coreum tokens everything will work out of the box because of two reasons:

1. When a user sends funds to the contract to be bridged, the fees (commission and burn) will be applied on top of that amount so the contract will already receive the amount deducted.

2. We recently disabled commission fees / burning fees when smart contracts are sending transactions so we don't need to worry about them because they don't affect our contract, and we can send full amounts without these fees messing with our values.

2 - good point thx.

---

contract/src/tests.rs line 5022 at r3 (raw file):

        // amount_to_send = 99999999950001 / (1+0.499999999) = 66666666677778.444451.... -> which after rounding down is 66666666677778
        // After truncating (because sending precision is 10, we will get 66666666600000 as amount to send and 77778 extra collected as fees)
        // The rest, 99999999950001 - 66666666677778 = 33333333272223 will be burnt or sent back after transaction confirmation/rejection

I was confused with will be burnt here.
you mean that it is automatically gets burnt when tx is sent ?

Code quote:

will be burnt

---

contract/src/tests.rs line 5139 at r3 (raw file):

        // Check relayer balances
        for relayer in relayer_accounts.iter() {
            let request_balance_token1 = asset_ft

nit: I would do another for loop here and define denom => expected_balance as map

---

contract/src/tests.rs line 5261 at r3 (raw file):

        }

        // If transaction is rejected, contract should send back the amount sent + transfer fees to sender

but bridging fee is applied ?

---

spec/spec.md line 138 at r3 (raw file):

Each token in the registry contains the fee config which consists of a bridging fee and a transfer fee. The bridging fee
is the fee that relayers earn for the transaction relaying. That fee covers their costs and provides some profit on top.
The transfer fee is an optional fee that is charged on XRPL on top of each transfer and is based on the transfer

here you can also add link to xrpl doc of fees IMO

[keyleu]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

---

contract/src/tests.rs line 5022 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

I was confused with will be burnt here.
you mean that it is automatically gets burnt when tx is sent ?

no it means, will be burnt after transaction confirmation/rejection.

It doesn't burn at this point, but it will burn later when the evidences confirming that transaction was accepted in XRPL arrive to the contract.
If the evidences confirm that transaction was rejected, these fees are returned to the user.

If you check at the end of this test function I do the test for confirming/rejecting and check that they are burnt/sent back.

Extended comment for clarification.

---

contract/src/tests.rs line 5139 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: I would do another for loop here and define denom => expected_balance as map

You're totally right, not sure why I did it like this when I'm doing double loops almost everywhere. Changed it.

---

contract/src/tests.rs line 5261 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

but bridging fee is applied ?

Correct. Bridging fee is always applied at the moment of sending, no matter if transactions is later confirmed or rejected.
Every time the relayers have to relay send operations these fees are applied.

Extended comment for clarification

---

spec/spec.md line 138 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

here you can also add link to xrpl doc of fees IMO

Added.

[ysv]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @wojtek-coreum)

[dzmitryhil]

Reviewed 1 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @wojtek-coreum)