Repository: CosmWasm/cosmwasm (rust)

Issue:
Bug in staking contract
With the Addr refactor in v0.14.0-beta2, we broke the staking contract.

When testing this in `wasmd` upgrade, I get: 

```
Error Trace:    staking_test.go:208
                                                        staking_test.go:550
                Error:          Received unexpected error:
                                
                                github.com/CosmWasm/wasmd/x/wasm/keeper.Keeper.instantiate
                                        /home/ethan/go/src/github.com/cosmwasm/wasmd/x/wasm/keeper/keeper.go:273
                                github.com/CosmWasm/wasmd/x/wasm/keeper.PermissionedKeeper.Instantiate
                                        /home/ethan/go/src/github.com/cosmwasm/wasmd/x/wasm/keeper/contract_keeper.go:43
                                github.com/CosmWasm/wasmd/x/wasm/keeper.initializeStaking
                                        /home/ethan/go/src/github.com/cosmwasm/wasmd/x/wasm/keeper/staking_test.go:207
                                github.com/CosmWasm/wasmd/x/wasm/keeper.TestQueryStakingPlugin
                                        /home/ethan/go/src/github.com/cosmwasm/wasmd/x/wasm/keeper/staking_test.go:550
                                Generic error: addr_validate errored: invalid Bech32 prefix; expected cosmos, got cosmosvaloper: instantiate wasm contract failed
```

The validator (valoper) addresses must always be unverified strings as we cannot validate them. 

We had this issue in wasmd some time ago: https://github.com/CosmWasm/wasmd/issues/264 and concluded that we cannot validate both AccAddress and ValOperAddress. Unless you want to extend the API (which I found unnecessary), let's keep validators as string. You can verify them in `init` simply by making a query using them.

Pull request:
Staking: Fix valoper address validation
Closes #871

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/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..18fd3eb49 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)?;
@@ -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/packages/std/src/mock.rs b/packages/std/src/mock.rs
index 836083f3c..740ce1b0e 100644
--- a/packages/std/src/mock.rs
+++ b/packages/std/src/mock.rs
@@ -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_ref() == 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),