Readme added for the Terra contracts, several security issues fixed in the contracts (#88)

terra/contracts/README.md

@@ -0,0 +1,47 @@
+# Terra Wormhole Contracts
+
+## Summary
+
+To facilitate token exchange via the Wormhole bridge blockchains must provide a set of smart contracts to process bridge commands on chain. Here are such contracts for the Terra blockchain.
+
+The first contract, `cw20-wrapped` is basically a `cw20-base` contract ([see here](https://github.com/CosmWasm/cosmwasm-plus/tree/master/contracts/cw20-base)) instantiated for every new token type issued on the blockchain by the bridge. And the second one, `wormhole` provides the bridge functionality itself:
+
+- It locks tokens on the Terra blockchain when they are sent out to other blockchains
+- It sends out wrapped or original tokens (depending on the blockchain origin of the token) to recipients when receiving tokens from the other blockchains
+
+## Details
+
+### `cw20-wrapped`
+
+This contract mostly wraps functionality of the `cw20-base` contract with the following differences:
+
+- It stores `WrappedAssetInfo` state with information about the source blockchain, asset address on this blockchain and the `wormhole` contract address
+- Once initialized it calls the hook action specified in the initialization params (`init_hook` field). It is used to record newly instantiated contract's address in the `wormhole` contract
+- Full mint authority is provided to the `wormhole` contract
+
+### `wormhole`
+
+This contract controls token transfers, minting and burning as well as maintaining the list of guardians: off-chain entities identified by their public keys, majority of whom can issue commands to the contract.
+
+`wormhole` bridge processes the following instructions.
+
+#### `SubmitVAA`
+
+Receives VAAs from the guardians (read about VAAs [here](../../docs/protocol.md)), verifies and processes them. In the current bridge implementation VAAs can trigger the following actions:
+
+- Send token to the Terra recipient
+- Update the list of guardians
+
+Sending tokens to the Terra recipient is handled by the `vaa_transfer` method. For the native Terra tokens it simply transfers the corresponding amount from its balance. For the non-native tokens `wormhole` either mints the corresponding amount from the already deployed `cw20-wrapped` contract or deploys a new one with the mint amount in the initialization message.
+
+#### `RegisterAssetHook`
+
+Gets called from the `cw20-wrapped` constructor to record its address in the contract's directory of wrapped assets. It is used later to check whether the wrapped contract for the asset is already deployed on Terra blockchain or not.
+
+#### `LockAssets`
+
+Called to initiate token transfer from the Terra blockchain to other blockchains. Caller must provide allowance to the `wormhole` contract to spend tokens, then the contract either transfers (if it is a native token) or burns it (if it is a wrapped token from the different blockchain). Then the information is logged to be read by the guardians operating the bridge, which triggers sending VAAs to the destination blockchain.
+
+#### `SetActive`
+
+Safety feature to turn off the `wormhole` contract in the case of any issues found in production. Only the owner can send this message, once the contract is inactive it stops processing token transfer commands.

terra/contracts/cw20-wrapped/src/contract.rs

@@ -1,5 +1,5 @@
 use cosmwasm_std::{
-    log, to_binary, Api, Binary, CosmosMsg, Env, Extern, HandleResponse, HumanAddr, InitResponse,
+    to_binary, Api, Binary, CosmosMsg, Env, Extern, HandleResponse, HumanAddr, InitResponse,
     Querier, StdError, StdResult, Storage, Uint128, WasmMsg,
 };
 
@@ -10,7 +10,7 @@ use cw20_base::allowances::{
 use cw20_base::contract::{
     handle_mint, handle_send, handle_transfer, query_balance, query_token_info,
 };
-use cw20_base::state::{balances, token_info, MinterData, TokenInfo};
+use cw20_base::state::{token_info, MinterData, TokenInfo};
 
 use crate::msg::{HandleMsg, InitMsg, QueryMsg, WrappedAssetInfoResponse};
 use crate::state::{wrapped_asset_info, wrapped_asset_info_read, WrappedAssetInfo};
@@ -71,7 +71,7 @@ pub fn handle<S: Storage, A: Api, Q: Querier>(
         HandleMsg::Transfer { recipient, amount } => {
             Ok(handle_transfer(deps, env, recipient, amount)?)
         }
-        HandleMsg::Burn { account, amount } => Ok(handle_burn_wrapped(deps, env, account, amount)?),
+        HandleMsg::Burn { account, amount } => Ok(handle_burn_from(deps, env, account, amount)?),
         HandleMsg::Send {
             contract,
             amount,
@@ -107,48 +107,6 @@ pub fn handle<S: Storage, A: Api, Q: Querier>(
     }
 }
 
-fn handle_burn_wrapped<S: Storage, A: Api, Q: Querier>(
-    deps: &mut Extern<S, A, Q>,
-    env: Env,
-    account: HumanAddr,
-    amount: Uint128,
-) -> StdResult<HandleResponse> {
-    // Only bridge can burn
-    let wrapped_info = wrapped_asset_info_read(&deps.storage).load()?;
-    if wrapped_info.bridge != deps.api.canonical_address(&env.message.sender)? {
-        return Err(StdError::unauthorized());
-    }
-
-    // Copy of CW20-base handle_burn, but burning from account sent with parameters, not from sender
-    if amount == Uint128::zero() {
-        return Err(StdError::generic_err("Invalid zero amount"));
-    }
-
-    let burn_from_raw = deps.api.canonical_address(&account)?;
-
-    // lower balance
-    let mut accounts = balances(&mut deps.storage);
-    accounts.update(burn_from_raw.as_slice(), |balance: Option<Uint128>| {
-        balance.unwrap_or_default() - amount
-    })?;
-    // reduce total_supply
-    token_info(&mut deps.storage).update(|mut info| {
-        info.total_supply = (info.total_supply - amount)?;
-        Ok(info)
-    })?;
-
-    let res = HandleResponse {
-        messages: vec![],
-        log: vec![
-            log("action", "burn"),
-            log("from", account),
-            log("amount", amount),
-        ],
-        data: None,
-    };
-    Ok(res)
-}
-
 fn handle_mint_wrapped<S: Storage, A: Api, Q: Querier>(
     deps: &mut Extern<S, A, Q>,
     env: Env,
@@ -299,58 +257,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn can_burn_by_minter() {
-        let mut deps = mock_dependencies(CANONICAL_LENGTH, &[]);
-        let minter = HumanAddr::from("minter");
-        let recipient = HumanAddr::from("recipient");
-        let amount = Uint128(222_222_222);
-        do_init_and_mint(&mut deps, &minter, &recipient, amount);
-
-        let amount_to_burn = Uint128(222_222_221);
-        let msg = HandleMsg::Burn {
-            account: recipient.clone(),
-            amount: amount_to_burn,
-        };
-
-        let env = mock_env(&minter, &[]);
-        let res = handle(&mut deps, env, msg.clone()).unwrap();
-        assert_eq!(0, res.messages.len());
-        assert_eq!(get_balance(&deps, recipient), Uint128(1));
-
-        assert_eq!(
-            query_token_info(&deps).unwrap(),
-            TokenInfoResponse {
-                name: "Wormhole Wrapped".to_string(),
-                symbol: "WWT".to_string(),
-                decimals: 10,
-                total_supply: Uint128(1),
-            }
-        );
-    }
-
-    #[test]
-    fn others_cannot_burn() {
-        let mut deps = mock_dependencies(CANONICAL_LENGTH, &[]);
-        let minter = HumanAddr::from("minter");
-        let recipient = HumanAddr::from("recipient");
-        let amount = Uint128(222_222_222);
-        do_init_and_mint(&mut deps, &minter, &recipient, amount);
-
-        let amount_to_burn = Uint128(222_222_221);
-        let msg = HandleMsg::Burn {
-            account: recipient.clone(),
-            amount: amount_to_burn,
-        };
-
-        let env = mock_env(&recipient, &[]);
-        let res = handle(&mut deps, env, msg.clone());
-        assert_eq!(
-            format!("{}", res.unwrap_err()),
-            format!("{}", crate::error::ContractError::Unauthorized {})
-        );
-    }
-
     #[test]
     fn transfer_balance_success() {
         let mut deps = mock_dependencies(CANONICAL_LENGTH, &[]);

terra/contracts/cw20-wrapped/tests/integration.rs

@@ -236,73 +236,4 @@ fn transfer_works() {
         &TestAddress::RECIPIENT.value(),
         &Uint128(123_123_000),
     );
-}
-
-#[test]
-fn burn_works() {
-    let mut deps = do_init(111);
-
-    do_mint(
-        &mut deps,
-        112,
-        &TestAddress::RECIPIENT.value(),
-        &Uint128(123_123_123),
-    );
-
-    let burn_msg = HandleMsg::Burn {
-        account: TestAddress::RECIPIENT.value(),
-        amount: Uint128(123_123_000),
-    };
-    let env = mock_env_height(&TestAddress::INITIALIZER.value(), 113, 0);
-    let handle_response: HandleResponse = handle(&mut deps, env, burn_msg).unwrap();
-    assert_eq!(0, handle_response.messages.len());
-
-    check_balance(&mut deps, &TestAddress::RECIPIENT.value(), &Uint128(123));
-    check_token_details(&mut deps, &Uint128(123));
-}
-
-#[test]
-fn others_cannot_burn() {
-    let mut deps = do_init(111);
-
-    do_mint(
-        &mut deps,
-        112,
-        &TestAddress::RECIPIENT.value(),
-        &Uint128(123_123_123),
-    );
-
-    let burn_msg = HandleMsg::Burn {
-        account: TestAddress::RECIPIENT.value(),
-        amount: Uint128(123_123_000),
-    };
-    let env = mock_env_height(&TestAddress::RECIPIENT.value(), 113, 0);
-    let handle_result: HandleResult<HandleResponse> = handle(&mut deps, env, burn_msg);
-    assert_eq!(
-        format!("{}", handle_result.unwrap_err()),
-        format!("{}", ContractError::Unauthorized {})
-    );
-}
-
-#[test]
-fn cannot_burn_more_than_available() {
-    let mut deps = do_init(111);
-
-    do_mint(
-        &mut deps,
-        112,
-        &TestAddress::RECIPIENT.value(),
-        &Uint128(123_123_123),
-    );
-
-    let burn_msg = HandleMsg::Burn {
-        account: TestAddress::RECIPIENT.value(),
-        amount: Uint128(123_123_124),
-    };
-    let env = mock_env_height(&TestAddress::INITIALIZER.value(), 113, 0);
-    let handle_result: HandleResult<HandleResponse> = handle(&mut deps, env, burn_msg);
-    assert_eq!(
-        format!("{}", handle_result.unwrap_err()),
-        "Cannot subtract 123123124 from 123123123"
-    );
-}
+}

terra/contracts/wormhole/src/contract.rs

@@ -77,25 +77,6 @@ pub fn handle<S: Storage, A: Api, Q: Querier>(
             target_chain,
             nonce,
         } => handle_lock_assets(deps, env, asset, amount, recipient, target_chain, nonce),
-        HandleMsg::TokensLocked {
-            target_chain,
-            token_chain,
-            token_decimals,
-            token,
-            sender,
-            recipient,
-            amount,
-            nonce,
-        } => handle_tokens_locked(
-            target_chain,
-            token_chain,
-            token_decimals,
-            token,
-            sender,
-            recipient,
-            amount,
-            nonce,
-        ),
         HandleMsg::SetActive { is_active } => handle_set_active(deps, env, is_active),
     }
 }
@@ -486,47 +467,14 @@ fn handle_lock_assets<S: Storage, A: Api, Q: Querier>(
         }
     };
 
-    messages.push(CosmosMsg::Wasm(WasmMsg::Execute {
-        contract_addr: env.contract.address,
-        msg: to_binary(&HandleMsg::TokensLocked {
-            target_chain,
-            token_chain: asset_chain,
-            token_decimals: decimals,
-            token: asset_address,
-            sender: extend_address_to_32(&deps.api.canonical_address(&env.message.sender)?),
-            recipient,
-            amount,
-            nonce,
-        })?,
-        send: vec![],
-    }));
-
     Ok(HandleResponse {
         messages,
-        log: vec![],
-        data: None,
-    })
-}
-
-fn handle_tokens_locked(
-    target_chain: u8,
-    token_chain: u8,
-    token_decimals: u8,
-    token: Vec<u8>,
-    sender: Vec<u8>,
-    recipient: Vec<u8>,
-    amount: Uint128,
-    nonce: u32,
-) -> StdResult<HandleResponse> {
-    // Dummy handler to record token lock as transaction
-    Ok(HandleResponse {
-        messages: vec![],
         log: vec![
             log("locked.target_chain", target_chain),
-            log("locked.token_chain", token_chain),
-            log("locked.token_decimals", token_decimals),
-            log("locked.token", hex::encode(token)),
-            log("locked.sender", hex::encode(sender)),
+            log("locked.token_chain", asset_chain),
+            log("locked.token_decimals", decimals),
+            log("locked.token", hex::encode(asset_address)),
+            log("locked.sender", hex::encode(extend_address_to_32(&deps.api.canonical_address(&env.message.sender)?))),
             log("locked.recipient", hex::encode(recipient)),
             log("locked.amount", amount),
             log("locked.nonce", nonce),
@@ -612,8 +560,7 @@ mod tests {
 
     const ADDR_1: &str = "beFA429d57cD18b7F8A4d91A2da9AB4AF05d0FBe";
     const ADDR_2: &str = "8575Df9b3c97B4E267Deb92d93137844A97A0132";
-
-    const VAA_VALID_TRANSFER: &str = "010000000001005468beb21caff68710b2af2d60a986245bf85099509b6babe990a6c32456b44b3e2e9493e3056b7d5892957e14beab24be02dab77ed6c8915000e4a1267f78f400000007d01000000038018002010400000000000000000000000000000000000000000000000000000000000101010101010101010101010101010101010101000000000000000000000000010000000000000000000000000347ef34687bdc9f189e87a9200658d9c40e9988080000000000000000000000000000000000000000000000000de0b6b3a7640000";
+    const VAA_VALID_TRANSFER: &str = "010000000001001063f503dd308134e0f158537f54c5799719f4fa2687dd276c72ef60ae0c82c47d4fb560545afaabdf60c15918e221763fd1892c75f2098c0ffd5db4af254a4501000007d01000000038010302010400000000000000000000000000000000000000000000000000000000000101010101010101010101010101010101010101000000000000000000000000010000000000000000000000000347ef34687bdc9f189e87a9200658d9c40e9988080000000000000000000000000000000000000000000000000de0b6b3a7640000";
     const VAA_VALID_GUARDIAN_SET_CHANGE: &str = "01000000000100d90d6f9cbc0458599cbe4d267bc9221b54955b94cb5cb338aeb845bdc9dd275f558871ea479de9cc0b44cfb2a07344431a3adbd2f98aa86f4e12ff4aba061b7f00000007d00100000001018575df9b3c97b4e267deb92d93137844a97a0132";
 
     const CANONICAL_LENGTH: usize = 20;

terra/contracts/wormhole/src/msg.rs

@@ -27,16 +27,6 @@ pub enum HandleMsg {
         target_chain: u8,
         nonce: u32,
     },
-    TokensLocked {
-        target_chain: u8,
-        token_chain: u8,
-        token_decimals: u8,
-        token: Vec<u8>,
-        sender: Vec<u8>,
-        recipient: Vec<u8>,
-        amount: Uint128,
-        nonce: u32,
-    },
     SetActive {
         is_active: bool,
     },