Contract: Check relayers are not prohibited addresses (#182)

contract/src/address.rs

@@ -1,8 +1,9 @@
-use crate::error::ContractError;
+use crate::{error::ContractError, state::PROHIBITED_XRPL_ADDRESSES};
 use bs58::Alphabet;
+use cosmwasm_std::Storage;
 use sha2::{Digest, Sha256};
 
-pub fn validate_xrpl_address(address: &str) -> Result<(), ContractError> {
+pub fn validate_xrpl_address_format(address: &str) -> Result<(), ContractError> {
     // We need to use the base58 dictionary for ripple which is rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz
     // To understand this alphabet, see https://xrpl.org/base58-encodings.html#ripple-base58-alphabet
     // In short, the alphabet represents the bytes values in the address. r = 0, p = 1, s = 2, etc.
@@ -43,3 +44,20 @@ pub fn validate_xrpl_address(address: &str) -> Result<(), ContractError> {
 pub fn checksum(data: &[u8]) -> Vec<u8> {
     Sha256::digest(Sha256::digest(data)).to_vec()
 }
+
+pub fn validate_xrpl_address_is_not_prohibited(
+    storage: &dyn Storage,
+    address: String,
+) -> Result<(), ContractError> {
+    if PROHIBITED_XRPL_ADDRESSES.has(storage, address) {
+        return Err(ContractError::ProhibitedAddress {});
+    }
+    Ok(())
+}
+
+// Checks that address is a valid XRPL address and that is not in the list of prohibited addresses
+pub fn validate_xrpl_address(storage: &dyn Storage, address: String) -> Result<(), ContractError> {
+    validate_xrpl_address_format(&address)?;
+    validate_xrpl_address_is_not_prohibited(storage, address)?;
+    Ok(())
+}

contract/src/contract.rs

@@ -1,15 +1,17 @@
 use std::collections::VecDeque;
 
 use crate::{
-    address::validate_xrpl_address,
+    address::{validate_xrpl_address, validate_xrpl_address_format},
     error::ContractError,
-    evidence::OperationResult::TicketsAllocation,
-    evidence::{handle_evidence, hash_bytes, Evidence, TransactionResult},
+    evidence::{
+        handle_evidence, hash_bytes, Evidence, OperationResult::TicketsAllocation,
+        TransactionResult,
+    },
     fees::{amount_after_bridge_fees, handle_fee_collection, substract_relayer_fees},
     msg::{
         AvailableTicketsResponse, BridgeStateResponse, CoreumTokensResponse, ExecuteMsg,
         FeesCollectedResponse, InstantiateMsg, PendingOperationsResponse, PendingRefund,
-        PendingRefundsResponse, ProcessedTxsResponse, ProhibitedXRPLRecipientsResponse, QueryMsg,
+        PendingRefundsResponse, ProcessedTxsResponse, ProhibitedXRPLAddressesResponse, QueryMsg,
         TransactionEvidence, TransactionEvidencesResponse, XRPLTokensResponse,
     },
     operation::{
@@ -22,7 +24,7 @@ use crate::{
         BridgeState, Config, ContractActions, CoreumToken, TokenState, UserType, XRPLToken,
         AVAILABLE_TICKETS, CONFIG, COREUM_TOKENS, FEES_COLLECTED, PENDING_OPERATIONS,
         PENDING_REFUNDS, PENDING_ROTATE_KEYS, PENDING_TICKET_UPDATE, PROCESSED_TXS,
-        PROHIBITED_XRPL_RECIPIENTS, TX_EVIDENCES, USED_TICKETS_COUNTER, XRPL_TOKENS,
+        PROHIBITED_XRPL_ADDRESSES, TX_EVIDENCES, USED_TICKETS_COUNTER, XRPL_TOKENS,
     },
     tickets::{allocate_ticket, register_used_ticket},
     token::{
@@ -88,7 +90,7 @@ pub const MIN_DENOM_LENGTH: usize = 3;
 pub const MAX_DENOM_LENGTH: usize = 128;
 pub const DENOM_SPECIAL_CHARACTERS: [char; 5] = ['/', ':', '.', '_', '-'];
 
-pub const INITIAL_PROHIBITED_XRPL_RECIPIENTS: [&str; 5] = [
+pub const INITIAL_PROHIBITED_XRPL_ADDRESSES: [&str; 5] = [
     "rrrrrrrrrrrrrrrrrrrrrhoLvTp", // ACCOUNT_ZERO: An address that is the XRP Ledger's base58 encoding of the value 0. In peer-to-peer communications, rippled uses this address as the issuer for XRP.
     "rrrrrrrrrrrrrrrrrrrrBZbvji", // ACCOUNT_ONE: An address that is the XRP Ledger's base58 encoding of the value 1. In the ledger, RippleState entries use this address as a placeholder for the issuer of a trust line balance.
     "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", // Genesis account: When rippled starts a new genesis ledger from scratch (for example, in stand-alone mode), this account holds all the XRP. This address is generated from the seed value masterpassphrase which is hard-coded.
@@ -110,14 +112,20 @@ pub fn instantiate(
         Some(deps.api.addr_validate(msg.owner.as_ref())?.as_ref()),
     )?;
 
+    // We store all the prohibited addresses in state, including the multisig address, which is also prohibited to send to
+    for address in INITIAL_PROHIBITED_XRPL_ADDRESSES {
+        PROHIBITED_XRPL_ADDRESSES.save(deps.storage, address.to_string(), &Empty {})?;
+    }
+    PROHIBITED_XRPL_ADDRESSES.save(deps.storage, msg.bridge_xrpl_address.clone(), &Empty {})?;
+
     validate_relayers(
         deps.as_ref().into_empty(),
         &msg.relayers,
         msg.evidence_threshold,
     )?;
 
     // The multisig address on XRPL must be valid
-    validate_xrpl_address(&msg.bridge_xrpl_address)?;
+    validate_xrpl_address_format(&msg.bridge_xrpl_address)?;
 
     // We want to check that exactly the issue fee was sent
     check_issue_fee(&deps, &info)?;
@@ -141,7 +149,7 @@ pub fn instantiate(
         evidence_threshold: msg.evidence_threshold,
         used_ticket_sequence_threshold: msg.used_ticket_sequence_threshold,
         trust_set_limit_amount: msg.trust_set_limit_amount,
-        bridge_xrpl_address: msg.bridge_xrpl_address.clone(),
+        bridge_xrpl_address: msg.bridge_xrpl_address,
         bridge_state: BridgeState::Active,
         xrpl_base_fee: msg.xrpl_base_fee,
     };
@@ -179,12 +187,6 @@ pub fn instantiate(
     let key = build_xrpl_token_key(XRP_ISSUER, XRP_CURRENCY);
     XRPL_TOKENS.save(deps.storage, key, &token)?;
 
-    // We store all the prohibited recipients in state, including the multisig address, which is also prohibited to send to
-    for address in INITIAL_PROHIBITED_XRPL_RECIPIENTS {
-        PROHIBITED_XRPL_RECIPIENTS.save(deps.storage, address.to_string(), &Empty {})?;
-    }
-    PROHIBITED_XRPL_RECIPIENTS.save(deps.storage, msg.bridge_xrpl_address, &Empty {})?;
-
     Ok(Response::new()
         .add_attribute("action", ContractActions::Instantiation.as_str())
         .add_attribute("contract_name", CONTRACT_NAME)
@@ -328,12 +330,12 @@ pub fn execute(
             new_relayers,
             new_evidence_threshold,
         ),
-        ExecuteMsg::UpdateProhibitedXRPLRecipients {
-            prohibited_xrpl_recipients,
-        } => update_prohibited_xrpl_recipients(
+        ExecuteMsg::UpdateProhibitedXRPLAddresses {
+            prohibited_xrpl_addresses,
+        } => update_prohibited_xrpl_addresses(
             deps.into_empty(),
             info.sender,
-            prohibited_xrpl_recipients,
+            prohibited_xrpl_addresses,
         ),
         ExecuteMsg::CancelPendingOperation { operation_id } => {
             cancel_pending_operation(deps.into_empty(), info.sender, operation_id)
@@ -438,7 +440,7 @@ fn register_xrpl_token(
         &ContractActions::RegisterXRPLToken,
     )?;
 
-    validate_xrpl_address(&issuer)?;
+    validate_xrpl_address(deps.storage, issuer.clone())?;
     validate_xrpl_currency(&currency)?;
 
     validate_sending_precision(sending_precision, XRPL_TOKENS_DECIMALS)?;
@@ -451,11 +453,6 @@ fn register_xrpl_token(
         return Err(ContractError::XRPLTokenAlreadyRegistered { issuer, currency });
     }
 
-    // We check that the issuer is not prohibited
-    if PROHIBITED_XRPL_RECIPIENTS.has(deps.storage, issuer.clone()) {
-        return Err(ContractError::ProhibitedRecipient {});
-    }
-
     // We generate a denom creating a Sha256 hash of the issuer, currency and current time
     let to_hash = format!("{}{}{}", issuer, currency, env.block.time.seconds()).into_bytes();
 
@@ -569,7 +566,7 @@ fn save_evidence(
 
             // If the recipient of the operation is the bridge contract address, we error
             if recipient.eq(&env.contract.address) {
-                return Err(ContractError::ProhibitedRecipient {});
+                return Err(ContractError::ProhibitedAddress {});
             }
 
             // This means the token is not a Coreum originated token (the issuer is not the XRPL multisig address)
@@ -906,13 +903,8 @@ fn send_to_xrpl(
     // Check that we are only sending 1 type of coin
     let funds = one_coin(&info)?;
 
-    // Check that the recipient is a valid XRPL address
-    validate_xrpl_address(&recipient)?;
-
-    // We don't allow sending to a prohibited recipient
-    if PROHIBITED_XRPL_RECIPIENTS.has(deps.storage, recipient.clone()) {
-        return Err(ContractError::ProhibitedRecipient {});
-    }
+    // Check that the recipient is a valid XRPL address and it's not prohibited
+    validate_xrpl_address(deps.storage, recipient.clone())?;
 
     // We check that deliver_amount is not greater than the funds sent
     if deliver_amount.is_some() && deliver_amount.unwrap().gt(&funds.amount) {
@@ -1338,35 +1330,35 @@ fn rotate_keys(
         .add_attribute("sender", sender))
 }
 
-fn update_prohibited_xrpl_recipients(
+fn update_prohibited_xrpl_addresses(
     deps: DepsMut,
     sender: Addr,
-    prohibited_xrpl_recipients: Vec<String>,
+    prohibited_xrpl_addresses: Vec<String>,
 ) -> CoreumResult<ContractError> {
     check_authorization(
         deps.as_ref().storage,
         &sender,
-        &ContractActions::UpdateProhibitedXRPLRecipients,
+        &ContractActions::UpdateProhibitedXRPLAddresses,
     )?;
 
-    // We clear the previous prohibited recipients
-    PROHIBITED_XRPL_RECIPIENTS.clear(deps.storage);
+    // We clear the previous prohibited addresses
+    PROHIBITED_XRPL_ADDRESSES.clear(deps.storage);
 
     // We add the current multisig address which is always prohibited
     let config = CONFIG.load(deps.storage)?;
-    PROHIBITED_XRPL_RECIPIENTS.save(deps.storage, config.bridge_xrpl_address, &Empty {})?;
+    PROHIBITED_XRPL_ADDRESSES.save(deps.storage, config.bridge_xrpl_address, &Empty {})?;
 
-    // Add all prohibited recipients provided
-    for prohibited_xrpl_recipient in prohibited_xrpl_recipients {
+    // Add all prohibited addresses provided
+    for prohibited_xrpl_address in prohibited_xrpl_addresses {
         // Validate the address that we are adding, to not add useless things
-        validate_xrpl_address(&prohibited_xrpl_recipient)?;
-        PROHIBITED_XRPL_RECIPIENTS.save(deps.storage, prohibited_xrpl_recipient, &Empty {})?;
+        validate_xrpl_address_format(&prohibited_xrpl_address)?;
+        PROHIBITED_XRPL_ADDRESSES.save(deps.storage, prohibited_xrpl_address, &Empty {})?;
     }
 
     Ok(Response::new()
         .add_attribute(
             "action",
-            ContractActions::UpdateProhibitedXRPLRecipients.as_str(),
+            ContractActions::UpdateProhibitedXRPLAddresses.as_str(),
         )
         .add_attribute("sender", sender))
 }
@@ -1453,8 +1445,8 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
             start_after_key,
             limit,
         } => to_json_binary(&query_processed_txs(deps, start_after_key, limit)),
-        QueryMsg::ProhibitedXRPLRecipients {} => {
-            to_json_binary(&query_prohibited_xrpl_recipients(deps))
+        QueryMsg::ProhibitedXRPLAddresses {} => {
+            to_json_binary(&query_prohibited_xrpl_addresses(deps))
         }
     }
 }
@@ -1652,15 +1644,15 @@ fn query_processed_txs(
     }
 }
 
-fn query_prohibited_xrpl_recipients(deps: Deps) -> ProhibitedXRPLRecipientsResponse {
-    let prohibited_xrpl_recipients: Vec<String> = PROHIBITED_XRPL_RECIPIENTS
+fn query_prohibited_xrpl_addresses(deps: Deps) -> ProhibitedXRPLAddressesResponse {
+    let prohibited_xrpl_addresses: Vec<String> = PROHIBITED_XRPL_ADDRESSES
         .range(deps.storage, None, None, Order::Ascending)
         .filter_map(Result::ok)
         .map(|(addr, _)| addr)
         .collect();
 
-    ProhibitedXRPLRecipientsResponse {
-        prohibited_xrpl_recipients,
+    ProhibitedXRPLAddressesResponse {
+        prohibited_xrpl_addresses,
     }
 }
 

contract/src/error.rs

@@ -179,8 +179,8 @@ pub enum ContractError {
     )]
     OperationVersionMismatch {},
 
-    #[error("ProhibitedRecipient: The address is prohibited. It can't issue or receive tokens")]
-    ProhibitedRecipient {},
+    #[error("ProhibitedAddress: The address is prohibited")]
+    ProhibitedAddress {},
 
     #[error("DeliverAmountIsProhibited: Optional deliver_amount field is only used for XRPL originated tokens (except XRP) being bridged back")]
     DeliverAmountIsProhibited {},

contract/src/msg.rs

@@ -140,11 +140,11 @@ pub enum ExecuteMsg {
         new_relayers: Vec<Relayer>,
         new_evidence_threshold: u32,
     },
-    // Update the prohibited recipients list
+    // Update the prohibited addresses list
     // Only the owner can do this
-    #[serde(rename = "update_prohibited_xrpl_recipients")]
-    UpdateProhibitedXRPLRecipients {
-        prohibited_xrpl_recipients: Vec<String>,
+    #[serde(rename = "update_prohibited_xrpl_addresses")]
+    UpdateProhibitedXRPLAddresses {
+        prohibited_xrpl_addresses: Vec<String>,
     },
     // Cancels a pending operation, considering it as invalid
     // This will almost NEVER be used, unless there is some expected operation that causes an error on relayers
@@ -202,9 +202,9 @@ pub enum QueryMsg {
         start_after_key: Option<String>,
         limit: Option<u32>,
     },
-    #[returns(ProhibitedXRPLRecipientsResponse)]
-    #[serde(rename = "prohibited_xrpl_recipients")]
-    ProhibitedXRPLRecipients {},
+    #[returns(ProhibitedXRPLAddressesResponse)]
+    #[serde(rename = "prohibited_xrpl_addresses")]
+    ProhibitedXRPLAddresses {},
 }
 
 #[cw_serde]
@@ -272,6 +272,6 @@ pub struct ProcessedTxsResponse {
 }
 
 #[cw_serde]
-pub struct ProhibitedXRPLRecipientsResponse {
-    pub prohibited_xrpl_recipients: Vec<String>,
+pub struct ProhibitedXRPLAddressesResponse {
+    pub prohibited_xrpl_addresses: Vec<String>,
 }

contract/src/relayer.rs

@@ -38,7 +38,7 @@ pub fn validate_relayers(
 
     for relayer in relayers {
         deps.api.addr_validate(relayer.coreum_address.as_ref())?;
-        validate_xrpl_address(&relayer.xrpl_address)?;
+        validate_xrpl_address(deps.storage, relayer.xrpl_address.clone())?;
 
         // If the set returns false during insertion it means that the key already exists and therefore is duplicated
         if !set_xrpl_addresses.insert(relayer.xrpl_address.clone()) {

contract/src/state.rs

@@ -24,7 +24,7 @@ pub enum TopKey {
     FeesCollected = b'c',
     FeeRemainders = b'd',
     PendingRotateKeys = b'e',
-    ProhibitedXRPLRecipients = b'f',
+    ProhibitedXRPLAddresses = b'f',
 }
 
 impl TopKey {
@@ -195,9 +195,9 @@ pub const FEES_COLLECTED: Map<Addr, Vec<Coin>> = Map::new(TopKey::FeesCollected.
 // Fees Remainders in case that we have some small amounts left after dividing fees between our relayers we will keep them here until next time we collect fees and can add them to the new amount
 // Key is Coin denom and value is Coin amount
 pub const FEE_REMAINDERS: Map<String, Uint128> = Map::new(TopKey::FeeRemainders.as_str());
-// XRPL addresses that have been marked as prohibited and can't be used for receiving funds
-pub const PROHIBITED_XRPL_RECIPIENTS: Map<String, Empty> =
-    Map::new(TopKey::ProhibitedXRPLRecipients.as_str());
+// XRPL addresses that have been marked as prohibited and can't be used for receiving funds, issuing tokens, or multisigning transactions
+pub const PROHIBITED_XRPL_ADDRESSES: Map<String, Empty> =
+    Map::new(TopKey::ProhibitedXRPLAddresses.as_str());
 
 pub enum ContractActions {
     Instantiation,
@@ -212,7 +212,7 @@ pub enum ContractActions {
     UpdateXRPLToken,
     UpdateCoreumToken,
     UpdateXRPLBaseFee,
-    UpdateProhibitedXRPLRecipients,
+    UpdateProhibitedXRPLAddresses,
     ClaimRefunds,
     HaltBridge,
     ResumeBridge,
@@ -240,7 +240,7 @@ impl UserType {
             ContractActions::UpdateXRPLToken => matches!(self, Self::Owner),
             ContractActions::UpdateCoreumToken => matches!(self, Self::Owner),
             ContractActions::UpdateXRPLBaseFee => matches!(self, Self::Owner),
-            ContractActions::UpdateProhibitedXRPLRecipients => matches!(self, Self::Owner),
+            ContractActions::UpdateProhibitedXRPLAddresses => matches!(self, Self::Owner),
             ContractActions::ClaimRefunds => true,
             ContractActions::HaltBridge => matches!(self, Self::Owner | Self::Relayer),
             ContractActions::ResumeBridge => matches!(self, Self::Owner),
@@ -266,7 +266,7 @@ impl ContractActions {
             Self::UpdateXRPLToken => "update_xrpl_token",
             Self::UpdateCoreumToken => "update_coreum_token",
             Self::UpdateXRPLBaseFee => "update_xrpl_base_fee",
-            Self::UpdateProhibitedXRPLRecipients => "update_invalid_xrpl_recipients",
+            Self::UpdateProhibitedXRPLAddresses => "update_invalid_xrpl_addresses",
             Self::HaltBridge => "halt_bridge",
             Self::ResumeBridge => "resume_bridge",
             Self::RotateKeys => "rotate_keys",