Implement TrustSet on the XRPL bridge account for the registered XRPL tokens.

[dzmitryhil]

Implement TrustSet on the XRPL bridge account for the registered XRPL tokens.

Description

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 

[ysv]

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

---

integration-tests/coreum/contract_client_test.go line 367 at r1 (raw file):

	require.NotNil(t, operation.OperationType.TrustSet)

	rejectTxEvidenceTrustSet := coreum.XRPLTransactionResultTrustSetEvidence{

nit: rejectedTxEvidenceTrustSet

---

integration-tests/coreum/contract_client_test.go line 428 at r1 (raw file):

	require.True(t, coreum.IsXRPLTokenNotEnabledError(err), err)

	_ = activeCurrency

do we need this ?

---

integration-tests/processes/send_from_xrpl_to_coreum_test.go line 159 at r1 (raw file):

	require.NoError(t, err)

	// fund owner to cover registration fees

nit: cover issuance fee ?

---

relayer/coreum/contract.go line 648 at r1 (raw file):

// GetXRPLToken returns an XRPL registered token or nil.
func (c *ContractClient) GetXRPLToken(ctx context.Context, issuer, currency string) (*XRPLToken, error) {
	tokens, err := c.GetXRPLTokens(ctx)

question:
Does this query return all tokens or enabled only?

---

relayer/processes/amount.go line 18 at r1 (raw file):

	// XRPLIssuedCurrencyDecimals is XRPL decimals used the on coreum.
	XRPLIssuedCurrencyDecimals = 15
	// XRPIssuer is XRP issuer name used the on coreum.

nit: WDYM used the on coreum ?

on the coreum probably ?

Also we don't use this amount and currency on coreum these values are used in XRPL, no ?

---

relayer/processes/amount.go line 54 at r1 (raw file):

		xrplValue, err := rippledata.NewValue(amountString, true)
		if err != nil {
			return rippledata.Amount{}, errors.Wrapf(err, "failed to convert amount stringy to ripple.Value, amount stirng: %s", amountString)

nit: amount stirng:

-> string

---

relayer/processes/xrpl_tx_observer.go line 171 at r1 (raw file):

// IsExpectedSendEvidenceError returns true is error is cause of the re-submitting of the transaction.
func IsExpectedSendEvidenceError(err error) bool {

more clear name would be: IsEvidenceErrorCausedByResubmition

---

relayer/processes/xrpl_tx_submitter_test.go line 383 at r1 (raw file):

}

func multiSignTrustSetsOperation(

nit: multiSignTrustSetOperation ?

[dzmitryhil]

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

---

integration-tests/coreum/contract_client_test.go line 367 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: rejectedTxEvidenceTrustSet

Done

---

integration-tests/coreum/contract_client_test.go line 428 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

do we need this ?

Removed.

---

integration-tests/processes/send_from_xrpl_to_coreum_test.go line 159 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: cover issuance fee ?

Done

---

relayer/coreum/contract.go line 648 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

question:
Does this query return all tokens or enabled only?

All.

---

relayer/processes/amount.go line 18 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: WDYM used the on coreum ?

on the coreum probably ?

Also we don't use this amount and currency on coreum these values are used in XRPL, no ?

Updated comment. Did't get last question.

---

relayer/processes/amount.go line 54 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: amount stirng:

-> string

Done

---

relayer/processes/xrpl_tx_observer.go line 171 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

more clear name would be: IsEvidenceErrorCausedByResubmition

Agree, updated.

---

relayer/processes/xrpl_tx_submitter_test.go line 383 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: multiSignTrustSetOperation ?

Done

[keyleu]

Reviewed 8 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @miladz68, @wojtek-coreum, and @ysv)

---

integration-tests/coreum/contract_client_test.go line 297 at r2 (raw file):

	issuerAcc := chains.XRPL.GenAccount(ctx, t, 0)
	issuer := issuerAcc.String()
	invactiveCurrency := "INA"

should be inactive not invactive

---

integration-tests/processes/ticket_allocation_test.go line 201 at r2 (raw file):

	// register more than threshold to activate tickets re-allocation
	numberOfXRPLTokensToRegister := int(numberOfTicketsToAllocate) - 1

maybe change this to UsedTicketsThreshold+1 to be more accurate with comment

---

relayer/coreum/contract.go line 658 at r2 (raw file):

	}

	return nil, nil //nolint:nilnil // is token not found we return nil instead of an error

if token is not found

---

relayer/coreum/contract.go line 902 at r2 (raw file):

}

// IsXRPLTokenNotEnabledError returns true if error is `XRPLTokenNotEnabled` error.

you can remove the last error from comment.

---

relayer/processes/amount.go line 35 at r2 (raw file):

		return sdkmath.NewIntFromBigInt(xrplRatAmount.Num()), nil
	}
	// not XRP value is repressed as value multiplied by 10^15

