MP-2840. Remove deposit 'on behalf of' [audit] (#230)

contracts/red-bank/src/contract.rs

@@ -41,11 +41,9 @@ pub fn execute(
             let user_addr = deps.api.addr_validate(&user)?;
             execute::update_uncollateralized_loan_limit(deps, info, user_addr, denom, new_limit)
         }
-        ExecuteMsg::Deposit {
-            on_behalf_of,
-        } => {
+        ExecuteMsg::Deposit {} => {
             let sent_coin = cw_utils::one_coin(&info)?;
-            execute::deposit(deps, env, info, on_behalf_of, sent_coin.denom, sent_coin.amount)
+            execute::deposit(deps, env, info, sent_coin.denom, sent_coin.amount)
         }
         ExecuteMsg::Withdraw {
             denom,

contracts/red-bank/src/execute.rs

@@ -277,18 +277,9 @@ pub fn deposit(
     deps: DepsMut,
     env: Env,
     info: MessageInfo,
-    on_behalf_of: Option<String>,
     denom: String,
     deposit_amount: Uint128,
 ) -> Result<Response, ContractError> {
-    let user_addr: Addr;
-    let user = if let Some(address) = on_behalf_of {
-        user_addr = deps.api.addr_validate(&address)?;
-        User(&user_addr)
-    } else {
-        User(&info.sender)
-    };
-
     let mut market = MARKETS.load(deps.storage, &denom)?;
 
     let config = CONFIG.load(deps.storage)?;
@@ -341,7 +332,7 @@ pub fn deposit(
     let deposit_amount_scaled =
         get_scaled_liquidity_amount(deposit_amount, &market, env.block.time.seconds())?;
 
-    response = user.increase_collateral(
+    response = User(&info.sender).increase_collateral(
         deps.storage,
         &market,
         deposit_amount_scaled,
@@ -358,7 +349,6 @@ pub fn deposit(
     Ok(response
         .add_attribute("action", "deposit")
         .add_attribute("sender", &info.sender)
-        .add_attribute("on_behalf_of", user)
         .add_attribute("denom", denom)
         .add_attribute("amount", deposit_amount)
         .add_attribute("amount_scaled", deposit_amount_scaled))

contracts/red-bank/tests/test_deposit.rs

@@ -96,9 +96,7 @@ fn depositing_with_no_coin_sent() {
         deps.as_mut(),
         mock_env(),
         mock_info(depositor_addr.as_str(), &[]),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap_err();
     assert_eq!(err, PaymentError::NoFunds {}.into());
@@ -118,9 +116,7 @@ fn depositing_with_multiple_coins_sent() {
         deps.as_mut(),
         mock_env(),
         mock_info(depositor_addr.as_str(), &sent_coins),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap_err();
     assert_eq!(err, PaymentError::MultipleDenoms {}.into());
@@ -141,9 +137,7 @@ fn depositing_to_non_existent_market() {
         deps.as_mut(),
         mock_env(),
         mock_info(depositor_addr.as_str(), &coins(123, false_denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap_err();
     assert_eq!(err, StdError::not_found(type_name::<Market>()).into());
@@ -182,9 +176,7 @@ fn depositing_to_disabled_market() {
         deps.as_mut(),
         mock_env(),
         mock_info(depositor_addr.as_str(), &coins(123, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap_err();
     assert_eq!(
@@ -236,9 +228,7 @@ fn depositing_above_cap() {
         deps.as_mut(),
         mock_env_at_block_time(10000100),
         mock_info(depositor_addr.as_str(), &coins(1_000_001, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap_err();
     assert_eq!(
@@ -253,9 +243,7 @@ fn depositing_above_cap() {
         deps.as_mut(),
         mock_env_at_block_time(10000100),
         mock_info(depositor_addr.as_str(), &coins(123, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     );
     assert!(result.is_ok());
 }
@@ -286,9 +274,7 @@ fn depositing_without_existing_position() {
         deps.as_mut(),
         mock_env_at_block_time(block_time),
         mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap();
 
@@ -315,7 +301,6 @@ fn depositing_without_existing_position() {
         vec![
             attr("action", "deposit"),
             attr("sender", &depositor_addr),
-            attr("on_behalf_of", &depositor_addr),
             attr("denom", denom),
             attr("amount", deposit_amount.to_string()),
             attr("amount_scaled", expected_mint_amount),
@@ -376,9 +361,7 @@ fn depositing_with_existing_position() {
         deps.as_mut(),
         mock_env_at_block_time(block_time),
         mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap();
 
@@ -414,93 +397,6 @@ fn depositing_with_existing_position() {
     );
 }
 
-#[test]
-fn depositing_on_behalf_of() {
-    let TestSuite {
-        mut deps,
-        denom,
-        depositor_addr,
-        initial_market,
-    } = setup_test();
-
-    let deposit_amount = 123456u128;
-    let on_behalf_of_addr = Addr::unchecked("jake");
-
-    // compute expected market parameters
-    let block_time = 10000300;
-    let expected_params =
-        th_get_expected_indices_and_rates(&initial_market, block_time, Default::default());
-    let expected_mint_amount = compute_scaled_amount(
-        Uint128::from(deposit_amount),
-        expected_params.liquidity_index,
-        ScalingOperation::Truncate,
-    )
-    .unwrap();
-    let expected_reward_amount_scaled = compute_scaled_amount(
-        expected_params.protocol_rewards_to_distribute,
-        expected_params.liquidity_index,
-        ScalingOperation::Truncate,
-    )
-    .unwrap();
-
-    let res = execute(
-        deps.as_mut(),
-        mock_env_at_block_time(block_time),
-        mock_info(depositor_addr.as_str(), &coins(deposit_amount, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: Some(on_behalf_of_addr.clone().into()),
-        },
-    )
-    .unwrap();
-
-    // NOTE: For this test, the accrued protocol reward is non-zero, so we do expect a message to
-    // update the index of the rewards collector.
-    assert_eq!(
-        res.messages,
-        vec![
-            SubMsg::new(WasmMsg::Execute {
-                contract_addr: MarsAddressType::Incentives.to_string(),
-                msg: to_binary(&incentives::ExecuteMsg::BalanceChange {
-                    user_addr: Addr::unchecked(MarsAddressType::RewardsCollector.to_string()),
-                    denom: initial_market.denom.clone(),
-                    user_amount_scaled_before: Uint128::zero(),
-                    total_amount_scaled_before: initial_market.collateral_total_scaled,
-                })
-                .unwrap(),
-                funds: vec![]
-            }),
-            SubMsg::new(WasmMsg::Execute {
-                contract_addr: MarsAddressType::Incentives.to_string(),
-                msg: to_binary(&incentives::ExecuteMsg::BalanceChange {
-                    user_addr: on_behalf_of_addr.clone(),
-                    denom: initial_market.denom.clone(),
-                    user_amount_scaled_before: Uint128::zero(),
-                    // NOTE: New collateral shares were minted to the rewards collector first, so
-                    // for the depositor this should be initial total supply + rewards shares minted
-                    total_amount_scaled_before: initial_market.collateral_total_scaled
-                        + expected_reward_amount_scaled,
-                })
-                .unwrap(),
-                funds: vec![]
-            })
-        ]
-    );
-
-    // depositor should not have created a new collateral position
-    let opt = COLLATERALS.may_load(deps.as_ref().storage, (&depositor_addr, denom)).unwrap();
-    assert!(opt.is_none());
-
-    // the recipient should have created a new collateral position
-    let collateral = COLLATERALS.load(deps.as_ref().storage, (&on_behalf_of_addr, denom)).unwrap();
-    assert_eq!(
-        collateral,
-        Collateral {
-            amount_scaled: expected_mint_amount,
-            enabled: true,
-        }
-    );
-}
-
 #[test]
 fn depositing_on_behalf_of_cannot_enable_collateral() {
     let TestSuite {
@@ -521,9 +417,7 @@ fn depositing_on_behalf_of_cannot_enable_collateral() {
         deps.as_mut(),
         mock_env_at_block_time(block_time),
         mock_info(on_behalf_of_addr.as_str(), &coins(1u128, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: None,
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap();
 
@@ -552,9 +446,7 @@ fn depositing_on_behalf_of_cannot_enable_collateral() {
         deps.as_mut(),
         mock_env_at_block_time(block_time),
         mock_info(depositor_addr.as_str(), &coins(1u128, denom)),
-        ExecuteMsg::Deposit {
-            on_behalf_of: Some(on_behalf_of_addr.to_string()),
-        },
+        ExecuteMsg::Deposit {},
     )
     .unwrap();
 

integration-tests/tests/test_oracles.rs

@@ -949,15 +949,7 @@ fn redbank_should_fail_if_no_price() {
     )
     .unwrap();
 
-    wasm.execute(
-        &red_bank_addr,
-        &Deposit {
-            on_behalf_of: None,
-        },
-        &[coin(1_000_000, "uatom")],
-        depositor,
-    )
-    .unwrap();
+    wasm.execute(&red_bank_addr, &Deposit {}, &[coin(1_000_000, "uatom")], depositor).unwrap();
 
     // execute msg should fail since it is attempting to query an asset from the oracle contract that doesn't have an LP pool set up
     wasm.execute(
@@ -1011,15 +1003,7 @@ fn redbank_quering_oracle_successfully() {
     )
     .unwrap();
 
-    wasm.execute(
-        &red_bank_addr,
-        &Deposit {
-            on_behalf_of: None,
-        },
-        &[coin(1_000_000, "uatom")],
-        depositor,
-    )
-    .unwrap();
+    wasm.execute(&red_bank_addr, &Deposit {}, &[coin(1_000_000, "uatom")], depositor).unwrap();
 
     wasm.execute(
         &red_bank_addr,

packages/testing/src/integration/mock_env.rs

@@ -228,9 +228,7 @@ impl RedBank {
         env.app.execute_contract(
             sender.clone(),
             self.contract_addr.clone(),
-            &red_bank::ExecuteMsg::Deposit {
-                on_behalf_of: None,
-            },
+            &red_bank::ExecuteMsg::Deposit {},
             &[coin],
         )
     }

packages/types/src/red_bank/msg.rs

@@ -53,10 +53,7 @@ pub enum ExecuteMsg {
 
     /// Deposit native coins. Deposited coins must be sent in the transaction
     /// this call is made
-    Deposit {
-        /// Address that will receive the coins
-        on_behalf_of: Option<String>,
-    },
+    Deposit {},
 
     /// Withdraw native coins
     Withdraw {

schemas/mars-red-bank/mars-red-bank.json

@@ -189,15 +189,6 @@
         "properties": {
           "deposit": {
             "type": "object",
-            "properties": {
-              "on_behalf_of": {
-                "description": "Address that will receive the coins",
-                "type": [
-                  "string",
-                  "null"
-                ]
-              }
-            },
             "additionalProperties": false
           }
         },

scripts/deploy/base/deployer.ts

@@ -119,7 +119,6 @@ export class Deployer {
       owner: this.deployerAddress,
       config: {
         address_provider: this.storage.addresses['address-provider']!,
-        close_factor: '0.5',
       },
     }
     await this.instantiate('red-bank', this.storage.codeIds['red-bank']!, msg)

scripts/package.json

@@ -16,23 +16,23 @@
     "format-check": "prettier --ignore-path .gitignore --check ."
   },
   "dependencies": {
-    "@cosmjs/cosmwasm-stargate": "^0.30.1",
-    "@cosmjs/proto-signing": "^0.30.1",
-    "@cosmjs/stargate": "^0.30.1",
+    "@cosmjs/cosmwasm-stargate": "^0.31.0",
+    "@cosmjs/proto-signing": "^0.31.0",
+    "@cosmjs/stargate": "^0.31.0",
     "@cosmwasm/ts-codegen": "^0.30.1",
     "chalk": "4.1.2",
     "cosmjs-types": "^0.8.0",
     "prepend-file": "^2.0.1",
     "ts-codegen": "^0.0.0"
   },
   "devDependencies": {
-    "@types/node": "^20.2.5",
-    "@typescript-eslint/eslint-plugin": "^5.59.9",
-    "@typescript-eslint/parser": "^5.59.9",
+    "@types/node": "^20.3.3",
+    "@typescript-eslint/eslint-plugin": "^5.61.0",
+    "@typescript-eslint/parser": "^5.61.0",
     "cosmjs-types": "^0.8.0",
-    "eslint": "^8.42.0",
+    "eslint": "^8.44.0",
     "eslint-config-prettier": "^8.8.0",
     "prettier": "^2.8.8",
-    "typescript": "^5.1.3"
+    "typescript": "^5.1.6"
   }
 }