From 2186761dd72f7a61063b1a21fc78b645b5d9c9dc Mon Sep 17 00:00:00 2001 From: Nikolay Yakimov Date: Wed, 8 Nov 2023 13:42:08 +0300 Subject: [PATCH 1/2] MIR: add missing gas cost for Or comparison --- contrib/mir/src/gas.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/contrib/mir/src/gas.rs b/contrib/mir/src/gas.rs index d65bffb4d48d..b41e71bce2c0 100644 --- a/contrib/mir/src/gas.rs +++ b/contrib/mir/src/gas.rs @@ -152,7 +152,7 @@ pub mod interpret_cost { use checked::Checked; use super::{AsGasCost, OutOfGas}; - use crate::ast::TypedValue; + use crate::ast::{Or, TypedValue}; pub const DIP: u32 = 10; pub const DROP: u32 = 10; @@ -246,6 +246,7 @@ pub mod interpret_cost { let cmp_option = Checked::from(10u32); const ADDRESS_SIZE: usize = 20 + 31; // hash size + max entrypoint size const CMP_CHAIN_ID: u32 = 30; + let cmp_or = Checked::from(10u32); Ok(match (v1, v2) { (V::Nat(l), V::Nat(r)) => { // NB: eventually when using BigInts, use BigInt::bits() &c @@ -269,6 +270,13 @@ pub mod interpret_cost { .as_gas_cost()?, (V::Address(..), V::Address(..)) => cmp_bytes(ADDRESS_SIZE, ADDRESS_SIZE)?, (V::ChainId(..), V::ChainId(..)) => CMP_CHAIN_ID, + (V::Or(l), V::Or(r)) => match (l.as_ref(), r.as_ref()) { + (Or::Left(x), Or::Left(y)) => cmp_or + compare(x, y)?, + (Or::Right(x), Or::Right(y)) => cmp_or + compare(x, y)?, + (Or::Left(_), Or::Right(_)) => cmp_or, + (Or::Right(_), Or::Left(_)) => cmp_or, + } + .as_gas_cost()?, _ => unreachable!("Comparison of incomparable values"), }) } -- GitLab From 77ebe0078e3e644a5e4251e4e1bd19c9a48af2e9 Mon Sep 17 00:00:00 2001 From: Nikolay Yakimov Date: Wed, 8 Nov 2023 13:44:38 +0300 Subject: [PATCH 2/2] MIR: make comparability pattern matches total Problem: Totality of pattern matches in comparability gas computation, as well as comparison itself wasn't checked by the compiler. Those are easy to miss. Solution: Explicitly match lhs against all patterns. When new constructors are added, the compiler will now complain about a non-total pattern match. --- contrib/mir/src/ast/comparable.rs | 25 ++++++++++++++++++++++++- contrib/mir/src/gas.rs | 28 +++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/contrib/mir/src/ast/comparable.rs b/contrib/mir/src/ast/comparable.rs index b455fc741d41..d3552427c51d 100644 --- a/contrib/mir/src/ast/comparable.rs +++ b/contrib/mir/src/ast/comparable.rs @@ -5,17 +5,40 @@ impl PartialOrd for TypedValue { use TypedValue::*; match (self, other) { (Int(a), Int(b)) => a.partial_cmp(b), + (Int(..), _) => None, + (Nat(a), Nat(b)) => a.partial_cmp(b), + (Nat(..), _) => None, + (Mutez(a), Mutez(b)) => a.partial_cmp(b), + (Mutez(..), _) => None, + (Bool(a), Bool(b)) => a.partial_cmp(b), + (Bool(..), _) => None, + (String(a), String(b)) => a.partial_cmp(b), + (String(..), _) => None, + (Unit, Unit) => Some(std::cmp::Ordering::Equal), + (Unit, _) => None, + (Pair(l), Pair(r)) => l.partial_cmp(r), + (Pair(..), _) => None, + (Option(x), Option(y)) => x.as_deref().partial_cmp(&y.as_deref()), + (Option(..), _) => None, + (Or(x), Or(y)) => x.as_ref().partial_cmp(y.as_ref()), + (Or(..), _) => None, + (Address(l), Address(r)) => l.partial_cmp(r), + (Address(..), _) => None, + (ChainId(l), ChainId(r)) => l.partial_cmp(r), - _ => None, + (ChainId(..), _) => None, + + // non-comparable types + (List(..) | Map(..) | Contract(..), _) => None, } } } diff --git a/contrib/mir/src/gas.rs b/contrib/mir/src/gas.rs index b41e71bce2c0..33b740c0b47d 100644 --- a/contrib/mir/src/gas.rs +++ b/contrib/mir/src/gas.rs @@ -247,20 +247,38 @@ pub mod interpret_cost { const ADDRESS_SIZE: usize = 20 + 31; // hash size + max entrypoint size const CMP_CHAIN_ID: u32 = 30; let cmp_or = Checked::from(10u32); + #[track_caller] + fn incomparable() -> ! { + unreachable!("Comparison of incomparable values") + } Ok(match (v1, v2) { (V::Nat(l), V::Nat(r)) => { // NB: eventually when using BigInts, use BigInt::bits() &c cmp_bytes(std::mem::size_of_val(l), std::mem::size_of_val(r))? } + (V::Nat(_), _) => incomparable(), + (V::Int(l), V::Int(r)) => { // NB: eventually when using BigInts, use BigInt::bits() &c cmp_bytes(std::mem::size_of_val(l), std::mem::size_of_val(r))? } + (V::Int(_), _) => incomparable(), + (V::Bool(_), V::Bool(_)) => cmp_bytes(1, 1)?, + (V::Bool(_), _) => incomparable(), + (V::Mutez(_), V::Mutez(_)) => cmp_bytes(8, 8)?, + (V::Mutez(_), _) => incomparable(), + (V::String(l), V::String(r)) => cmp_bytes(l.len(), r.len())?, + (V::String(_), _) => incomparable(), + (V::Unit, V::Unit) => 10, + (V::Unit, _) => incomparable(), + (V::Pair(l), V::Pair(r)) => cmp_pair(l.as_ref(), r.as_ref())?, + (V::Pair(_), _) => incomparable(), + (V::Option(l), V::Option(r)) => match (l, r) { (None, None) => cmp_option, (None, Some(_)) => cmp_option, @@ -268,8 +286,14 @@ pub mod interpret_cost { (Some(l), Some(r)) => cmp_option + compare(l, r)?, } .as_gas_cost()?, + (V::Option(_), _) => incomparable(), + (V::Address(..), V::Address(..)) => cmp_bytes(ADDRESS_SIZE, ADDRESS_SIZE)?, + (V::Address(_), _) => incomparable(), + (V::ChainId(..), V::ChainId(..)) => CMP_CHAIN_ID, + (V::ChainId(_), _) => incomparable(), + (V::Or(l), V::Or(r)) => match (l.as_ref(), r.as_ref()) { (Or::Left(x), Or::Left(y)) => cmp_or + compare(x, y)?, (Or::Right(x), Or::Right(y)) => cmp_or + compare(x, y)?, @@ -277,7 +301,9 @@ pub mod interpret_cost { (Or::Right(_), Or::Left(_)) => cmp_or, } .as_gas_cost()?, - _ => unreachable!("Comparison of incomparable values"), + (V::Or(..), _) => incomparable(), + + (V::List(..) | V::Map(..) | V::Contract(_), _) => incomparable(), }) } -- GitLab