Repository: CosmWasm/cosmwasm
Pull request #874 (2021-04-12 09:30:06)

Add Addr::as_str
Builds on #873

I would really like to avoid adding an `as_bytes` because an `Addr` is a string and in the context of addresses, there are multiple binary representations competing. I think it is fair to require explicit byte conversion via string: `.as_str().as_bytes()`.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d0efa661a..7387b35aa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -54,8 +54,6 @@ and this project adheres to
   as well as `From<u{32,16,8}> for Uint128`.
 - cosmwasm-std: Create new address type `Addr`. This is human readable (like
   `HumanAddr`) but is immutable and always contains a valid address ([#802]).
-- cosmwasm-std: Replace `HumanAddr` with `String` in `BankQuery`, `StakingQuery`
-  and `WasmQuery` query requests.
 - cosmwasm-vm: Add import `addr_validate` ([#802]).
 - cosmwasm-std: Add `BankMsg::Burn` variant when you want the tokens to
   disappear ([#860])
@@ -148,6 +146,12 @@ and this project adheres to
       library. Please use the explicit `*_sub` methods introduced above. In a
       couple of releases from now, we want to introduce the operator again with
       panicking overflow behaviour ([#858]).
+- cosmwasm-std: Replace `HumanAddr` with `String` in `BankQuery`, `StakingQuery`
+  and `WasmQuery` query requests ([#802]).
+- cosmwasm-std: In staking query response types `Delegation`, `FullDelegation`
+  and `Validator` the validator address fields were changed from `HumanAddr` to
+  `String`. The new `Addr` type cannot be used here because it only supports
+  standard account addresses via `Api::addr_*` ([#871]).
 - cosmwasm-std: Change address types in `BankMsg`, `IbcMsg` and `WasmMsg` from
   `HumanAddr` to `String` ([#802]).
 - cosmwasm-std: `Api::addr_humanize` now returns `Addr` instead of `HumanAddr`
@@ -162,6 +166,7 @@ and this project adheres to
 [#858]: https://github.com/CosmWasm/cosmwasm/issues/858
 [u128]: https://doc.rust-lang.org/std/primitive.u128.html
 [#802]: https://github.com/CosmWasm/cosmwasm/pull/802
+[#871]: https://github.com/CosmWasm/cosmwasm/issues/871
 
 ### Deprecated
 
diff --git a/contracts/hackatom/src/contract.rs b/contracts/hackatom/src/contract.rs
index 49b5a2bc8..e4421b58e 100644
--- a/contracts/hackatom/src/contract.rs
+++ b/contracts/hackatom/src/contract.rs
@@ -254,7 +254,7 @@ fn query_other_balance(deps: Deps, address: String) -> StdResult<AllBalanceRespo
 
 fn query_recurse(deps: Deps, depth: u32, work: u32, contract: Addr) -> StdResult<RecurseResponse> {
     // perform all hashes as requested
-    let mut hashed: Vec<u8> = contract.as_ref().into();
+    let mut hashed: Vec<u8> = contract.as_str().into();
     for _ in 0..work {
         hashed = Sha256::digest(&hashed).to_vec()
     }
diff --git a/contracts/staking/schema/investment_info.json b/contracts/staking/schema/investment_info.json
index 9ea300a35..1716f35b8 100644
--- a/contracts/staking/schema/investment_info.json
+++ b/contracts/staking/schema/investment_info.json
@@ -40,7 +40,7 @@
       ]
     },
     "validator": {
-      "description": "All tokens are bonded to this validator FIXME: humanize/canonicalize address doesn't work for validator addrresses",
+      "description": "All tokens are bonded to this validator addr_humanize/addr_canonicalize doesn't work for validator addrresses (e.g. cosmosvaloper1...)",
       "type": "string"
     }
   },
diff --git a/contracts/staking/src/contract.rs b/contracts/staking/src/contract.rs
index d087a80cc..657fbfb39 100644
--- a/contracts/staking/src/contract.rs
+++ b/contracts/staking/src/contract.rs
@@ -43,7 +43,7 @@ pub fn instantiate(
         owner: info.sender,
         exit_tax: msg.exit_tax,
         bond_denom: denom,
-        validator: deps.api.addr_validate(&msg.validator)?.into(),
+        validator: msg.validator,
         min_withdrawal: msg.min_withdrawal,
     };
     invest_info(deps.storage).save(&invest)?;
@@ -82,7 +82,7 @@ pub fn transfer(
     send: Uint128,
 ) -> StdResult<Response> {
     let rcpt_raw = deps.api.addr_canonicalize(&recipient)?;
-    let sender_raw = deps.api.addr_canonicalize(info.sender.as_ref())?;
+    let sender_raw = deps.api.addr_canonicalize(info.sender.as_str())?;
 
     let mut accounts = balances(deps.storage);
     accounts.update(&sender_raw, |balance: Option<Uint128>| -> StdResult<_> {
@@ -139,7 +139,7 @@ fn assert_bonds(supply: &Supply, bonded: Uint128) -> StdResult<()> {
 }
 
 pub fn bond(deps: DepsMut, env: Env, info: MessageInfo) -> StdResult<Response> {
-    let sender_raw = deps.api.addr_canonicalize(info.sender.as_ref())?;
+    let sender_raw = deps.api.addr_canonicalize(info.sender.as_str())?;
 
     // ensure we have the proper denom
     let invest = invest_info_read(deps.storage).load()?;
@@ -201,8 +201,8 @@ pub fn unbond(deps: DepsMut, env: Env, info: MessageInfo, amount: Uint128) -> St
         )));
     }
 
-    let sender_raw = deps.api.addr_canonicalize(info.sender.as_ref())?;
-    let owner_raw = deps.api.addr_canonicalize(invest.owner.as_ref())?;
+    let sender_raw = deps.api.addr_canonicalize(info.sender.as_str())?;
+    let owner_raw = deps.api.addr_canonicalize(invest.owner.as_str())?;
 
     // calculate tax and remainer to unbond
     let tax = amount * invest.exit_tax;
@@ -272,7 +272,7 @@ pub fn claim(deps: DepsMut, env: Env, info: MessageInfo) -> StdResult<Response>
     }
 
     // check how much to send - min(balance, claims[sender]), and reduce the claim
-    let sender_raw = deps.api.addr_canonicalize(info.sender.as_ref())?;
+    let sender_raw = deps.api.addr_canonicalize(info.sender.as_str())?;
     let mut to_send = balance.amount;
     claims(deps.storage).update(sender_raw.as_slice(), |claim| -> StdResult<_> {
         let claim = claim.ok_or_else(|| StdError::generic_err("no claim for this address"))?;
@@ -451,7 +451,7 @@ mod tests {
 
     fn sample_validator(addr: &str) -> Validator {
         Validator {
-            address: Addr::unchecked(addr),
+            address: addr.to_owned(),
             commission: Decimal::percent(3),
             max_commission: Decimal::percent(10),
             max_change_rate: Decimal::percent(1),
@@ -461,7 +461,7 @@ mod tests {
     fn sample_delegation(validator_addr: &str, amount: Coin) -> FullDelegation {
         let can_redelegate = amount.clone();
         FullDelegation {
-            validator: Addr::unchecked(validator_addr),
+            validator: validator_addr.to_owned(),
             delegator: Addr::unchecked(MOCK_CONTRACT_ADDR),
             amount,
             can_redelegate,
diff --git a/contracts/staking/src/state.rs b/contracts/staking/src/state.rs
index f59c2ccea..665b6125f 100644
--- a/contracts/staking/src/state.rs
+++ b/contracts/staking/src/state.rs
@@ -42,7 +42,7 @@ pub struct InvestmentInfo {
     /// this is how much the owner takes as a cut when someone unbonds
     pub exit_tax: Decimal,
     /// All tokens are bonded to this validator
-    /// FIXME: humanize/canonicalize address doesn't work for validator addrresses
+    /// addr_humanize/addr_canonicalize doesn't work for validator addrresses (e.g. cosmosvaloper1...)
     pub validator: String,
     /// This is the minimum amount we will pull out to reinvest, as well as a minumum
     /// that can be unbonded (to avoid needless staking tx)
diff --git a/devtools/check_contracts_full.sh b/devtools/check_contracts_full.sh
index 87e7ec9d1..7ab64ebc8 100755
--- a/devtools/check_contracts_full.sh
+++ b/devtools/check_contracts_full.sh
@@ -10,6 +10,7 @@ for contract_dir in contracts/*/; do
     touch target/wasm32-unknown-unknown/release/"$(basename "$contract_dir" | tr - _)".wasm
     cargo check --tests
     cargo unit-test
+    touch src/*.rs # Needed because check and clippy share the same build cache such that clippy warnings are hidden when check wrote to the build cache
     cargo clippy --tests -- -D warnings
     cargo schema
     cargo wasm
diff --git a/devtools/check_contracts_medium.sh b/devtools/check_contracts_medium.sh
index 7f4b84e12..98da5b0c6 100755
--- a/devtools/check_contracts_medium.sh
+++ b/devtools/check_contracts_medium.sh
@@ -10,6 +10,7 @@ for contract_dir in contracts/*/; do
     touch target/wasm32-unknown-unknown/release/"$(basename "$contract_dir" | tr - _)".wasm
     cargo check --tests
     cargo unit-test
+    touch src/*.rs # Needed because check and clippy share the same build cache such that clippy warnings are hidden when check wrote to the build cache
     cargo clippy --tests -- -D warnings
     cargo schema
     cargo wasm-debug
diff --git a/packages/std/src/addresses.rs b/packages/std/src/addresses.rs
index 953866db3..c076d6806 100644
--- a/packages/std/src/addresses.rs
+++ b/packages/std/src/addresses.rs
@@ -45,6 +45,11 @@ impl Addr {
     pub fn unchecked<T: Into<String>>(input: T) -> Addr {
         Addr(input.into())
     }
+
+    #[inline]
+    pub fn as_str(&self) -> &str {
+        self.0.as_str()
+    }
 }
 
 impl fmt::Display for Addr {
@@ -56,7 +61,7 @@ impl fmt::Display for Addr {
 impl AsRef<str> for Addr {
     #[inline]
     fn as_ref(&self) -> &str {
-        &self.0
+        self.as_str()
     }
 }
 
@@ -274,6 +279,12 @@ mod tests {
         assert_ne!(a, b);
     }
 
+    #[test]
+    fn addr_as_str_works() {
+        let addr = Addr::unchecked("literal-string");
+        assert_eq!(addr.as_str(), "literal-string");
+    }
+
     #[test]
     fn addr_implements_display() {
         let addr = Addr::unchecked("cos934gh9034hg04g0h134");
diff --git a/packages/std/src/mock.rs b/packages/std/src/mock.rs
index 836083f3c..e61fdd6e4 100644
--- a/packages/std/src/mock.rs
+++ b/packages/std/src/mock.rs
@@ -461,7 +461,7 @@ impl StakingQuerier {
                 let delegations: Vec<_> = self
                     .delegations
                     .iter()
-                    .filter(|d| d.delegator.as_ref() == delegator)
+                    .filter(|d| d.delegator.as_str() == delegator)
                     .cloned()
                     .map(|d| d.into())
                     .collect();
@@ -472,9 +472,10 @@ impl StakingQuerier {
                 delegator,
                 validator,
             } => {
-                let delegation = self.delegations.iter().find(|d| {
-                    d.delegator.as_ref() == delegator && d.validator.as_ref() == validator
-                });
+                let delegation = self
+                    .delegations
+                    .iter()
+                    .find(|d| d.delegator.as_str() == delegator && d.validator == *validator);
                 let res = DelegationResponse {
                     delegation: delegation.cloned(),
                 };
@@ -865,13 +866,13 @@ mod tests {
     #[test]
     fn staking_querier_validators() {
         let val1 = Validator {
-            address: Addr::unchecked("validator-one"),
+            address: String::from("validator-one"),
             commission: Decimal::percent(1),
             max_commission: Decimal::percent(3),
             max_change_rate: Decimal::percent(1),
         };
         let val2 = Validator {
-            address: Addr::unchecked("validator-two"),
+            address: String::from("validator-two"),
             commission: Decimal::permille(15),
             max_commission: Decimal::permille(40),
             max_change_rate: Decimal::permille(5),
@@ -922,8 +923,8 @@ mod tests {
 
     #[test]
     fn staking_querier_delegations() {
-        let val1 = Addr::unchecked("validator-one");
-        let val2 = Addr::unchecked("validator-two");
+        let val1 = String::from("validator-one");
+        let val2 = String::from("validator-two");
 
         let user_a = Addr::unchecked("investor");
         let user_b = Addr::unchecked("speculator");
diff --git a/packages/std/src/query.rs b/packages/std/src/query.rs
index 7491f8288..149d6bdad 100644
--- a/packages/std/src/query.rs
+++ b/packages/std/src/query.rs
@@ -181,7 +181,8 @@ pub struct AllDelegationsResponse {
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 pub struct Delegation {
     pub delegator: Addr,
-    pub validator: Addr,
+    /// A validator address (e.g. cosmosvaloper1...)
+    pub validator: String,
     /// How much we have locked in the delegation
     pub amount: Coin,
 }
@@ -210,7 +211,8 @@ pub struct DelegationResponse {
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 pub struct FullDelegation {
     pub delegator: Addr,
-    pub validator: Addr,
+    /// A validator address (e.g. cosmosvaloper1...)
+    pub validator: String,
     /// How much we have locked in the delegation
     pub amount: Coin,
     /// can_redelegate captures how much can be immediately redelegated.
@@ -230,7 +232,8 @@ pub struct ValidatorsResponse {
 /// Instances are created in the querier.
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 pub struct Validator {
-    pub address: Addr,
+    /// A validator address (e.g. cosmosvaloper1...)
+    pub address: String,
     pub commission: Decimal,
     pub max_commission: Decimal,
     /// TODO: what units are these (in terms of time)?


Test patch:
diff --git a/contracts/staking/tests/integration.rs b/contracts/staking/tests/integration.rs
index 0d5fc5c33..f908d62e1 100644
--- a/contracts/staking/tests/integration.rs
+++ b/contracts/staking/tests/integration.rs
@@ -17,9 +17,7 @@
 //!      });
 //! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...)
 
-use cosmwasm_std::{
-    coin, from_binary, Addr, ContractResult, Decimal, Response, Uint128, Validator,
-};
+use cosmwasm_std::{coin, from_binary, ContractResult, Decimal, Response, Uint128, Validator};
 use cosmwasm_vm::testing::{
     instantiate, mock_backend, mock_env, mock_info, mock_instance_options, query,
 };
@@ -37,7 +35,7 @@ static WASM: &[u8] = include_bytes!("../target/wasm32-unknown-unknown/release/st
 
 fn sample_validator(addr: &str) -> Validator {
     Validator {
-        address: Addr::unchecked(addr),
+        address: addr.to_owned(),
         commission: Decimal::percent(3),
         max_commission: Decimal::percent(10),
         max_change_rate: Decimal::percent(1),