diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index f14b6047d9..f5417de09c 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -38,60 +38,80 @@ fn extract_masp_tx( tx: &Tx, masp_refs: &MaspTxRefs, ) -> Result, Error> { - let mut masp_txs = Vec::new(); - // FIXME: use iter? - for masp_ref in &masp_refs.0 { - match masp_ref { - MaspTxRef::MaspSection(id) => { - // NOTE: simply looking for masp sections attached to the tx - // is not safe. We don't validate the sections attached to a - // transaction se we could end up with transactions carrying - // an unnecessary masp section. We must instead look for the - // required masp sections published in the events - let transaction = tx - .get_masp_section(id) - .ok_or_else(|| { - Error::Other(format!( - "Missing expected masp transaction with id {id}" - )) - })? - .clone(); - masp_txs.push(transaction); - } - // FIXME: we are not using this hash, why???? - MaspTxRef::IbcData(hash) => { - // FIXME: improve if possible - let mut ibc_masp_tx = None; - - for cmt in &tx.header.batch { - let Some(tx_data) = tx.data(cmt) else { - continue; - }; - let Ok(IbcMessage::Envelope(ref envelope)) = + // NOTE: It is possible to have two identical references in a same batch: + // this is because, some types of MASP data packet can be correctly executed + // more than once. We have to make sure we account for this by not using + // collections that allow for duplicates (both in the inputs and in the + // outputs): if the same reference shows up multiple times in the input we + // must process it the same number of times to ensure we contruct the + // correct state + masp_refs + .0 + .iter() + .try_fold(vec![], |mut acc, ref masp_ref| { + match masp_ref { + MaspTxRef::MaspSection(id) => { + // Simply looking for masp sections attached to the tx + // is not safe. We don't validate the sections attached to a + // transaction se we could end up with transactions carrying + // an unnecessary masp section. We must instead look for the + // required masp sections published in the events + let transaction = tx + .get_masp_section(id) + .ok_or_else(|| { + Error::Other(format!( + "Missing expected masp transaction with id \ + {id}" + )) + })? + .clone(); + acc.push(transaction); + + Ok(acc) + } + MaspTxRef::IbcData(hash) => { + // Dereference the masp ref to the first instance that + // matches is, even if it is not the exact one that produced + // the event, the data we extract will be exactly the same + let masp_ibc_tx = tx + .commitments() + .iter() + .find(|cmt| cmt.data_sechash() == hash) + .ok_or_else(|| { + Error::Other(format!( + "Couldn't find data section with hash {hash}" + )) + })?; + let tx_data = tx.data(masp_ibc_tx).ok_or_else(|| { + Error::Other( + "Missing expected data section".to_string(), + ) + })?; + + let IbcMessage::Envelope(envelope) = decode_message::(&tx_data) + .map_err(|e| Error::Other(e.to_string()))? else { - continue; + return Err(Error::Other( + "Expected IBC packet to be an envelope".to_string(), + )); }; - if let Some(masp_tx) = - extract_masp_tx_from_envelope(envelope) + + if let Some(transaction) = + extract_masp_tx_from_envelope(&envelope) { - ibc_masp_tx = Some(masp_tx); - } - } + acc.push(transaction); - if let Some(transaction) = ibc_masp_tx { - masp_txs.push(transaction); - } else { - return Err(Error::Other(format!( - "Missing expected ibc masp transaction with data \ - section hash: {hash}" - ))); + Ok(acc) + } else { + Err(Error::Other( + "Failed to retrieve MASP over IBC transaction" + .to_string(), + )) + } } } - } - } - - Ok(masp_txs) + }) } // Retrieves all the indexes at the specified height which refer diff --git a/crates/tx/src/data/mod.rs b/crates/tx/src/data/mod.rs index d5cabcb82a..364e7f4af9 100644 --- a/crates/tx/src/data/mod.rs +++ b/crates/tx/src/data/mod.rs @@ -192,7 +192,9 @@ pub struct ExtendedTxResult { /// for shielded actions) // NOTE: it's paramount to enforce a single, ordered collection for all the // masp transactions to ensure that the exact view on the tx sequence is - // preserved in the events + // preserved in the events. Also, it is possible for two refs to be exactly + // the same, we must make sure to emit events for both so that the + // client/indexer can properly construct their internal state pub masp_tx_refs: MaspTxRefs, }