Add XRPL to coreum and back rounding spec with the test cases.

[dzmitryhil]

Description

Add XRPL to coreum and back rounding spec with the test cases.

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 

[wojtek-coreum]

Reviewed all commit messages.
Reviewable status: 0 of 5 files reviewed, 3 unresolved discussions (waiting on @dzmitryhil, @miladz68, and @ysv)

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

const (
	// MinMinSendingDecimals - min decimals we allow to use for the truncation and rounding.
	MinMinSendingDecimals = -15

Why MinMin?

---

relayer/fee/rounding_test.go line 18 at r1 (raw file):

	MinMinSendingDecimals = -15
	// MaxMinSendingDecimals - max decimals we allow to use for the truncation and rounding.
	MaxMinSendingDecimals = 16

why MaxMin?

---

relayer/fee/rounding_test.go line 20 at r1 (raw file):

	MaxMinSendingDecimals = 16
	// TransferRateDenominator - the rate denominator XRPL uses for the transfer.
	// e.g. transferRate of 1005000000 is equivalent to a transfer fee of 0.5%.

could you explain this math? I understand that this is like 100.5, but why is 100 there?

[dzmitryhil]

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

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

Why MinMin?

Min and Max becase MinSendingDecimals should be in ranged.

---

relayer/fee/rounding_test.go line 18 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

why MaxMin?

Replied in prev PR.

---

relayer/fee/rounding_test.go line 20 at r1 (raw file):

Previously, wojtek-coreum (Wojtek) wrote…

could you explain this math? I understand that this is like 100.5, but why is 100 there?

The 1000000000 is static deniminator for the tokens with fee. So if you use fee rate = 1005000000 fee is 1005000000/1000000000 = 1.005, which means that 0.005 will be sent to issuer.

[ysv]

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

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Min and Max becase MinSendingDecimals should be in ranged.

still not clear for me
why not MinSendingDecimals and MaxSendingDecimals

---

relayer/fee/rounding_test.go line 41 at r1 (raw file):

			sendingValue:        convertStringToRippleValue(t, "1111.001111"),
			minSendingPrecision: 3,
			tokenDecimals:       5,

why tokenDecimals is 5 everywhere ?
I thought we decided to always use 15 ?

---

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

does a pre-validation of the value. If, after the calculation, the `receivedAmount <= 0` or
`receivedAmount > max allowed value` returns an error. Once the evidence threshold is reached, the contract executes
the calculation one more time, and sends the amount to the recipient. The rounding reminder is left on the contract,

nit: actually it is stored on multisig addr on XRPL side

Code quote:

left on the contract,

---

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

1. Send low and high amount to coreum and return high and low back.
   1.1. XRPLUser sends 10 XRPL native token to coreumUser (bridge account balance: 10, coreumUser balance: 10)

but I guess it doesn't work like this for xrp
Only for issued currencies

Or by native token you mean issued token. Because native token of XRPL is XRP

---

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

The `min sending precision` is the number in range `-15:17` we use for each register token. That value defines the min
precision for the amount you can send. Base on the `min sending precision` we can define the max amount for the
single sending equal to `1e(18 - min sending precision)-1`. That max amount together with the rounding rules will keep

I think this value is the limit of balance we can safely hold on our multisig acc

Code quote:

single sending equal to `1e(18 - min sending precision)-1`

[dzmitryhil]

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

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

still not clear for me
why not MinSendingDecimals and MaxSendingDecimals

MinSendingDecimals is param. MinMinSendingDecimals - min allowed value, max - max.

---

relayer/fee/rounding_test.go line 41 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

why tokenDecimals is 5 everywhere ?
I thought we decided to always use 15 ?

Just for the simplicity of the calculation, with 15 you will get a lot of zeros.

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

nit: actually it is stored on multisig addr on XRPL side

Right, updated.

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

but I guess it doesn't work like this for xrp
Only for issued currencies

Or by native token you mean issued token. Because native token of XRPL is XRP

I guess it works the same.

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

I think this value is the limit of balance we can safely hold on our multisig acc

This is the amount we can correctly convert to XRPL float with the min sending precision. That's why this is the max sending value. Technically we can limit XRPL native tokens with the trustset equal to 1e(18 - min sending precision)-1, but that will overcoplicate the min sending precision updated and won't give us 100% protection anyway. So since we still do not have a solution for the Issue 2 I propose not to try to fix it for Issue 1, we do soft protection, where it is hard (almost impossible) to reach such a state where it is possible to archive either Issue 1 or Issue 2 .

[ysv]

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

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

MinSendingDecimals is param. MinMinSendingDecimals - min allowed value, max - max.

decided to rename var MinSendingDecimals -> SendingPrecision

and then we will have Min/MaxSendingPrecision

[dzmitryhil]

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

---

relayer/fee/rounding_test.go line 16 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

decided to rename var MinSendingDecimals -> SendingPrecision

and then we will have Min/MaxSendingPrecision

Done.

[miladz68]

Reviewed 3 of 5 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @wojtek-coreum and @ysv)

