diff --git a/.changelog/unreleased/improvements/4644-fee-foreign-reserve.md b/.changelog/unreleased/improvements/4644-fee-foreign-reserve.md new file mode 100644 index 00000000000..25844165392 --- /dev/null +++ b/.changelog/unreleased/improvements/4644-fee-foreign-reserve.md @@ -0,0 +1,4 @@ +- Modified the protocol logic of fees so that only the tip is sent to the + block proposer. The base fee is instead burnt if fees are paid with the + native token or sent to an internal account in case of a foreign token. + ([\#4644](https://github.com/anoma/namada/pull/4644)) \ No newline at end of file diff --git a/crates/node/src/protocol.rs b/crates/node/src/protocol.rs index 63e0b748ea9..5ca9f7c29bc 100644 --- a/crates/node/src/protocol.rs +++ b/crates/node/src/protocol.rs @@ -469,6 +469,29 @@ pub struct MaspTxResult { masp_section_ref: MaspTxRef, } +pub(crate) struct ProtocolGasPrice(pub(crate) token::Amount); + +/// A trait to handle the available gas prices for a given token, the protocol +/// one and the optional override coming from a block proposer local +/// configuration +pub(crate) trait MinimumGasPrice { + /// The gas price, potentially coming from the local configuration of the + /// block proposer (only for prepare_proposal) + fn price(&self) -> token::Amount; + /// The protocol imposed gas price + fn protocol_price(&self) -> token::Amount; +} + +impl MinimumGasPrice for ProtocolGasPrice { + fn price(&self) -> token::Amount { + self.0 + } + + fn protocol_price(&self) -> token::Amount { + self.0 + } +} + /// Performs the required operation on a wrapper transaction: /// - replay protection /// - fee payment @@ -501,12 +524,32 @@ where .write_log_mut() .write_tx_hash(tx.header_hash()) .expect("Error while writing tx hash to storage"); + let protocol_minimum_gas_price = + parameters::read_gas_cost(shell_params.state, &wrapper.fee.token) + .expect("Must be able to read gas cost parameter") + .ok_or(Error::FeeError(format!( + "The provided {} token is not allowed for fee payment", + wrapper.fee.token + )))?; // Charge or check fees, propagate any errors to prevent committing invalid // data let payment_result = match block_proposer { Some(block_proposer) => { - transfer_fee(shell_params, block_proposer, tx, wrapper, tx_index)? + let fee_components = crate::shell::get_fee_components( + wrapper, + shell_params.state, + &ProtocolGasPrice(protocol_minimum_gas_price), + ) + .map_err(|e| Error::FeeError(e.to_string()))?; + transfer_fee( + shell_params, + block_proposer, + tx, + wrapper, + tx_index, + &fee_components, + )? } None => check_fees(shell_params, tx, wrapper)?, }; @@ -562,15 +605,32 @@ where Ok(batch_results) } -/// Perform the actual transfer of fees from the fee payer to the block -/// proposer. No modifications to the write log are committed or dropped in this -/// function: this logic is up to the caller. +/// The fee amounts split between the tip and the base fee +pub struct FeeComponents { + pub base: token::Amount, + pub tip: token::Amount, +} + +impl FeeComponents { + pub fn get_total_fee(&self) -> Result { + checked!(self.base + self.tip).map_err(|_| { + Error::FeeError( + "Overflow in total fee reconstruction, this should not happen" + .to_string(), + ) + }) + } +} + +/// Perform the actual transfer of fees. No modifications to the write log are +/// committed or dropped in this function: this logic is up to the caller. pub fn transfer_fee( shell_params: &mut ShellParams<'_, S, D, H, CA>, block_proposer: &Address, tx: &Tx, wrapper: &WrapperTx, tx_index: &TxIndex, + fee_components: &FeeComponents, ) -> Result> where S: 'static @@ -584,149 +644,173 @@ where H: 'static + StorageHasher + Sync, CA: 'static + WasmCacheAccess + Sync, { - match wrapper.get_tx_fee() { - Ok(fees) => { - let fees = token::denom_to_amount( - fees, - &wrapper.fee.token, - shell_params.state, - ) - .map_err(Error::Error)?; + #[cfg(not(fuzzing))] + let balance = token::read_balance( + shell_params.state, + &wrapper.fee.token, + &wrapper.fee_payer(), + ) + .map_err(Error::Error)?; - #[cfg(not(fuzzing))] - let balance = token::read_balance( - shell_params.state, - &wrapper.fee.token, - &wrapper.fee_payer(), - ) - .map_err(Error::Error)?; + // Use half of the max value to make the balance check pass + // sometimes with arbitrary fees + #[cfg(fuzzing)] + let balance = Amount::max().checked_div_u64(2).unwrap(); - // Use half of the max value to make the balance check pass - // sometimes with arbitrary fees - #[cfg(fuzzing)] - let balance = Amount::max().checked_div_u64(2).unwrap(); + let fees = fee_components.get_total_fee()?; - let (post_bal, valid_batched_tx_result) = if let Some(post_bal) = - balance.checked_sub(fees) - { - fee_token_transfer( + let (post_bal, valid_batched_tx_result) = if let Some(post_bal) = + balance.checked_sub(fees) + { + fee_token_transfer( + shell_params.state, + &wrapper.fee.token, + &wrapper.fee_payer(), + block_proposer, + fee_components, + )?; + + (post_bal, None) + } else { + // See if the first inner transaction of the batch pays the fees + // with a masp unshield + match try_masp_fee_payment(shell_params, tx, tx_index) { + Ok(valid_batched_tx_result) => { + #[cfg(not(fuzzing))] + let balance = token::read_balance( shell_params.state, &wrapper.fee.token, &wrapper.fee_payer(), - block_proposer, - fees, - )?; - - (post_bal, None) - } else { - // See if the first inner transaction of the batch pays the fees - // with a masp unshield - match try_masp_fee_payment(shell_params, tx, tx_index) { - Ok(valid_batched_tx_result) => { - #[cfg(not(fuzzing))] - let balance = token::read_balance( + ) + .expect("Could not read balance key from storage"); + #[cfg(fuzzing)] + let balance = Amount::max().checked_div_u64(2).unwrap(); + + let post_bal = match balance.checked_sub(fees) { + Some(post_bal) => { + // This cannot fail given the checked_sub check + // here above + fee_token_transfer( shell_params.state, &wrapper.fee.token, &wrapper.fee_payer(), - ) - .expect("Could not read balance key from storage"); - #[cfg(fuzzing)] - let balance = Amount::max().checked_div_u64(2).unwrap(); - - let post_bal = match balance.checked_sub(fees) { - Some(post_bal) => { - // This cannot fail given the checked_sub check - // here above - fee_token_transfer( - shell_params.state, - &wrapper.fee.token, - &wrapper.fee_payer(), - block_proposer, - fees, - )?; - - post_bal - } - None => { - // This shouldn't happen as it should be - // prevented - // from process_proposal. - tracing::error!( - "Transfer of tx fee cannot be applied to \ - due to insufficient funds. This \ - shouldn't happen." - ); - return Err(Error::FeeError( - "Insufficient funds for fee payment" - .to_string(), - )); - } - }; + block_proposer, + fee_components, + )?; - // Batched tx result must be returned (and considered) - // only if fee payment was - // successful - (post_bal, Some(valid_batched_tx_result)) + post_bal } - Err(e) => { + None => { // This shouldn't happen as it should be prevented by // process_proposal. tracing::error!( - "Transfer of tx fee cannot be applied because of \ - an error: {}. This shouldn't happen.", - e + "Transfer of tx fee cannot be applied to due to \ + insufficient funds. This shouldn't happen." ); - return Err(e.into()); + return Err(Error::FeeError( + "Insufficient funds for fee payment".to_string(), + )); } - } - }; - - let target_post_balance = Some( - token::read_balance( - shell_params.state, - &wrapper.fee.token, - block_proposer, - ) - .map_err(Error::Error)? - .into(), - ); + }; - const FEE_PAYMENT_DESCRIPTOR: std::borrow::Cow<'static, str> = - std::borrow::Cow::Borrowed("wrapper-fee-payment"); - let current_block_height = shell_params - .state - .in_mem() - .get_last_block_height() - .next_height(); - shell_params.state.write_log_mut().emit_event( - TokenEvent { - descriptor: FEE_PAYMENT_DESCRIPTOR, - level: EventLevel::Tx, - operation: TokenOperation::transfer( - UserAccount::Internal(wrapper.fee_payer()), - UserAccount::Internal(block_proposer.clone()), - wrapper.fee.token.clone(), - fees.into(), - post_bal.into(), - target_post_balance, - ), - } - .with(HeightAttr(current_block_height)) - .with(TxHashAttr(tx.header_hash())), - ); + // Batched tx result must be returned (and considered) only if + // fee payment was successful + (post_bal, Some(valid_batched_tx_result)) + } + Err(e) => { + // This shouldn't happen as it should be prevented by + // process_proposal. + tracing::error!( + "Transfer of tx fee cannot be applied because of an \ + error: {}. This shouldn't happen.", + e + ); + return Err(e.into()); + } + } + }; - Ok(valid_batched_tx_result) + const BASE_FEE_PAYMENT_DESCRIPTOR: std::borrow::Cow<'static, str> = + std::borrow::Cow::Borrowed("wrapper-base-fee"); + const FEE_TIP_PAYMENT_DESCRIPTOR: std::borrow::Cow<'static, str> = + std::borrow::Cow::Borrowed("wrapper-fee-tip"); + let current_block_height = shell_params + .state + .in_mem() + .get_last_block_height() + .next_height(); + + // Event for the tip (if present) + if fee_components.tip.is_positive() { + let proposer_post_balance = Some( + token::read_balance( + shell_params.state, + &wrapper.fee.token, + block_proposer, + ) + .map_err(Error::Error)? + .into(), + ); + shell_params.state.write_log_mut().emit_event( + TokenEvent { + descriptor: FEE_TIP_PAYMENT_DESCRIPTOR, + level: EventLevel::Tx, + operation: TokenOperation::transfer( + UserAccount::Internal(wrapper.fee_payer()), + UserAccount::Internal(block_proposer.clone()), + wrapper.fee.token.clone(), + fee_components.tip.into(), + post_bal.into(), + proposer_post_balance, + ), + } + .with(HeightAttr(current_block_height)) + .with(TxHashAttr(tx.header_hash())), + ); + } + // Event for the base fee + let operation = if wrapper.fee.token + == shell_params + .state + .get_native_token() + .map_err(Error::Error)? + { + TokenOperation::Burn { + target_account: UserAccount::Internal(wrapper.fee_payer()), + token: wrapper.fee.token.clone(), + amount: fee_components.base.into(), + post_balance: post_bal.into(), } - Err(e) => { - // Fee overflow. This shouldn't happen as it should be prevented - // by process_proposal. - tracing::error!( - "Transfer of tx fee cannot be applied to due to fee overflow. \ - This shouldn't happen." - ); - Err(Error::FeeError(format!("{}", e))) + } else { + let pgf_post_balance = Some( + token::read_balance( + shell_params.state, + &wrapper.fee.token, + &namada_sdk::address::PGF, + ) + .map_err(Error::Error)? + .into(), + ); + TokenOperation::transfer( + UserAccount::Internal(wrapper.fee_payer()), + UserAccount::Internal(namada_sdk::address::PGF), + wrapper.fee.token.clone(), + fee_components.base.into(), + post_bal.into(), + pgf_post_balance, + ) + }; + shell_params.state.write_log_mut().emit_event( + TokenEvent { + descriptor: BASE_FEE_PAYMENT_DESCRIPTOR, + level: EventLevel::Tx, + operation, } - } + .with(HeightAttr(current_block_height)) + .with(TxHashAttr(tx.header_hash())), + ); + + Ok(valid_batched_tx_result) } /// Custom wrapper type for masp fee payment errors. The purpose of this type is @@ -926,24 +1010,67 @@ fn get_optional_masp_ref>( } // Manage the token transfer for the fee payment. If an error is detected the -// write log is dropped to prevent committing an inconsistent state. Propagates -// the result to the caller +// write log is dropped to prevent committing an inconsistent state. +// WARNING: the `token::burn_tokens` function does not return an error if the +// amount to burn exceeds the balance of the target address: it just burns +// whatever balance is available. Because of this, the caller of this function +// must ensure that the balance of the fee payer is enough to cover the entire +// fee cost. fn fee_token_transfer( state: &mut WLS, token: &Address, src: &Address, dest: &Address, - amount: Amount, + fee_components: &FeeComponents, ) -> Result<()> where WLS: State + StorageRead + TxWrites, { - token::transfer(&mut state.with_tx_writes(), token, src, dest, amount) - .map_err(|err| { - state.write_log_mut().drop_tx(); + fn fee_transfer_inner( + state: &mut WLS, + token: &Address, + src: &Address, + dest: &Address, + fee_components: &FeeComponents, + ) -> std::result::Result<(), state::Error> + where + WLS: State + StorageRead + TxWrites, + { + if token == &state.get_native_token()? { + // Burn the base fee + token::burn_tokens( + &mut state.with_tx_writes(), + token, + src, + fee_components.base, + )?; + } else { + // Transfer base fee to the PGF account + token::transfer( + &mut state.with_tx_writes(), + token, + src, + &namada_sdk::address::PGF, + fee_components.base, + )?; + } - Error::Error(err) - }) + // Transfer tip to the block proposer + token::transfer( + &mut state.with_tx_writes(), + token, + src, + dest, + fee_components.tip, + ) + } + + // Make sure to drop the content of the write log in case of any error + fee_transfer_inner(state, token, src, dest, fee_components).map_err(|err| { + state.write_log_mut().drop_tx(); + + Error::Error(err) + }) } /// Check if the fee payer has enough transparent balance to pay fees diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index db8eed3d1dc..8e8957eb04c 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -1353,6 +1353,7 @@ mod test_finalize_block { use super::*; use crate::oracle::control::Command; + use crate::protocol::ProtocolGasPrice; use crate::shell::test_utils::*; use crate::shims::abcipp_shim_types::shim::request::{ FinalizeBlock, ProcessedTx, @@ -1372,7 +1373,7 @@ mod test_finalize_block { let mut wrapper_tx = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair.ref_to(), @@ -1412,7 +1413,7 @@ mod test_finalize_block { let mut batch = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, sk.ref_to(), @@ -3276,7 +3277,7 @@ mod test_finalize_block { let mut wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair.ref_to(), @@ -3289,7 +3290,7 @@ mod test_finalize_block { let mut new_wrapper = wrapper.clone(); new_wrapper.update_header(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair_2.ref_to(), @@ -3383,7 +3384,7 @@ mod test_finalize_block { let mut wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair.ref_to(), @@ -3405,7 +3406,7 @@ mod test_finalize_block { let mut new_wrapper = wrapper.clone(); new_wrapper.update_header(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair_2.ref_to(), @@ -3491,7 +3492,7 @@ mod test_finalize_block { Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { amount_per_gas_unit: DenominatedAmount::native( - 1.into(), + 10.into(), ), token: shell.state.in_mem().native_token.clone(), }, @@ -3509,9 +3510,7 @@ mod test_finalize_block { let mut unsigned_wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native( - Amount::zero(), - ), + amount_per_gas_unit: DenominatedAmount::native(10.into()), token: shell.state.in_mem().native_token.clone(), }, keypair.ref_to(), @@ -3703,7 +3702,9 @@ mod test_finalize_block { let mut wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(0.into()), + amount_per_gas_unit: DenominatedAmount::native( + Amount::zero(), + ), token: shell.state.in_mem().native_token.clone(), }, keypair.ref_to(), @@ -3991,10 +3992,11 @@ mod test_finalize_block { assert_eq!(balance, 0.into()); } - // Test that the fees collected from a block are withdrew from the wrapper - // signer and credited to the block proposer + // Test that when fees are paid in the native token, the tip is withdrew + // from the wrapper signer and credited to the block proposer and the + // base fee is burned #[test] - fn test_fee_payment_to_block_proposer() { + fn test_fee_distribution_native_token() { let (mut shell, _, _, _) = setup(); let validator = shell.mode.get_validator_address().unwrap().to_owned(); @@ -4014,11 +4016,14 @@ mod test_finalize_block { &validator, ) .unwrap(); + let total_supply = + namada_sdk::token::get_effective_total_native_supply(&shell.state) + .unwrap(); let mut wrapper = Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { - amount_per_gas_unit: DenominatedAmount::native(1.into()), + amount_per_gas_unit: DenominatedAmount::native(11.into()), token: shell.state.in_mem().native_token.clone(), }, namada_apps_lib::wallet::defaults::albert_keypair().ref_to(), @@ -4029,12 +4034,16 @@ mod test_finalize_block { .add_code(TestWasms::TxNoOp.read_bytes(), None) .add_data("Transaction data"); wrapper.sign_wrapper(albert_keypair()); - let fee_amount = - wrapper.header().wrapper().unwrap().get_tx_fee().unwrap(); - let fee_amount = namada_sdk::token::denom_to_amount( - fee_amount, - &wrapper.header().wrapper().unwrap().fee.token, + let protocol_minimum_gas_price = parameters::read_gas_cost( &shell.state, + &wrapper.header.wrapper().unwrap().fee.token, + ) + .unwrap() + .unwrap(); + let fee_components = get_fee_components( + &wrapper.header().wrapper().unwrap(), + &shell.state, + &ProtocolGasPrice(protocol_minimum_gas_price), ) .unwrap(); @@ -4074,7 +4083,7 @@ mod test_finalize_block { .unwrap(); assert_eq!( new_proposer_balance, - proposer_balance.checked_add(fee_amount).unwrap() + proposer_balance.checked_add(fee_components.tip).unwrap() ); let new_signer_balance = namada_sdk::token::read_balance( @@ -4085,8 +4094,171 @@ mod test_finalize_block { .unwrap(); assert_eq!( new_signer_balance, - signer_balance.checked_sub(fee_amount).unwrap() + signer_balance + .checked_sub(fee_components.get_total_fee().unwrap()) + .unwrap() + ); + + // Check burning of the base fee + let new_total_supply = + namada_sdk::token::get_effective_total_native_supply(&shell.state) + .unwrap(); + assert_eq!( + new_total_supply, + total_supply.checked_sub(fee_components.base).unwrap() + ) + } + + // Test that when fees are paid in a foreign token, the tip is withdrew from + // the wrapper signer and credited to the block proposer and the base fee is + // sent to the PGF account + #[test] + fn test_fee_distribution_foreign_token() { + let (mut shell, _, _, _) = setup(); + let btc = namada_sdk::address::testing::btc(); + let btc_denom = read_denom(&shell.state, &btc).unwrap().unwrap(); + let fee_amount: Amount = (WRAPPER_GAS_LIMIT * 2).into(); + + // Credit some tokens for fee payment + namada_sdk::token::credit_tokens( + &mut shell.state, + &btc, + &Address::from(&albert_keypair().to_public()), + fee_amount, ) + .unwrap(); + let signer_balance = read_balance( + &shell.state, + &btc, + &Address::from(&albert_keypair().to_public()), + ) + .unwrap(); + assert_eq!(signer_balance, fee_amount.clone()); + + // Whitelist BTC for fee payment + let gas_cost_key = namada_sdk::parameters::storage::get_gas_cost_key(); + let mut gas_prices: BTreeMap = + shell.read_storage_key(&gas_cost_key).unwrap(); + gas_prices.insert(btc.clone(), 1.into()); + shell.shell.state.write(&gas_cost_key, gas_prices).unwrap(); + shell.commit(); + + let validator = shell.mode.get_validator_address().unwrap().to_owned(); + let pos_params = read_pos_params(&shell.state).unwrap(); + let consensus_key = + proof_of_stake::storage::validator_consensus_key_handle(&validator) + .get(&shell.state, Epoch::default(), &pos_params) + .unwrap() + .unwrap(); + let proposer_address = HEXUPPER + .decode(consensus_key.tm_raw_hash().as_bytes()) + .unwrap(); + + let proposer_balance = + namada_sdk::token::read_balance(&shell.state, &btc, &validator) + .unwrap(); + let total_btc_supply = + namada_sdk::token::read_total_supply(&shell.state, &btc).unwrap(); + + let mut wrapper = + Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( + Fee { + amount_per_gas_unit: DenominatedAmount::new( + 2.into(), + btc_denom, + ), + token: btc.clone(), + }, + namada_apps_lib::wallet::defaults::albert_keypair().ref_to(), + WRAPPER_GAS_LIMIT.into(), + )))); + wrapper.header.chain_id = shell.chain_id.clone(); + wrapper + .add_code(TestWasms::TxNoOp.read_bytes(), None) + .add_data("Transaction data"); + wrapper.sign_wrapper(albert_keypair()); + let protocol_minimum_gas_price = parameters::read_gas_cost( + &shell.state, + &wrapper.header.wrapper().unwrap().fee.token, + ) + .unwrap() + .unwrap(); + let fee_components = get_fee_components( + &wrapper.header().wrapper().unwrap(), + &shell.state, + &ProtocolGasPrice(protocol_minimum_gas_price), + ) + .unwrap(); + + let signer_balance = namada_sdk::token::read_balance( + &shell.state, + &btc, + &wrapper.header().wrapper().unwrap().fee_payer(), + ) + .unwrap(); + let pgf_balance = namada_sdk::token::read_balance( + &shell.state, + &btc, + &namada_sdk::address::PGF, + ) + .unwrap(); + + let processed_tx = ProcessedTx { + tx: wrapper.to_bytes().into(), + result: TxResult { + code: ResultCode::Ok.into(), + info: "".into(), + }, + }; + + let event = &shell + .finalize_block(FinalizeBlock { + txs: vec![processed_tx], + proposer_address, + ..Default::default() + }) + .expect("Test failed")[0]; + + // Check fee payment + assert_eq!(*event.kind(), APPLIED_TX); + let code = event.read_attribute::().expect("Test failed"); + assert_eq!(code, ResultCode::Ok); + + let new_proposer_balance = + namada_sdk::token::read_balance(&shell.state, &btc, &validator) + .unwrap(); + assert_eq!( + new_proposer_balance, + proposer_balance.checked_add(fee_components.tip).unwrap() + ); + + let new_signer_balance = namada_sdk::token::read_balance( + &shell.state, + &btc, + &wrapper.header().wrapper().unwrap().fee_payer(), + ) + .unwrap(); + assert_eq!( + new_signer_balance, + signer_balance + .checked_sub(fee_components.get_total_fee().unwrap()) + .unwrap() + ); + + // Check that the base fee has been sent to pgf and no burning happened + let new_pgf_balance = namada_sdk::token::read_balance( + &shell.state, + &btc, + &namada_sdk::address::PGF, + ) + .unwrap(); + assert_eq!( + new_pgf_balance, + pgf_balance.checked_add(fee_components.base).unwrap() + ); + let new_total_supply = + namada_sdk::token::read_total_supply(&shell.state, &btc).unwrap(); + assert_eq!(new_total_supply, total_btc_supply) } #[test] @@ -6095,7 +6267,7 @@ mod test_finalize_block { Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { amount_per_gas_unit: DenominatedAmount::native( - 1.into(), + 10.into(), ), token: shell.state.in_mem().native_token.clone(), }, @@ -6177,7 +6349,7 @@ mod test_finalize_block { Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { amount_per_gas_unit: DenominatedAmount::native( - 1.into(), + 10.into(), ), token: shell.state.in_mem().native_token.clone(), }, @@ -6255,7 +6427,7 @@ mod test_finalize_block { Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { amount_per_gas_unit: DenominatedAmount::native( - 1.into(), + 10.into(), ), token: shell.state.in_mem().native_token.clone(), }, @@ -6326,7 +6498,7 @@ mod test_finalize_block { Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new( Fee { amount_per_gas_unit: DenominatedAmount::native( - 1.into(), + 10.into(), ), token: shell.state.in_mem().native_token.clone(), }, diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index a6e47671f82..c94ae87a3a3 100644 --- a/crates/node/src/shell/mod.rs +++ b/crates/node/src/shell/mod.rs @@ -74,7 +74,7 @@ use tokio::sync::mpsc::{Receiver, UnboundedSender}; use super::ethereum_oracle::{self as oracle, last_processed_block}; use crate::config::{self, TendermintMode, ValidatorLocalConfig, genesis}; -use crate::protocol::ShellParams; +use crate::protocol::{FeeComponents, ShellParams}; use crate::shims::abcipp_shim_types::shim; use crate::shims::abcipp_shim_types::shim::TakeSnapshot; use crate::shims::abcipp_shim_types::shim::response::TxResult; @@ -1510,7 +1510,7 @@ where H: StorageHasher + Sync + 'static, CA: 'static + WasmCacheAccess + Sync, { - let minimum_gas_price = + let protocol_gas_price = parameters::read_gas_cost(shell_params.state, &wrapper.fee.token) .expect("Must be able to read gas cost parameter") .ok_or(Error::TxApply(protocol::Error::FeeError(format!( @@ -1518,49 +1518,78 @@ where wrapper.fee.token ))))?; - fee_data_check(wrapper, minimum_gas_price, shell_params)?; + fee_data_check( + wrapper, + &crate::protocol::ProtocolGasPrice(protocol_gas_price), + shell_params.state, + )?; protocol::check_fees(shell_params, tx, wrapper) .map_err(Error::TxApply) .map(|_| ()) } -/// Check the validity of the fee data -pub fn fee_data_check( +/// Check the validity of the fee data and return the fee information split +/// between base and tip +pub(crate) fn fee_data_check( wrapper: &WrapperTx, - minimum_gas_price: token::Amount, - shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, -) -> ShellResult<()> -where - D: DB + for<'iter> DBIter<'iter> + Sync + 'static, - H: StorageHasher + Sync + 'static, - CA: 'static + WasmCacheAccess + Sync, -{ + minimum_gas_price: &impl crate::protocol::MinimumGasPrice, + storage: &impl StorageRead, +) -> ShellResult { match token::denom_to_amount( wrapper.fee.amount_per_gas_unit, &wrapper.fee.token, - shell_params.state, + storage, ) { - Ok(amount_per_gas_unit) if amount_per_gas_unit < minimum_gas_price => { - // The fees do not match the minimum required - return Err(Error::TxApply(protocol::Error::FeeError(format!( - "Fee amount {:?} does not match the minimum required amount \ - {:?} for token {}", - wrapper.fee.amount_per_gas_unit, - minimum_gas_price, - wrapper.fee.token - )))); - } - Ok(_) => {} - Err(err) => { - return Err(Error::TxApply(protocol::Error::FeeError(format!( - "The precision of the fee amount {:?} is higher than the \ - denomination for token {}: {}", - wrapper.fee.amount_per_gas_unit, wrapper.fee.token, err, - )))); + Ok(amount_per_gas_unit) => { + if amount_per_gas_unit < minimum_gas_price.price() { + // The fees do not match the minimum required + Err(Error::TxApply(protocol::Error::FeeError(format!( + "Fee amount {:?} does not match the minimum required \ + amount {:?} for token {}", + wrapper.fee.amount_per_gas_unit, + minimum_gas_price.price(), + wrapper.fee.token + )))) + } else { + get_fee_components(wrapper, storage, minimum_gas_price) + } } + Err(err) => Err(Error::TxApply(protocol::Error::FeeError(format!( + "The precision of the fee amount {:?} is higher than the \ + denomination for token {}: {}", + wrapper.fee.amount_per_gas_unit, wrapper.fee.token, err, + )))), } +} - Ok(()) +/// Extract the components of the fee for the provided wrapper transaction +pub(crate) fn get_fee_components( + wrapper: &WrapperTx, + storage: &impl StorageRead, + minimum_gas_price: &impl crate::protocol::MinimumGasPrice, +) -> ShellResult { + let denom_amt = wrapper.get_tx_fee().map_err(|e| { + Error::TxApply(protocol::Error::FeeError(e.to_string())) + })?; + let fees = token::denom_to_amount(denom_amt, &wrapper.fee.token, storage)?; + let base_fee = minimum_gas_price + .protocol_price() + .checked_mul(token::Amount::from(wrapper.gas_limit)) + .ok_or_else(|| { + Error::TxApply(protocol::Error::FeeError( + "Overflow in base fee computation".to_string(), + )) + })?; + let tip = fees.checked_sub(base_fee).ok_or_else(|| { + Error::TxApply(protocol::Error::FeeError( + "Underflow in tip computation".to_string(), + )) + })?; + + Ok(FeeComponents { + base: base_fee, + tip, + }) } /// for the shell diff --git a/crates/node/src/shell/prepare_proposal.rs b/crates/node/src/shell/prepare_proposal.rs index 5461021deb6..9c0d5394c9d 100644 --- a/crates/node/src/shell/prepare_proposal.rs +++ b/crates/node/src/shell/prepare_proposal.rs @@ -344,29 +344,57 @@ where H: StorageHasher + Sync + 'static, CA: 'static + WasmCacheAccess + Sync, { - let minimum_gas_price = compute_min_gas_price( + let proposer_gas_price = get_proposer_gas_price( &wrapper.fee.token, proposer_local_config, shell_params.state, )?; + let fee_components = super::fee_data_check( + wrapper, + &proposer_gas_price, + shell_params.state, + )?; - super::fee_data_check(wrapper, minimum_gas_price, shell_params)?; + protocol::transfer_fee( + shell_params, + proposer, + tx, + wrapper, + tx_index, + &fee_components, + ) + .map_or_else(|e| Err(Error::TxApply(e)), |_| Ok(())) +} - protocol::transfer_fee(shell_params, proposer, tx, wrapper, tx_index) - .map_or_else(|e| Err(Error::TxApply(e)), |_| Ok(())) +// The gas prices of the block proposer. +// WARNING: this type should not be exposed to other modules as the local config +// price override shall not be considered outside the prepare proposal step +struct ProposerGasPrice { + protocol: Amount, + proposer: Option, } -fn compute_min_gas_price( +impl protocol::MinimumGasPrice for ProposerGasPrice { + fn price(&self) -> namada_sdk::token::Amount { + self.proposer.unwrap_or(self.protocol) + } + + fn protocol_price(&self) -> namada_sdk::token::Amount { + self.protocol + } +} + +fn get_proposer_gas_price( fee_token: &Address, proposer_local_config: Option<&ValidatorLocalConfig>, temp_state: &TempWlState<'_, D, H>, -) -> Result +) -> Result where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, H: StorageHasher + Sync + 'static, { #[cfg(not(fuzzing))] - let consensus_min_gas_price = + let protocol_min_gas_price = namada_sdk::parameters::read_gas_cost(temp_state, fee_token) .expect("Must be able to read gas cost parameter") .ok_or_else(|| { @@ -376,16 +404,20 @@ where ))) })?; #[cfg(fuzzing)] - let consensus_min_gas_price = { + let protocol_min_gas_price = { let _ = temp_state; Amount::from_u64(10) }; + let mut minimum_gas_price = ProposerGasPrice { + protocol: protocol_min_gas_price, + proposer: None, + }; let Some(config) = proposer_local_config else { - return Ok(consensus_min_gas_price); + return Ok(minimum_gas_price); }; - let validator_min_gas_price = config + let proposer_min_gas_price = config .accepted_gas_tokens .get(fee_token) .ok_or_else(|| { @@ -396,23 +428,23 @@ where })? .to_owned(); - // The validator's local config overrides the consensus param - // when creating a block, as long as its min gas price for - // `token` is not lower than the consensus value - Ok(if validator_min_gas_price < consensus_min_gas_price { + // The validator's local config overrides the consensus param when creating + // a block, as long as its min gas price for `token` is not lower than the + // consensus value + if proposer_min_gas_price < protocol_min_gas_price { tracing::warn!( fee_token = %fee_token, - validator_min_gas_price = %DenominatedAmount::from(validator_min_gas_price), - consensus_min_gas_price = %DenominatedAmount::from(consensus_min_gas_price), + validator_min_gas_price = %DenominatedAmount::from(proposer_min_gas_price), + consensus_min_gas_price = %DenominatedAmount::from(protocol_min_gas_price), "The gas price for the given token set by the block proposer \ is lower than the value agreed upon by consensus. \ Falling back to consensus value." ); - - consensus_min_gas_price } else { - validator_min_gas_price - }) + minimum_gas_price.proposer = Some(proposer_min_gas_price); + }; + + Ok(minimum_gas_price) } #[allow(clippy::cast_possible_wrap, clippy::cast_possible_truncation)] @@ -443,6 +475,7 @@ mod test_prepare_proposal { use namada_vote_ext::{ethereum_events, ethereum_tx_data_variants}; use super::*; + use crate::protocol::MinimumGasPrice; use crate::shell::EthereumTxData; use crate::shell::test_utils::{ self, TestShell, gen_keypair, get_pkh_from_address, @@ -1501,13 +1534,13 @@ mod test_prepare_proposal { m }, }; - let computed_min_gas_price = compute_min_gas_price( + let computed_min_gas_price = get_proposer_gas_price( &shell.state.in_mem().native_token, Some(&config), &temp_state, ) .unwrap(); - assert_eq!(computed_min_gas_price, consensus_min_gas_price); + assert_eq!(computed_min_gas_price.price(), consensus_min_gas_price); } } diff --git a/crates/node/src/shell/process_proposal.rs b/crates/node/src/shell/process_proposal.rs index 915b8daf9e0..18e525b5c31 100644 --- a/crates/node/src/shell/process_proposal.rs +++ b/crates/node/src/shell/process_proposal.rs @@ -576,18 +576,27 @@ where H: StorageHasher + Sync + 'static, CA: 'static + WasmCacheAccess + Sync, { - let minimum_gas_price = + let protocol_minimum_gas_price = parameters::read_gas_cost(shell_params.state, &wrapper.fee.token) .expect("Must be able to read gas cost parameter") .ok_or(Error::TxApply(protocol::Error::FeeError(format!( "The provided {} token is not allowed for fee payment", wrapper.fee.token ))))?; - - fee_data_check(wrapper, minimum_gas_price, shell_params)?; - - protocol::transfer_fee(shell_params, proposer, tx, wrapper, tx_index) - .map_or_else(|e| Err(Error::TxApply(e)), |_| Ok(())) + let minimum_gas_price = + protocol::ProtocolGasPrice(protocol_minimum_gas_price); + let fee_components = + fee_data_check(wrapper, &minimum_gas_price, shell_params.state)?; + + protocol::transfer_fee( + shell_params, + proposer, + tx, + wrapper, + tx_index, + &fee_components, + ) + .map_or_else(|e| Err(Error::TxApply(e)), |_| Ok(())) } /// We test the failure cases of [`process_proposal`]. The happy flows diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 77eebb1b5b3..12e829281a9 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -854,14 +854,14 @@ fn fee_payment_with_ibc_token() -> Result<()> { epoch = epoch_sleep(&test, &rpc, 120)?; } - // Transfer 250 samoleans from Gaia to Namada + // Transfer 500 samoleans from Gaia to Namada let namada_receiver = find_address(&test, ALBERT_KEY)?.to_string(); transfer_from_cosmos( &test_gaia, COSMOS_USER, &namada_receiver, COSMOS_COIN, - 250, + 500, &port_id_gaia, &channel_id_gaia, None, @@ -870,8 +870,10 @@ fn fee_payment_with_ibc_token() -> Result<()> { wait_for_packet_relay(&hermes_dir, &port_id_gaia, &channel_id_gaia, &test)?; // Check the token on Namada - check_balance(&test, ALBERT_KEY, &ibc_token_on_namada, 250)?; - check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 750)?; + check_balance(&test, ALBERT_KEY, &ibc_token_on_namada, 500)?; + check_cosmos_balance(&test_gaia, COSMOS_USER, COSMOS_COIN, 500)?; + check_balance(&test, PGF_ADDRESS.to_string(), &ibc_token_on_namada, 0)?; + check_ibc_token_supply(&test, &ibc_token_on_namada, 500)?; // Transparent transfer in Namada paying gas with samoleans transfer_on_chain( @@ -882,12 +884,21 @@ fn fee_payment_with_ibc_token() -> Result<()> { NAM, 50, ALBERT_KEY, - &["--gas-token", &ibc_token_on_namada, "--gas-limit", "250"], + &[ + "--gas-token", + &ibc_token_on_namada, + "--gas-limit", + "250", + "--gas-price", + "2", + ], )?; check_balance(&test, ALBERT, NAM, 1_999_950)?; check_balance(&test, CHRISTEL, NAM, 2_000_050)?; check_balance(&test, ALBERT_KEY, &ibc_token_on_namada, 0)?; check_balance(&test, "validator-0", &ibc_token_on_namada, 250)?; + check_balance(&test, PGF_ADDRESS.to_string(), &ibc_token_on_namada, 250)?; + check_ibc_token_supply(&test, &ibc_token_on_namada, 500)?; Ok(()) } @@ -3089,6 +3100,23 @@ fn check_balance( Ok(()) } +fn check_ibc_token_supply( + test: &Test, + token: impl AsRef, + expected_amount: u64, +) -> Result<()> { + let rpc = get_actor_rpc(test, Who::Validator(0)); + + let query_args = + vec!["total-supply", "--token", token.as_ref(), "--node", &rpc]; + let mut client = run!(test, Bin::Client, query_args, Some(40))?; + let expected = format!("Total supply of token tnam.*: {expected_amount}"); + + client.exp_regex(&expected)?; + client.assert_success(); + Ok(()) +} + fn check_shielded_balance( test: &Test, owner: impl AsRef, diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index d2727bb628a..2cdee00a578 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -554,10 +554,10 @@ fn pos_rewards() -> Result<()> { }); assert_matches!(captured.result, Ok(_)); let _res = captured - .matches(r"Current annual staking rewards rate: 65.705255877354") + .matches(r"Current annual staking rewards rate: 65.705256154607") .expect("Test failed"); let _res = captured - .matches(r"PoS inflation rate: 0.066593164444") + .matches(r"PoS inflation rate: 0.066593164725") .expect("Test failed"); Ok(()) @@ -1299,7 +1299,7 @@ fn pgf_governance_proposal() -> Result<()> { CapturedOutput::of(|| run(&node, Bin::Client, query_total_supply_args)); assert_matches!(captured.result, Ok(_)); assert!(captured.contains( - "token tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5: 118400024.740301" + "token tnam1q9kn74xfzytqkqyycfrhycr8ajam8ny935cge0z5: 118400022.740301" )); let query_native_supply_args = @@ -1308,7 +1308,7 @@ fn pgf_governance_proposal() -> Result<()> { run(&node, Bin::Client, query_native_supply_args) }); assert_matches!(captured.result, Ok(_)); - assert!(captured.contains("nam: 118400010.473048")); + assert!(captured.contains("nam: 118400008.473048")); // 8. Submit proposal funding let albert = defaults::albert_address();