Masp/Base boundary conditions

[CarloModicaPortfolio]

Looking at the MASP VP, and it looks like some of the coundary conditions are not checked. More specifically:

According to specs: If the source address is not the MASP validity predicate, then the transparent transaction value pool's amount must equal zero. But in vp_masp.rs:

        if transfer.source != masp() {
            // Handle transparent input
            // Note that the asset type is timestamped so shields
            // where the shielded value has an incorrect timestamp
            // are automatically rejected
            for denom in token::MaspDenom::iter() {
                let (_transp_asset, transp_amt) = convert_amount(
                    ctx.get_block_epoch().unwrap(),
                    &transfer.token,
                    transfer.amount.into(),
                    denom,
                );

                // Non-masp sources add to transparent tx pool
                transparent_tx_pool += transp_amt;
           

looks like that transparent_tx_pool is never checked to be zero.

Similarly when the source is the MASP vp, both according to specs and code comment the transparent transaction value pool's amount must equal the containing wrapper transaction's fee amount. This checks seems to be missing in the code. Nor seems to be checked if the transparent transaction value pool's asset type is be derived from the containing wrapper transaction's fee token.
The code below:

            // Handle shielded input
            // The following boundary conditions must be satisfied
            // 1. Zero transparent input
            // 2. the transparent transaction value pool's amount must equal< the
            // containing wrapper transaction's fee amount
            // Satisfies 1.
            if let Some(transp_bundle) = shielded_tx.transparent_bundle() {
                if !transp_bundle.vin.is_empty() {
                    debug_log!(
                        "Transparent input to a transaction from the masp \
                         must be 0 but is {}",
                        transp_bundle.vin.len()
                    );
                    return reject();

Lastly, there is a missmatch between specs and code when the target adress is not the MASP vp, as the specs say there must be exactly one transparent output, while in the code there can be up to 4.

[grarco]

With regards to the fees, the logic in the code should be correct as we decided to not allow fee payment directly in a masp transfer, the check is here:

namada/shared/src/ledger/native_vp/masp.rs

Lines 280 to 284 in a07a3b2

	Some(Ordering::Greater) => {
	tracing::debug!(
	"Transaction fees cannot be paid inside MASP transaction."
	);
	return Ok(false);

I probably forgot to remove the misleading comment and update the specs

[cwgoes]

Superseded by more recent work.