[ysv]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @dzmitryhil and @wojtek-coreum)

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

I guess it works the same.

I don't think so
AFAIK they use drops which is int to store XRP value

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

This is the amount we can correctly convert to XRPL float with the min sending precision. That's why this is the max sending value. Technically we can limit XRPL native tokens with the trustset equal to 1e(18 - min sending precision)-1, but that will overcoplicate the min sending precision updated and won't give us 100% protection anyway. So since we still do not have a solution for the Issue 2 I propose not to try to fix it for Issue 1, we do soft protection, where it is hard (almost impossible) to reach such a state where it is possible to archive either Issue 1 or Issue 2 .

I see
Makes sense
Lets quickly discuss after daily I have a few questions to this approach

[dzmitryhil]

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

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

I don't think so
AFAIK they use drops which is int to store XRP value

OK, got your concern. Tested it on the localnet, and the issue is that I don't have enough XRP tokens on the localnet to reach the high enough amount. What also doesn't let me do it is 10XRP required reserve. So the balance of the user is always greater than 10XRP. As a result, to check the XRP minting I need about 10e18 which I simply don't have.
Anyways the rule with the SendingPrecision should be unified for all tokens, for the XRP we will have just use 6 value.

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

I see
Makes sense
Lets quickly discuss after daily I have a few questions to this approach

Sure.

[dzmitryhil]

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

---

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

submitting on XRPL transaction just one transaction. But with that coefficient, the account will have to submit
`1e complexityCoefficient` transactions minimum to do it. The recommended value for the `complexityCoefficient`
is `4`, so it means `10000` transactions minimum are needed to produce the `significant amount`.

@ysv @miladz68 @keyleu @wojtek-coreum
Let's discuss that value, not sure that it should be 4 isn't it too strict?

---

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

is `4`, so it means `10000` transactions minimum are needed to produce the `significant amount`.

The `maxHoldingAmount` is `1e(16 - sending precision)`.

@ysv @miladz68 @keyleu @wojtek-coreum
Let's discuss that formula because now the value for each token is a bit less than the total supply.
But we can make it less and as a result make the complexityCoefficient less as well.

[ysv]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dzmitryhil, @keyleu, @miladz68, and @wojtek-coreum)

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

@ysv @miladz68 @keyleu @wojtek-coreum
Let's discuss that value, not sure that it should be 4 isn't it too strict?

3-4 seems reasonable
Lets discuss

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

@ysv @miladz68 @keyleu @wojtek-coreum
Let's discuss that formula because now the value for each token is a bit less than the total supply.
But we can make it less and as a result make the complexityCoefficient less as well.

Lets discuss

---

spec/spec.md line 274 at r4 (raw file):

provides a `risks handling` when an account holding the total supply is able to produce `significant amount`
submitting on XRPL transaction just one transaction. But with that coefficient, the account will have to submit
`1e complexityCoefficient` transactions minimum to do it. The recommended value for the `complexityCoefficient`

nit: maybe writing here it as 10^complexityCoefficient will be more clear

