Skip to content

Commit

Permalink
Contract: Add Hexadecimal currency uppercase restrictions and generat…
Browse files Browse the repository at this point in the history
…ions (#71)

# Description
- We check that hexadecimal currencies registered are all uppercase.
- We check that we can't register XRP currency
- We generate the hexadecimal currencies in uppercase for Coreum
originated tokens representation.
  • Loading branch information
keyleu authored Dec 19, 2023
1 parent 0a750e4 commit 82fd1ca
Show file tree
Hide file tree
Showing 19 changed files with 155 additions and 60 deletions.
36 changes: 24 additions & 12 deletions contract/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ fn register_coreum_token(
.to_string()
.to_lowercase();

// Format will be the hex representation in XRPL of the string coreum<hash>
// Format will be the hex representation in XRPL of the string coreum<hash> in uppercase
let xrpl_currency =
convert_currency_to_xrpl_hexadecimal(format!("{}{}", COREUM_CURRENCY_PREFIX, hex_string));

Expand Down Expand Up @@ -799,11 +799,8 @@ fn send_to_xrpl(
amount_after_transfer_fees(amount_after_bridge_fees, xrpl_token.transfer_rate)?;

// We don't need any decimal conversion because the token is an XRPL originated token and they are issued with same decimals
(amount_to_send, remainder) = truncate_amount(
xrpl_token.sending_precision,
decimals,
amount_after_fees,
)?;
(amount_to_send, remainder) =
truncate_amount(xrpl_token.sending_precision, decimals, amount_after_fees)?;

handle_fee_collection(
deps.storage,
Expand Down Expand Up @@ -1002,11 +999,26 @@ pub fn validate_xrpl_issuer_and_currency(
) -> Result<(), ContractError> {
validate_xrpl_address(issuer).map_err(|_| ContractError::InvalidXRPLIssuer {})?;

// We check that currency is either a standard 3 character currency or it's a 40 character hex string currency
if !(currency.len() == 3 && currency.is_ascii()
|| currency.len() == 40 && currency.chars().all(|c| c.is_ascii_hexdigit()))
{
return Err(ContractError::InvalidXRPLCurrency {});
// We check that currency is either a standard 3 character currency or it's a 40 character hex string currency, any other scenario is invalid
match currency.len() {
3 => {
if !currency.is_ascii() {
return Err(ContractError::InvalidXRPLCurrency {});
}

if currency == "XRP" {
return Err(ContractError::InvalidXRPLCurrency {});
}
}
40 => {
if !currency
.chars()
.all(|c| c.is_ascii_hexdigit() && (c.is_numeric() || c.is_uppercase()))
{
return Err(ContractError::InvalidXRPLCurrency {});
}
}
_ => return Err(ContractError::InvalidXRPLCurrency {}),
}

Ok(())
Expand Down Expand Up @@ -1124,5 +1136,5 @@ fn is_token_xrp(issuer: String, currency: String) -> bool {

fn convert_currency_to_xrpl_hexadecimal(currency: String) -> String {
// Fill with zeros to get the correct hex representation in XRPL of our currency.
format!("{:0<40}", hex::encode(currency))
format!("{:0<40}", hex::encode(currency)).to_uppercase()
}
2 changes: 1 addition & 1 deletion contract/src/evidence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl Evidence {
Evidence::XRPLToCoreumTransfer { tx_hash, .. } => tx_hash.clone(),
Evidence::XRPLTransactionResult { tx_hash, .. } => tx_hash.clone().unwrap(),
}
.to_lowercase()
.to_uppercase()
}
pub fn is_operation_valid(&self) -> bool {
match self {
Expand Down
10 changes: 8 additions & 2 deletions contract/src/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,21 @@ pub fn handle_coreum_to_xrpl_transfer_confirmation(
// If transaction was accepted and the token that was sent back was an XRPL originated token, we must burn the token amount and transfer_fee
if transaction_result.eq(&TransactionResult::Accepted) {
let burn_msg = CosmosMsg::from(CoreumMsg::AssetFT(assetft::Msg::Burn {
coin: coin(amount.checked_add(transfer_fee)?.u128(), xrpl_token.coreum_denom),
coin: coin(
amount.checked_add(transfer_fee)?.u128(),
xrpl_token.coreum_denom,
),
}));

*response = response.to_owned().add_message(burn_msg);
} else {
// If transaction was rejected, we must send the token amount and transfer_fee back to the user
let send_msg = BankMsg::Send {
to_address: sender.to_string(),
amount: coins(amount.checked_add(transfer_fee)?.u128(), xrpl_token.coreum_denom),
amount: coins(
amount.checked_add(transfer_fee)?.u128(),
xrpl_token.coreum_denom,
),
};

*response = response.to_owned().add_message(send_msg);
Expand Down
50 changes: 50 additions & 0 deletions contract/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,14 @@ mod tests {
assert_eq!(query_coreum_tokens.tokens.len(), 2);
assert_eq!(query_coreum_tokens.tokens[0].denom, test_tokens[0].denom);
assert_eq!(query_coreum_tokens.tokens[1].denom, test_tokens[1].denom);
assert_eq!(
query_coreum_tokens.tokens[0].xrpl_currency,
query_coreum_tokens.tokens[0].xrpl_currency.to_uppercase()
);
assert_eq!(
query_coreum_tokens.tokens[1].xrpl_currency,
query_coreum_tokens.tokens[1].xrpl_currency.to_uppercase()
);

// Query tokens with limit
let query_coreum_tokens = wasm
Expand Down Expand Up @@ -859,6 +867,48 @@ mod tests {
.to_string()
.contains(ContractError::InvalidXRPLCurrency {}.to_string().as_str()));

// Registering a token with an invalid hexadecimal currency (not uppercase) should fail
let currency_error = wasm
.execute::<ExecuteMsg>(
&contract_addr,
&ExecuteMsg::RegisterXRPLToken {
issuer: test_tokens[1].issuer.clone(),
currency: "015841551A748AD2C1f76FF6ECB0CCCD00000000".to_string(),
sending_precision: test_tokens[1].sending_precision.clone(),
max_holding_amount: test_tokens[1].max_holding_amount.clone(),
bridging_fee: test_tokens[1].bridging_fee,
transfer_rate: test_tokens[0].transfer_rate,
},
&query_issue_fee(&asset_ft),
&signer,
)
.unwrap_err();

assert!(currency_error
.to_string()
.contains(ContractError::InvalidXRPLCurrency {}.to_string().as_str()));

// Registering a token with an "XRP" as currency should fail
let currency_error = wasm
.execute::<ExecuteMsg>(
&contract_addr,
&ExecuteMsg::RegisterXRPLToken {
issuer: test_tokens[1].issuer.clone(),
currency: "XRP".to_string(),
sending_precision: test_tokens[1].sending_precision.clone(),
max_holding_amount: test_tokens[1].max_holding_amount.clone(),
bridging_fee: test_tokens[1].bridging_fee,
transfer_rate: test_tokens[0].transfer_rate,
},
&query_issue_fee(&asset_ft),
&signer,
)
.unwrap_err();

assert!(currency_error
.to_string()
.contains(ContractError::InvalidXRPLCurrency {}.to_string().as_str()));

// Registering a token with an invalid transfer_rate should fail.
let transfer_rate_error = wasm
.execute::<ExecuteMsg>(
Expand Down
12 changes: 7 additions & 5 deletions integration-tests/coreum/contract_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ func TestRegisterCoreumToken(t *testing.T) {
require.NoError(t, chains.XRPL.AutoFillSignAndSubmitTx(ctx, t, &paymentTx, issuerAcc))
balancesAfterSend := chains.XRPL.GetAccountBalances(ctx, t, recipientAcc)
t.Logf("Recipient account balances after send: %s", balancesAfterSend)
receiveAmount, ok := balancesAfterSend[fmt.Sprintf("%s/%s", currency.String(), issuerAcc.String())]
receiveAmount, ok := balancesAfterSend[fmt.Sprintf("%s/%s",
xrpl.ConvertCurrencyToString(currency), issuerAcc.String(),
)]
require.True(t, ok)
require.Equal(t, amountToSend.String(), receiveAmount.Value.String())
}
Expand Down Expand Up @@ -842,13 +844,13 @@ func TestSendFromXRPLToCoreumXRPToken(t *testing.T) {
registeredXRPToken, err := contractClient.GetXRPLTokenByIssuerAndCurrency(
ctx,
xrpl.XRPTokenIssuer.String(),
xrpl.XRPTokenCurrency.String(),
xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
)
require.NoError(t, err)

require.Equal(t, coreum.XRPLToken{
Issuer: xrpl.XRPTokenIssuer.String(),
Currency: xrpl.XRPTokenCurrency.String(),
Currency: xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
CoreumDenom: assetfttypes.BuildDenom("drop", contractClient.GetContractAddress()),
SendingPrecision: 6,
MaxHoldingAmount: sdkmath.NewInt(10000000000000000),
Expand Down Expand Up @@ -2135,7 +2137,7 @@ func TestSendFromCoreumToXRPLXRPToken(t *testing.T) {
xrpl.GenPrivKeyTxSigner().Account().String(),
)
registeredXRPToken, err := contractClient.GetXRPLTokenByIssuerAndCurrency(
ctx, xrpl.XRPTokenIssuer.String(), xrpl.XRPTokenCurrency.String(),
ctx, xrpl.XRPTokenIssuer.String(), xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
)
require.NoError(t, err)

Expand Down Expand Up @@ -3008,5 +3010,5 @@ func genXRPLTxHash(t *testing.T) string {
_, err := rand.Read(hash[:])
require.NoError(t, err)

return hash.String()
return strings.ToUpper(hash.String())
}
4 changes: 2 additions & 2 deletions integration-tests/processes/send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestSendXRPTokenFromXRPLToCoreumAndBack(t *testing.T) {
t.Logf("XRPL sender: %s", xrplSenderAddress.String())

registeredXRPToken, err := runnerEnv.ContractClient.GetXRPLTokenByIssuerAndCurrency(
ctx, xrpl.XRPTokenIssuer.String(), xrpl.XRPTokenCurrency.String(),
ctx, xrpl.XRPTokenIssuer.String(), xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
)
require.NoError(t, err)

Expand Down Expand Up @@ -603,7 +603,7 @@ func TestRecoverXRPLOriginatedTokenRegistrationAndSendFromXRPLToCoreumAndBack(t
runnerEnv.Chains.XRPL.CreateAccount(ctx, t, xrplIssuerAddress, 1)
// recover from owner
_, err = runnerEnv.ContractClient.RecoverXRPLTokenRegistration(
ctx, runnerEnv.ContractOwner, xrplIssuerAddress.String(), registeredXRPLCurrency.String(),
ctx, runnerEnv.ContractOwner, xrplIssuerAddress.String(), xrpl.ConvertCurrencyToString(registeredXRPLCurrency),
)
require.NoError(t, err)
runnerEnv.AwaitNoPendingOperations(ctx, t)
Expand Down
11 changes: 8 additions & 3 deletions integration-tests/xrpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ func (c XRPLChain) AutoFillTx(ctx context.Context, t *testing.T, tx rippledata.T
func (c XRPLChain) GetAccountBalance(
ctx context.Context, t *testing.T, account, issuer rippledata.Account, currency rippledata.Currency,
) rippledata.Amount {
balance, ok := c.GetAccountBalances(ctx, t, account)[fmt.Sprintf("%s/%s", currency.String(), issuer.String())]
balance, ok := c.GetAccountBalances(ctx, t, account)[fmt.Sprintf("%s/%s",
xrpl.ConvertCurrencyToString(currency), issuer.String())]
if !ok {
// equal to zero
return rippledata.Amount{
Expand All @@ -254,7 +255,9 @@ func (c XRPLChain) GetAccountBalances(

accInfo, err := c.rpcClient.AccountInfo(ctx, acc)
require.NoError(t, err)
amounts[fmt.Sprintf("%s/%s", xrpl.XRPTokenCurrency.String(), xrpl.XRPTokenIssuer.String())] = rippledata.Amount{
amounts[fmt.Sprintf(
"%s/%s",
xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency), xrpl.XRPTokenIssuer.String())] = rippledata.Amount{
Value: accInfo.AccountData.Balance,
}
// none xrp amounts
Expand All @@ -263,7 +266,9 @@ func (c XRPLChain) GetAccountBalances(

for _, line := range accLines.Lines {
lineCopy := line
amounts[fmt.Sprintf("%s/%s", lineCopy.Currency.String(), lineCopy.Account.String())] = rippledata.Amount{
amounts[fmt.Sprintf(
"%s/%s",
xrpl.ConvertCurrencyToString(lineCopy.Currency), lineCopy.Account.String())] = rippledata.Amount{
Value: &lineCopy.Balance.Value,
Currency: lineCopy.Currency,
Issuer: lineCopy.Account,
Expand Down
8 changes: 5 additions & 3 deletions integration-tests/xrpl/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ package xrpl_test
import (
"context"
"fmt"
"strings"
"testing"

rippledata "github.com/rubblelabs/ripple/data"
"github.com/samber/lo"
"github.com/stretchr/testify/require"

integrationtests "github.com/CoreumFoundation/coreumbridge-xrpl/integration-tests"
"github.com/CoreumFoundation/coreumbridge-xrpl/relayer/xrpl"
)

func TestXRPAndIssuedTokensPayment(t *testing.T) {
Expand Down Expand Up @@ -146,7 +148,7 @@ func TestMultisigPayment(t *testing.T) {

// compare hashes
t.Logf("TwoSignersHash/ThreeSignersHash: %s/%s", xrpPaymentTxTwoSigners.Hash, xrpPaymentTxThreeSigners.Hash)
require.NotEqual(t, xrpPaymentTxTwoSigners.Hash.String(), xrpPaymentTxThreeSigners.Hash.String())
require.NotEqual(t, strings.ToUpper(xrpPaymentTxTwoSigners.GetHash().String()), xrpPaymentTxThreeSigners.Hash.String())

t.Logf(
"Recipient account balance before: %s",
Expand Down Expand Up @@ -865,8 +867,8 @@ func getBalanceAccount(
currency rippledata.Currency,
) string {
t.Helper()
issuerCurrencyKey := fmt.Sprintf("%s/%s", currency.String(), issuer.String())
balance, ok := xrplChain.GetAccountBalances(ctx, t, acc)[issuerCurrencyKey]
currencyIssuerKey := fmt.Sprintf("%s/%s", xrpl.ConvertCurrencyToString(currency), issuer.String())
balance, ok := xrplChain.GetAccountBalances(ctx, t, acc)[currencyIssuerKey]
if !ok {
return "0"
}
Expand Down
9 changes: 5 additions & 4 deletions integration-tests/xrpl/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package xrpl_test

import (
"context"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -121,7 +122,7 @@ func TestRecentHistoryScanAccountTx(t *testing.T) {
spawn("wait", parallel.Exit, func(ctx context.Context) error {
t.Logf("Waiting for %d transactions to be scanned by the scanner", txsCount)
for tx := range txsCh {
receivedTxHashes[tx.GetHash().String()] = struct{}{}
receivedTxHashes[strings.ToUpper(tx.GetHash().String())] = struct{}{}
if len(receivedTxHashes) == txsCount {
return nil
}
Expand Down Expand Up @@ -156,11 +157,11 @@ func sendMultipleTxs(
err = xrplChain.AutoFillSignAndSubmitTx(ctx, t, &xrpPaymentTx, senderAcc)
if errors.Is(err, context.Canceled) {
// we add the hash here since we cancel the context once we read it
writtenTxHashes[xrpPaymentTx.Hash.String()] = struct{}{}
writtenTxHashes[strings.ToUpper(xrpPaymentTx.GetHash().String())] = struct{}{}
return writtenTxHashes
}
require.NoError(t, err)
writtenTxHashes[xrpPaymentTx.Hash.String()] = struct{}{}
writtenTxHashes[strings.ToUpper(xrpPaymentTx.GetHash().String())] = struct{}{}
}
t.Logf("Successfully sent %d transactions", len(writtenTxHashes))
return writtenTxHashes
Expand All @@ -182,7 +183,7 @@ func validateTxsHashesInChannel(
return scanCtx.Err()
case tx := <-txsCh:
// validate that we have all sent hashed and no duplicated
hash := tx.GetHash().String()
hash := strings.ToUpper(tx.GetHash().String())
if _, found := expectedHashes[hash]; !found {
return errors.Errorf("not found expected tx hash:%s", hash)
}
Expand Down
2 changes: 1 addition & 1 deletion relayer/processes/amount.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,5 +121,5 @@ func convertCoreumAmountToXRPLAmountWithDecimals(
}

func isXRPToken(issuer, currency string) bool {
return issuer == xrpl.XRPTokenIssuer.String() && currency == xrpl.XRPTokenCurrency.String()
return issuer == xrpl.XRPTokenIssuer.String() && currency == xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency)
}
8 changes: 4 additions & 4 deletions relayer/processes/amount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,28 @@ func TestConvertXRPLOriginatedTokenCoreumAmountToXRPLAmount(t *testing.T) {
name: "one_coreum_XRP_to_XRPL_XRP",
coreumAmount: sdkmath.NewIntFromUint64(1_000_000),
issuer: xrpl.XRPTokenIssuer.String(),
currency: xrpl.XRPTokenCurrency.String(),
currency: xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
want: amountStringToXRPLAmount(t, "1.0XRP"),
},
{
name: "one_with_some_decimals_coreum_XRP_to_XRPL_XRP",
coreumAmount: sdkmath.NewIntFromUint64(1000100),
issuer: xrpl.XRPTokenIssuer.String(),
currency: xrpl.XRPTokenCurrency.String(),
currency: xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
want: amountStringToXRPLAmount(t, "1.0001XRP"),
},
{
name: "min_decimals_coreum_XRP_to_XRPL_XRP",
coreumAmount: sdkmath.NewIntFromUint64(999000001),
issuer: xrpl.XRPTokenIssuer.String(),
currency: xrpl.XRPTokenCurrency.String(),
currency: xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
want: amountStringToXRPLAmount(t, "999.000001XRP"),
},
{
name: "high_value_coreum_XRP_to_XRPL_XRP",
coreumAmount: sdkmath.NewIntFromUint64(1000000000000001),
issuer: xrpl.XRPTokenIssuer.String(),
currency: xrpl.XRPTokenCurrency.String(),
currency: xrpl.ConvertCurrencyToString(xrpl.XRPTokenCurrency),
want: amountStringToXRPLAmount(t, "1000000000.000001XRP"),
},
{
Expand Down
Loading

0 comments on commit 82fd1ca

Please sign in to comment.