From dc7e0e54f580d43d477380cdc7f96b3027598ad0 Mon Sep 17 00:00:00 2001 From: Graydon Hoare Date: Mon, 16 Oct 2023 20:26:30 -0700 Subject: [PATCH] Add missing fixed-size metering in comparison.rs, clean up a bit --- soroban-env-host/src/host/comparison.rs | 97 ++++++++++++++++------ soroban-env-host/src/host/declared_size.rs | 27 ++++-- 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/soroban-env-host/src/host/comparison.rs b/soroban-env-host/src/host/comparison.rs index c802d9440..0dee88736 100644 --- a/soroban-env-host/src/host/comparison.rs +++ b/soroban-env-host/src/host/comparison.rs @@ -6,9 +6,11 @@ use crate::{ storage::Storage, xdr::{ AccountId, ContractCostType, ContractDataDurability, ContractExecutable, - CreateContractArgs, DepthLimiter, Duration, Hash, LedgerKey, LedgerKeyAccount, - LedgerKeyContractCode, LedgerKeyTrustLine, PublicKey, ScAddress, ScErrorCode, ScErrorType, - ScMap, ScMapEntry, ScNonceKey, ScVal, ScVec, TimePoint, TrustLineAsset, Uint256, + CreateContractArgs, DepthLimiter, Duration, Hash, Int128Parts, Int256Parts, LedgerKey, + LedgerKeyAccount, LedgerKeyContractCode, LedgerKeyContractData, LedgerKeyTrustLine, + PublicKey, ScAddress, ScContractInstance, ScError, ScErrorCode, ScErrorType, ScMap, + ScMapEntry, ScNonceKey, ScVal, ScVec, TimePoint, TrustLineAsset, UInt128Parts, + UInt256Parts, Uint256, }, Compare, Host, HostError, SymbolStr, I256, U256, }; @@ -168,12 +170,17 @@ impl_compare_fixed_size_ord_type!(i128); impl_compare_fixed_size_ord_type!(U256); impl_compare_fixed_size_ord_type!(I256); +impl_compare_fixed_size_ord_type!(Int128Parts); +impl_compare_fixed_size_ord_type!(UInt128Parts); +impl_compare_fixed_size_ord_type!(Int256Parts); +impl_compare_fixed_size_ord_type!(UInt256Parts); impl_compare_fixed_size_ord_type!(TimePoint); impl_compare_fixed_size_ord_type!(Duration); impl_compare_fixed_size_ord_type!(Hash); impl_compare_fixed_size_ord_type!(Uint256); impl_compare_fixed_size_ord_type!(ContractExecutable); impl_compare_fixed_size_ord_type!(AccountId); +impl_compare_fixed_size_ord_type!(ScError); impl_compare_fixed_size_ord_type!(ScAddress); impl_compare_fixed_size_ord_type!(ScNonceKey); impl_compare_fixed_size_ord_type!(PublicKey); @@ -254,16 +261,38 @@ impl Compare for Budget { >::compare(self, &a.as_slice(), &b.as_slice()) } - (ContractInstance(a), ContractInstance(b)) => { - let cmp = self.compare(&a.executable, &b.executable)?; - if cmp.is_eq() { - self.compare(&a.storage, &b.storage) - } else { - Ok(cmp) - } - } - - (Bool(_), _) + (ContractInstance(a), ContractInstance(b)) => self.compare(&a, &b), + + // These two cases are content-free, besides their discriminant. + (Void, Void) => Ok(Ordering::Equal), + (LedgerKeyContractInstance, LedgerKeyContractInstance) => Ok(Ordering::Equal), + + // Handle types with impl_compare_fixed_size_ord_type: + (Bool(a), Bool(b)) => self.compare(&a, &b), + (Error(a), Error(b)) => self.compare(&a, &b), + (U32(a), U32(b)) => self.compare(&a, &b), + (I32(a), I32(b)) => self.compare(&a, &b), + (U64(a), U64(b)) => self.compare(&a, &b), + (I64(a), I64(b)) => self.compare(&a, &b), + (Timepoint(a), Timepoint(b)) => self.compare(&a, &b), + (Duration(a), Duration(b)) => self.compare(&a, &b), + (U128(a), U128(b)) => self.compare(&a, &b), + (I128(a), I128(b)) => self.compare(&a, &b), + (U256(a), U256(b)) => self.compare(&a, &b), + (I256(a), I256(b)) => self.compare(&a, &b), + (Address(a), Address(b)) => self.compare(&a, &b), + (LedgerKeyNonce(a), LedgerKeyNonce(b)) => self.compare(&a, &b), + + // List out at least one side of all the remaining cases here so + // we don't accidentally forget to update this when/if a new + // ScVal type is added. + (Vec(_), _) + | (Map(_), _) + | (Bytes(_), _) + | (String(_), _) + | (Symbol(_), _) + | (ContractInstance(_), _) + | (Bool(_), _) | (Void, _) | (Error(_), _) | (U32(_), _) @@ -276,19 +305,40 @@ impl Compare for Budget { | (I128(_), _) | (U256(_), _) | (I256(_), _) - | (Bytes(_), _) - | (String(_), _) - | (Symbol(_), _) - | (Vec(_), _) - | (Map(_), _) | (Address(_), _) | (LedgerKeyContractInstance, _) - | (LedgerKeyNonce(_), _) - | (ContractInstance(_), _) => Ok(a.cmp(b)), + | (LedgerKeyNonce(_), _) => Ok(a.discriminant().cmp(&b.discriminant())), }) } } +impl Compare for Budget { + type Error = HostError; + + fn compare( + &self, + a: &ScContractInstance, + b: &ScContractInstance, + ) -> Result { + self.compare(&(&a.executable, &a.storage), &(&b.executable, &b.storage)) + } +} + +impl Compare for Budget { + type Error = HostError; + + fn compare( + &self, + a: &LedgerKeyContractData, + b: &LedgerKeyContractData, + ) -> Result { + self.compare( + &(&a.contract, &a.key, &a.durability), + &(&b.contract, &b.key, &b.durability), + ) + } +} + impl Compare for Budget { type Error = HostError; @@ -299,10 +349,7 @@ impl Compare for Budget { match (a, b) { (Account(a), Account(b)) => self.compare(&a, &b), (Trustline(a), Trustline(b)) => self.compare(&a, &b), - (ContractData(a), ContractData(b)) => self.compare( - &(&a.contract, &a.key, &a.durability), - &(&b.contract, &b.key, &b.durability), - ), + (ContractData(a), ContractData(b)) => self.compare(&a, &b), (ContractCode(a), ContractCode(b)) => self.compare(&a, &b), // All these cases should have been rejected above by check_supported_ledger_key_type. @@ -323,7 +370,7 @@ impl Compare for Budget { // we remember to update this code if LedgerKey changes. We don't // charge for these since they're just 1-integer compares. (Account(_), _) | (Trustline(_), _) | (ContractData(_), _) | (ContractCode(_), _) => { - Ok(a.cmp(b)) + Ok(a.discriminant().cmp(&b.discriminant())) } } } diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index c87b71bfa..619cf11e9 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -11,11 +11,12 @@ use crate::{ xdr::{ AccountEntry, AccountId, Asset, BytesM, ContractCodeEntry, ContractDataDurability, ContractEvent, ContractExecutable, ContractIdPreimage, CreateContractArgs, Duration, - ExtensionPoint, Hash, LedgerEntry, LedgerEntryExt, LedgerKey, LedgerKeyAccount, - LedgerKeyContractCode, LedgerKeyTrustLine, PublicKey, ScAddress, ScBytes, - ScContractInstance, ScMap, ScMapEntry, ScNonceKey, ScString, ScSymbol, ScVal, ScVec, - Signer, SorobanAuthorizationEntry, SorobanAuthorizedInvocation, StringM, TimePoint, - TrustLineAsset, TrustLineEntry, TtlEntry, Uint256, SCSYMBOL_LIMIT, + ExtensionPoint, Hash, Int128Parts, Int256Parts, LedgerEntry, LedgerEntryExt, LedgerKey, + LedgerKeyAccount, LedgerKeyContractCode, LedgerKeyTrustLine, PublicKey, ScAddress, ScBytes, + ScContractInstance, ScError, ScMap, ScMapEntry, ScNonceKey, ScString, ScSymbol, ScVal, + ScVec, Signer, SorobanAuthorizationEntry, SorobanAuthorizedInvocation, StringM, TimePoint, + TrustLineAsset, TrustLineEntry, TtlEntry, UInt128Parts, UInt256Parts, Uint256, + SCSYMBOL_LIMIT, }, AddressObject, Bool, BytesObject, DurationObject, DurationSmall, DurationVal, Error, HostError, I128Object, I128Small, I128Val, I256Object, I256Small, I256Val, I32Val, I64Object, I64Small, @@ -121,6 +122,11 @@ impl_declared_size_type!(PublicKey, 32); impl_declared_size_type!(TrustLineAsset, 45); impl_declared_size_type!(Signer, 72); +impl_declared_size_type!(Int128Parts, 16); +impl_declared_size_type!(UInt128Parts, 16); +impl_declared_size_type!(Int256Parts, 32); +impl_declared_size_type!(UInt256Parts, 32); + impl_declared_size_type!(LedgerKeyAccount, 32); impl_declared_size_type!(LedgerKeyTrustLine, 77); impl_declared_size_type!(LedgerKeyContractCode, 36); @@ -146,6 +152,7 @@ impl_declared_size_type!(EventError, 1); impl_declared_size_type!(ScBytes, 24); impl_declared_size_type!(ScString, 24); impl_declared_size_type!(ScSymbol, 24); +impl_declared_size_type!(ScError, 8); impl_declared_size_type!(CreateContractArgs, 98); impl_declared_size_type!(ContractIdPreimage, 65); impl_declared_size_type!(ContractDataDurability, 4); @@ -308,6 +315,10 @@ mod test { expect!["24"].assert_eq(size_of::().to_string().as_str()); expect!["32"].assert_eq(size_of::().to_string().as_str()); expect!["32"].assert_eq(size_of::().to_string().as_str()); + expect!["16"].assert_eq(size_of::().to_string().as_str()); + expect!["16"].assert_eq(size_of::().to_string().as_str()); + expect!["32"].assert_eq(size_of::().to_string().as_str()); + expect!["32"].assert_eq(size_of::().to_string().as_str()); expect!["33"].assert_eq(size_of::().to_string().as_str()); expect!["32"].assert_eq(size_of::().to_string().as_str()); expect!["33"].assert_eq(size_of::().to_string().as_str()); @@ -335,6 +346,7 @@ mod test { expect!["24"].assert_eq(size_of::().to_string().as_str()); expect!["24"].assert_eq(size_of::().to_string().as_str()); expect!["24"].assert_eq(size_of::().to_string().as_str()); + expect!["8"].assert_eq(size_of::().to_string().as_str()); expect!["98"].assert_eq(size_of::().to_string().as_str()); expect!["65"].assert_eq(size_of::().to_string().as_str()); expect!["4"].assert_eq(size_of::().to_string().as_str()); @@ -466,6 +478,10 @@ mod test { assert_mem_size_le_declared_size!(ScMap); assert_mem_size_le_declared_size!(Hash); assert_mem_size_le_declared_size!(Uint256); + assert_mem_size_le_declared_size!(Int256Parts); + assert_mem_size_le_declared_size!(UInt256Parts); + assert_mem_size_le_declared_size!(Int128Parts); + assert_mem_size_le_declared_size!(UInt128Parts); assert_mem_size_le_declared_size!(ContractExecutable); assert_mem_size_le_declared_size!(AccountId); assert_mem_size_le_declared_size!(ScAddress); @@ -492,6 +508,7 @@ mod test { assert_mem_size_le_declared_size!(ScBytes); assert_mem_size_le_declared_size!(ScString); assert_mem_size_le_declared_size!(ScSymbol); + assert_mem_size_le_declared_size!(ScError); assert_mem_size_le_declared_size!(CreateContractArgs); assert_mem_size_le_declared_size!(ContractDataDurability); assert_mem_size_le_declared_size!(ExtensionPoint);