[miladz68]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dzmitryhil, @keyleu, and @wojtek-coreum)

[ysv]

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

---

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

Previously, dzmitryhil (Dzmitry Hil) wrote…

But we wanted to keep it flexible, so IMO better not to have the relationships on the contract level

discussing in slack

---

spec/spec.md line 72 at r5 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Done.

because if we allow both increase and decrease for both of them then:

• if we decrease max_holding. Contract might end up with more balance than current max_holding
• if we allow to increase sending_precision. Someone might be able to bridge to one side but will not be able to bridge back because of sending_precision increase

But IMO these cases are rare and we should not handle them on contract now.
Just keep in mind & maybe add to specs

[dzmitryhil]

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

---

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

Previously, ysv (Yaroslav Savchuk) wrote…

discussing in slack

Added new rule, that max holding amount > balance in the update max holding amount section. Becase we don't need to limit the min amount, the less we allow to hold and less the precision, the less the chance to reproduce sending issues.

---

spec/spec.md line 72 at r5 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

because if we allow both increase and decrease for both of them then:

• if we decrease max_holding. Contract might end up with more balance than current max_holding
• if we allow to increase sending_precision. Someone might be able to bridge to one side but will not be able to bridge back because of sending_precision increase

But IMO these cases are rare and we should not handle them on contract now.
Just keep in mind & maybe add to specs

Right, util free amount is enough, but that might be for the case when max holding amount < total supply, which is expected.

[miladz68]

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @keyleu, @wojtek-coreum, and @ysv)

[ysv]

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu and @wojtek-coreum)

[dzmitryhil]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @keyleu and @wojtek-coreum)

---

spec/spec.md line 248 at r7 (raw file):

`max holding amount` for to hold per token. We have both limits to make hardly possible both issues described before.
The rounding with `sending precision` will eliminate the risk of sending to low amount which could influence on the
bridge, and the `max holding amount` will significantly limit the chance to hold and amount using which a holder can

Agreed to have:

mha = 1e^(16 - sp) - the rule for the max holding amount

cc := 4
sp = 4
spo = 0
mha = 1e16

cc := 4
sp = 5
spo = 1
mha = 1e15

(we don't validate it in the contract and use as a docs reference)

[keyleu]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @wojtek-coreum)

[keyleu]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @wojtek-coreum)

[keyleu]

Reviewable status: 5 of 7 files reviewed, all discussions resolved (waiting on @miladz68, @wojtek-coreum, and @ysv)

---

spec/spec.md line 43 at r8 (raw file):

The XRP token is registered in the token registry on the contract instantiation. That token doesn't have an issuer and
currency which indicates that this is the XRP token. That token can be enabled or disabled by the owner similar to other

We decided to use these in the contract for XRP.

const XRP_CURRENCY: &str = "XRP";
const XRP_ISSUER: &str = "rrrrrrrrrrrrrrrrrrrrrho";

---

spec/spec.md line 44 at r8 (raw file):

The XRP token is registered in the token registry on the contract instantiation. That token doesn't have an issuer and
currency which indicates that this is the XRP token. That token can be enabled or disabled by the owner similar to other
tokens. Similar to XRPL native tokens the XRP token has the `sending precision` and `max holding amount`.

You can mention we will have default values for these at the start.

[keyleu]

Reviewed 1 of 2 files at r3, 4 of 6 files at r7, 2 of 2 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68, @wojtek-coreum, and @ysv)

[ysv]

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

[dzmitryhil]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @miladz68 and @wojtek-coreum)

---

spec/spec.md line 43 at r8 (raw file):

Previously, keyleu (Keyne) wrote…

We decided to use these in the contract for XRP.

const XRP_CURRENCY: &str = "XRP";
const XRP_ISSUER: &str = "rrrrrrrrrrrrrrrrrrrrrho";

Done.

---

spec/spec.md line 44 at r8 (raw file):

Previously, keyleu (Keyne) wrote…

You can mention we will have default values for these at the start.

Done.

[ysv]

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

[keyleu]

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