feat: Add UTxO transaction phase 1 validation rules for Shelley Era
#439
Conversation
|
|
||
| /// **Cause:** Some of transaction inputs are not in current UTxOs set. | ||
| #[error("Bad inputs: bad_input={bad_input}, bad_input_index={bad_input_index}")] | ||
| BadInputsUTxO { |
There was a problem hiding this comment.
BadInputUTxO would be more appropriate in my opinion.
There was a problem hiding this comment.
Agreed, the Shelley Rules pdf (if we want to go by that as well) defines it as
In the case of any input not being valid, there is a
BadInputfailure.
I think it's worth being as semantically aligned with the Shelley ledger rules as we can be. Thoughts?
| /// Validate transaction's TTL field | ||
| /// pass if ttl >= current_slot | ||
| /// Reference: https://github.com/IntersectMBO/cardano-ledger/blob/24ef1741c5e0109e4d73685a24d8e753e225656d/eras/shelley/impl/src/Cardano/Ledger/Shelley/Rules/Utxo.hs#L421 | ||
| pub fn validate_time_to_live(tx: &alonzo::MintedTx, current_slot: u64) -> UTxOValidationResult { |
There was a problem hiding this comment.
In my opinion TTL validation isn't related to UTxO validation.
There was a problem hiding this comment.
Curious why you think this? Looking at the ledger rules for UTxOs in Shelley, they say
The UTXO rule has ten predicate failures:
- In the case of the current slot being greater than the time-to-live of the current transaction,
there is anExpiredfailure.
There was a problem hiding this comment.
wonder why it's UTxO-level if it only involves Tx-level fields
There was a problem hiding this comment.
Yeah, that is true that ttl is transaction level.
The reason why I chose this is to use the same Error types (naming) and category as cardano-ledger so it is easier to match with the ledger code.
Or, should we update the naming and category as it makes sense to us?
There was a problem hiding this comment.
I think you are correct to match cardano-ledger. Cardano-ledger made wacky decisions, but it makes sense to me to follow them.
|
|
||
| /// Validate transaction size is under the limit | ||
| /// Reference: https://github.com/IntersectMBO/cardano-ledger/blob/24ef1741c5e0109e4d73685a24d8e753e225656d/eras/shelley/impl/src/Cardano/Ledger/Shelley/Rules/Utxo.hs#L575 | ||
| pub fn validate_max_tx_size_utxo( |
There was a problem hiding this comment.
I also wouldn't consider tx size validation as apart of UTxO validation.
There was a problem hiding this comment.
I'm also curious why not here? This is also specified in the formal ledger rules for UTxO.
In the case that the transaction is too large, there is a MaxTxSize failure.
There was a problem hiding this comment.
As Simon said above, tx size and TTL are both transaction level rules which are not specific to UTxOs (Although the size and number of UTxOs do count toward tx size).
| pub fn validate_shelley_tx<F>( | ||
| raw_tx: &[u8], | ||
| shelley_params: &ShelleyParams, | ||
| current_slot: u64, | ||
| lookup_by_hash: F, | ||
| ) -> Result<(), TransactionValidationError> | ||
| where | ||
| F: Fn(TxOutRef) -> Result<TxIdentifier>, | ||
| { | ||
| let tx = MultiEraTx::decode_for_era(traverse::Era::Shelley, raw_tx) | ||
| .map_err(|e| TransactionValidationError::CborDecodeError(e.to_string()))?; | ||
| utxo::validate_shelley_tx(&tx, shelley_params, current_slot, lookup_by_hash).map_err(|e| *e)?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
I think it makes more sense to call validate_shelley_tx when BlockInfo::Era >= Shelley in tx_unpacker.rs and passing in the already decoded tx.
There was a problem hiding this comment.
But, some some checks (e.g. ttl check) are not available from Allegra era. (Will integrate this validate function to omnibus process in coming PR)
| validate_wrong_network(transaction_body, network_id.clone())?; | ||
| validate_wrong_network_withdrawal(transaction_body, network_id.clone())?; | ||
| validate_output_too_small_utxo(transaction_body, shelley_params)?; | ||
| validate_max_tx_size_utxo(tx_size, shelley_params)?; |
There was a problem hiding this comment.
There are two other rules I don't see here, but that are part of the Shelley Ledger rules for UTxOs, unless those are accounted for elsewhere? Those being
- OutputBootAddrAttrsTooBig -- output bootstrap address attributes too big
If the transaction creates any boostrap outputs whose attributes have size bigger than 64,
there is a OutputBootAddrAttrsTooBig failure.
In this case, Bootstrap (i.e. Byron) addresses have variable sized attributes in them. It is important to limit their overall size.
- ValueNotConservedUTxO - ensure that value consumed and produced matches up exactly
If the transaction does not correctly conserve the balance, there is a ValueNotConserved
failure.
There was a problem hiding this comment.
Ah, I see you have the latter accounted for here.
|
|
||
| /// **Cause:** The transaction size is too big. | ||
| #[error("Max tx size: supplied={supplied}, max={max}")] | ||
| MaxTxSizeUTxO { supplied: u32, max: u32 }, |
There was a problem hiding this comment.
| MaxTxSizeUTxO { supplied: u32, max: u32 }, | |
| MaxTxSize { supplied: u32, max: u32 }, |
Description
This PR implements transaction phase 1 validation rule:
UTxOrule forShelleyEra.Related Issue(s)
Related to #329
How was this tested?
UTxOrulecargo test --package acropolis_module_tx_unpacker --lib -- validations::shelley::utxo::tests::shelley_test --nocaptureChecklist
Impact / Side effects
No side effects
Reviewer notes / Areas to focus
Every
UTxOrule validation lives undertx_unpacker/validations/shelley/utxo.rsValueNotConservedUTxO,UpdateFailurerules will be implemented by following PRs.OutputBootAddrAttrsTooBig: For now, we only store raw payload forByronaddress, should we decode allByronaddresses and validate this rule?