I suggest keeping scientific notation everywhere as a standard (like in spec). Use 1e15 (instead of 10^15) notation everywhere. I will double check the contract for this as well.

[dzmitryhil]

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

---

integration-tests/coreum/contract_client_test.go line 297 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

should be inactive not invactive

Done

---

integration-tests/processes/ticket_allocation_test.go line 201 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

maybe change this to UsedTicketsThreshold+1 to be more accurate with comment

Agree, done.

---

relayer/coreum/contract.go line 658 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

if token is not found

Done

---

relayer/coreum/contract.go line 902 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

you can remove the last error from comment.

Done.

---

relayer/processes/amount.go line 35 at r2 (raw file):

Previously, keyleu (Keyne) wrote…

I suggest keeping scientific notation everywhere as a standard (like in spec). Use 1e15 (instead of 10^15) notation everywhere. I will double check the contract for this as well.

Done

[ysv]

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

---

integration-tests/processes/ticket_allocation_test.go line 214 at r3 (raw file):

	runnerEnv.AwaitNoPendingOperations(ctx, t)

	availableTicketsAfterReAllocation, err := runnerEnv.ContractClient.GetAvailableTickets(ctx)

nit: Reallocation single word

---

relayer/processes/amount.go line 18 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Updated comment. Did't get last question.

// XRPIssuer is XRP issuer name used on the coreum.
XRPIssuer = "rrrrrrrrrrrrrrrrrrrrrho"

but this it is XRP issuer address used on XRPL ?

[dzmitryhil]

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

---

integration-tests/processes/ticket_allocation_test.go line 214 at r3 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

nit: Reallocation single word

Done

---

relayer/processes/amount.go line 18 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

// XRPIssuer is XRP issuer name used on the coreum.
XRPIssuer = "rrrrrrrrrrrrrrrrrrrrrho"

but this it is XRP issuer address used on XRPL ?

Clear now, updated.

[keyleu]

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

[ysv]

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

---

relayer/processes/amount.go line 18 at r1 (raw file):

Previously, dzmitryhil (Dzmitry Hil) wrote…

Clear now, updated.

For simplicity we consider that issuer for XRP on XRPL is rrrrrrrrrrrrrrrrrrrrrho and use this value to generate corresponding token on Coreum side. This is done to unify representation & flows for all XRPL originated tokens (XRP and issued tokens) on bridge side.

[dzmitryhil]

Reviewable status: 30 of 31 files reviewed, 1 unresolved discussion (waiting on @keyleu, @miladz68, @wojtek-coreum, and @ysv)

---

relayer/processes/amount.go line 18 at r1 (raw file):

Previously, ysv (Yaroslav Savchuk) wrote…

For simplicity we consider that issuer for XRP on XRPL is rrrrrrrrrrrrrrrrrrrrrho and use this value to generate corresponding token on Coreum side. This is done to unify representation & flows for all XRPL originated tokens (XRP and issued tokens) on bridge side.

Updated taken your example.

[ysv]

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

[keyleu]

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