From 0713c87797dba0a9fbf83b79fc9be9a3e6cdee38 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Mon, 15 Apr 2024 18:42:21 -0400 Subject: [PATCH 01/21] Add new `Hash` type and adapt existing interfaces to it --- soroban-sdk/src/auth.rs | 4 +-- soroban-sdk/src/crypto.rs | 59 +++++++++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/soroban-sdk/src/auth.rs b/soroban-sdk/src/auth.rs index 40c0b4cc8..7cac83494 100644 --- a/soroban-sdk/src/auth.rs +++ b/soroban-sdk/src/auth.rs @@ -1,6 +1,6 @@ //! Auth contains types for building custom account contracts. -use crate::{contracttype, Address, BytesN, Env, Error, Symbol, Val, Vec}; +use crate::{contracttype, crypto::Hash, Address, BytesN, Env, Error, Symbol, Val, Vec}; /// Context of a single authorized call performed by an address. /// @@ -77,7 +77,7 @@ pub trait CustomAccountInterface { /// Check that the signatures and auth contexts are valid. fn __check_auth( env: Env, - signature_payload: BytesN<32>, + signature_payload: Hash<32>, signatures: Self::Signature, auth_contexts: Vec, ) -> Result<(), Self::Error>; diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 215e067b3..66948d4b3 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -1,5 +1,54 @@ //! Crypto contains functions for cryptographic functions. -use crate::{env::internal, unwrap::UnwrapInfallible, Bytes, BytesN, Env}; + +use crate::{ + env::internal, unwrap::UnwrapInfallible, Bytes, BytesN, ConversionError, Env, IntoVal, + TryFromVal, Val, +}; + +/// A wrapper type for a cryptographic hash. +/// +/// This struct is designed to be used in contexts where a hash value generated +/// by a secure cryptographic function is required. +pub struct Hash(BytesN); + +impl Hash { + /// Constructs a new `Hash` from a fixed-length bytes array. + pub fn from_bytes(bytes: BytesN) -> Self { + Self(bytes) + } +} + +impl IntoVal for Hash { + fn into_val(&self, e: &Env) -> Val { + self.0.into_val(e) + } +} + +impl IntoVal> for Hash { + fn into_val(&self, _e: &Env) -> BytesN { + self.0.clone() + } +} + +impl Into> for Hash { + fn into(self) -> BytesN { + self.0 + } +} + +impl Into<[u8; N]> for Hash { + fn into(self) -> [u8; N] { + self.0.into() + } +} + +impl TryFromVal for Hash { + type Error = ConversionError; + + fn try_from_val(env: &Env, v: &Val) -> Result { + Ok(Hash(BytesN::::try_from_val(env, v)?)) + } +} /// Crypto provides access to cryptographic functions. pub struct Crypto { @@ -16,17 +65,17 @@ impl Crypto { } /// Returns the SHA-256 hash of the data. - pub fn sha256(&self, data: &Bytes) -> BytesN<32> { + pub fn sha256(&self, data: &Bytes) -> Hash<32> { let env = self.env(); let bin = internal::Env::compute_hash_sha256(env, data.into()).unwrap_infallible(); - unsafe { BytesN::unchecked_new(env.clone(), bin) } + unsafe { Hash(BytesN::unchecked_new(env.clone(), bin)) } } /// Returns the Keccak-256 hash of the data. - pub fn keccak256(&self, data: &Bytes) -> BytesN<32> { + pub fn keccak256(&self, data: &Bytes) -> Hash<32> { let env = self.env(); let bin = internal::Env::compute_hash_keccak256(env, data.into()).unwrap_infallible(); - unsafe { BytesN::unchecked_new(env.clone(), bin) } + unsafe { Hash(BytesN::unchecked_new(env.clone(), bin)) } } /// Verifies an ed25519 signature. From 22591b0579b80a6b0345a61c36773f7279a98200 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Mon, 15 Apr 2024 18:47:52 -0400 Subject: [PATCH 02/21] Add secp256r1 sig verify, split crypto interface into "safe" (using `Hash` type) and "hazmat" remove mistakenly checked-in test observation file --- soroban-sdk/src/crypto.rs | 77 +++++++++++++++++++++++ soroban-sdk/src/env.rs | 20 +++++- soroban-sdk/src/tests.rs | 1 + soroban-sdk/src/tests/crypto_keccak256.rs | 5 +- soroban-sdk/src/tests/crypto_secp256k1.rs | 6 +- soroban-sdk/src/tests/crypto_secp256r1.rs | 25 ++++++++ soroban-sdk/src/tests/crypto_sha256.rs | 5 +- 7 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 soroban-sdk/src/tests/crypto_secp256r1.rs diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 66948d4b3..6a56bfce0 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -101,6 +101,59 @@ impl Crypto { /// The public key returned is the SEC-1-encoded ECDSA secp256k1 public key /// that produced the 64-byte signature over a given 32-byte message digest, /// for a given recovery_id byte. + pub fn secp256k1_recover( + &self, + message_digest: &Hash<32>, + signature: &BytesN<64>, + recorvery_id: u32, + ) -> BytesN<65> { + let env = self.env(); + CryptoHazmat::new(env).secp256k1_recover(&message_digest.0, signature, recorvery_id) + } + + /// Verifies the ECDSA secp256r1 signature. + /// + /// The SEC-1-encoded public key is provided along with the message, + /// verifies the 64-byte signature. + pub fn secp256r1_verify( + &self, + public_key: &BytesN<65>, + message_digest: &Hash<32>, + signature: &BytesN<64>, + ) { + let env = self.env(); + CryptoHazmat::new(env).secp256r1_verify(public_key, &message_digest.0, signature) + } +} + +/// Hazardous Materials +/// +/// Cryptographic functions under [CryptoHazmat] are low-leveled which can be +/// insecure if misused. They are not generally recommended. Using them +/// incorrectly can introduce security vulnerabilities. Please use [Crypto] if +/// possible. +pub struct CryptoHazmat { + env: Env, +} + +impl CryptoHazmat { + pub(crate) fn new(env: &Env) -> CryptoHazmat { + CryptoHazmat { env: env.clone() } + } + + pub fn env(&self) -> &Env { + &self.env + } + + /// Recovers the ECDSA secp256k1 public key. + /// + /// The public key returned is the SEC-1-encoded ECDSA secp256k1 public key + /// that produced the 64-byte signature over a given 32-byte message digest, + /// for a given recovery_id byte. + /// + /// WARNING: The `message_digest` must be produced by a secure cryptographic + /// hash function on the message, otherwise the attacker can potentially + /// forge signatures. pub fn secp256k1_recover( &self, message_digest: &BytesN<32>, @@ -117,4 +170,28 @@ impl Crypto { .unwrap_infallible(); unsafe { BytesN::unchecked_new(env.clone(), bytes) } } + + /// Verifies the ECDSA secp256r1 signature. + /// + /// The SEC-1-encoded public key is provided along with a 32-byte message + /// digest, verifies the 64-byte signature. + /// + /// WARNING: The `message_digest` must be produced by a secure cryptographic + /// hash function on the message, otherwise the attacker can potentially + /// forge signatures. + pub fn secp256r1_verify( + &self, + public_key: &BytesN<65>, + message_digest: &BytesN<32>, + signature: &BytesN<64>, + ) { + let env = self.env(); + let _ = internal::Env::verify_sig_ecdsa_secp256r1( + env, + public_key.to_object(), + message_digest.to_object(), + signature.to_object(), + ) + .unwrap_infallible(); + } } diff --git a/soroban-sdk/src/env.rs b/soroban-sdk/src/env.rs index 954122c93..4de8a3c9d 100644 --- a/soroban-sdk/src/env.rs +++ b/soroban-sdk/src/env.rs @@ -117,8 +117,14 @@ use crate::unwrap::UnwrapInfallible; use crate::unwrap::UnwrapOptimized; use crate::InvokeError; use crate::{ - crypto::Crypto, deploy::Deployer, events::Events, ledger::Ledger, logs::Logs, prng::Prng, - storage::Storage, Address, Vec, + crypto::{Crypto, CryptoHazmat}, + deploy::Deployer, + events::Events, + ledger::Ledger, + logs::Logs, + prng::Prng, + storage::Storage, + Address, Vec, }; use internal::{ AddressObject, Bool, BytesObject, DurationObject, I128Object, I256Object, I256Val, I64Object, @@ -313,6 +319,16 @@ impl Env { Crypto::new(self) } + /// Hazardous Materials + /// + /// Get a [CryptoHazmat] for accessing the cryptographic functions that are + /// not generally recommended. Using them incorrectly can introduce security + /// vulnerabilities. Please use [Crypto] if possible. + #[inline(always)] + pub fn crypto_hazmat(&self) -> CryptoHazmat { + CryptoHazmat::new(self) + } + /// Get a [Prng] for accessing the current functions which provide pseudo-randomness. /// /// # Warning diff --git a/soroban-sdk/src/tests.rs b/soroban-sdk/src/tests.rs index 1c1f6bdb6..fe43b9eb1 100644 --- a/soroban-sdk/src/tests.rs +++ b/soroban-sdk/src/tests.rs @@ -24,6 +24,7 @@ mod contractimport_with_error; mod crypto_ed25519; mod crypto_keccak256; mod crypto_secp256k1; +mod crypto_secp256r1; mod crypto_sha256; mod env; mod max_ttl; diff --git a/soroban-sdk/src/tests/crypto_keccak256.rs b/soroban-sdk/src/tests/crypto_keccak256.rs index 932c9ae4a..1a3728647 100644 --- a/soroban-sdk/src/tests/crypto_keccak256.rs +++ b/soroban-sdk/src/tests/crypto_keccak256.rs @@ -1,4 +1,4 @@ -use crate::{bytesn, Env, IntoVal}; +use crate::{bytesn, BytesN, Env, IntoVal}; #[test] fn test_keccak256() { @@ -9,5 +9,6 @@ fn test_keccak256() { &env, 0x352fe2eaddf44eb02eb3eab1f8d6ff4ba426df4f1734b1e3f210d621ee8853d9 ); - assert_eq!(env.crypto().keccak256(&bytes), expect); + let hash: BytesN<32> = env.crypto().keccak256(&bytes).into(); + assert_eq!(hash, expect); } diff --git a/soroban-sdk/src/tests/crypto_secp256k1.rs b/soroban-sdk/src/tests/crypto_secp256k1.rs index 250d93327..b4c6fc2ee 100644 --- a/soroban-sdk/src/tests/crypto_secp256k1.rs +++ b/soroban-sdk/src/tests/crypto_secp256k1.rs @@ -1,14 +1,14 @@ -use crate::{bytesn, Env}; +use crate::{bytesn, crypto::Hash, Env}; #[test] fn test_recover_key_ecdsa_secp256k1() { let env = Env::default(); // From: https://github.com/ethereum/go-ethereum/blob/90d5bd85bcf2919ac2735a47fde675213348a0a6/crypto/secp256k1/secp256_test.go#L204-L217 - let message_digest = bytesn!( + let message_digest = Hash::from_bytes(bytesn!( &env, 0xce0677bb30baa8cf067c88db9811f4333d131bf8bcf12fe7065d211dce971008 - ); + )); let signature = bytesn!( &env, 0x90f27b8b488db00b00606796d2987f6a5f59ae62ea05effe84fef5b8b0e549984a691139ad57a3f0b906637673aa2f63d1f55cb1a69199d4009eea23ceaddc93 diff --git a/soroban-sdk/src/tests/crypto_secp256r1.rs b/soroban-sdk/src/tests/crypto_secp256r1.rs new file mode 100644 index 000000000..ef2499f8d --- /dev/null +++ b/soroban-sdk/src/tests/crypto_secp256r1.rs @@ -0,0 +1,25 @@ +use crate::{bytesn, crypto::Hash, Env}; + +#[test] +fn test_verify_sig_ecdsa_secp256r1() { + let env = Env::default(); + + // Test vector copied and adapted from + // https://csrc.nist.gov/groups/STM/cavp/documents/dss/186-3ecdsatestvectors.zip + // `SigVer.rsp` section [P-256,SHA-256] + let message_digest = Hash::from_bytes(bytesn!( + &env, + 0xd1b8ef21eb4182ee270638061063a3f3c16c114e33937f69fb232cc833965a94 + )); + let signature = bytesn!( + &env, + 0xbf96b99aa49c705c910be33142017c642ff540c76349b9dab72f981fd9347f4f17c55095819089c2e03b9cd415abdf12444e323075d98f31920b9e0f57ec871c + ); + let public_key = bytesn!( + &env, + 0x04e424dc61d4bb3cb7ef4344a7f8957a0c5134e16f7a67c074f82e6e12f49abf3c970eed7aa2bc48651545949de1dddaf0127e5965ac85d1243d6f60e7dfaee927 + ); + + env.crypto() + .secp256r1_verify(&public_key, &message_digest, &signature) +} diff --git a/soroban-sdk/src/tests/crypto_sha256.rs b/soroban-sdk/src/tests/crypto_sha256.rs index 9c48c536f..97cb19511 100644 --- a/soroban-sdk/src/tests/crypto_sha256.rs +++ b/soroban-sdk/src/tests/crypto_sha256.rs @@ -1,4 +1,4 @@ -use crate::{bytes, bytesn, Env}; +use crate::{bytes, bytesn, BytesN, Env}; #[test] fn test_sha256() { @@ -9,5 +9,6 @@ fn test_sha256() { &env, 0x4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a ); - assert_eq!(env.crypto().sha256(&input), expect); + let hash: BytesN<32> = env.crypto().sha256(&input).into(); + assert_eq!(hash, expect); } From 1c0e4b7ca8364321200a8c8e31cb644a55c07763 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Mon, 15 Apr 2024 19:32:36 -0400 Subject: [PATCH 03/21] Make `Hash` not construct-able --- soroban-sdk/src/crypto.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 6a56bfce0..12d0dfe87 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -8,11 +8,17 @@ use crate::{ /// A wrapper type for a cryptographic hash. /// /// This struct is designed to be used in contexts where a hash value generated -/// by a secure cryptographic function is required. +/// by a secure cryptographic function is required. It can only be constructed +/// via secure manners, i.e. output from a secure hash function, or received +/// from the host (e.g. via `CustomAccountInterface`) pub struct Hash(BytesN); impl Hash { /// Constructs a new `Hash` from a fixed-length bytes array. + /// + /// This is intended for test-only, since `Hash` type is only meant to be + /// constructed via secure manners. + #[cfg(test)] pub fn from_bytes(bytes: BytesN) -> Self { Self(bytes) } From c5550c6d7df761759d2ef94aea1d0da23003d7d7 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Tue, 16 Apr 2024 18:35:47 -0400 Subject: [PATCH 04/21] reject `Hash` as public user function input --- soroban-sdk-macros/src/derive_fn.rs | 24 +++++++++++++++++++++++- soroban-sdk-macros/src/derive_spec_fn.rs | 9 +++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index f83c20a61..76104d690 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -54,7 +54,29 @@ pub fn derive_fn( .skip(if env_input.is_some() { 1 } else { 0 }) .enumerate() .map(|(i, a)| match a { - FnArg::Typed(_) => { + FnArg::Typed(pat_ty) => { + if ident != "__check_auth" { + let mut ty = &*pat_ty.ty; + if let Type::Reference(TypeReference { elem, .. }) = ty { + ty = elem; + } + if let Type::Path(TypePath { + path: syn::Path { segments, .. }, + .. + }) = ty + { + if segments.last().map_or(false, |s| s.ident == "Hash" && !s.arguments.is_none()) { + errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, + since there is no guarantee the received input is from a secure hash function. + If you still intend to use a hash with such a guarantee, please use `ByteN`")); + } else { + () + } + } else { + () + } + } + let ident = format_ident!("arg_{}", i); let arg = FnArg::Typed(PatType { attrs: vec![], diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index 962c2ad9e..86cc12290 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -71,6 +71,15 @@ pub fn derive_fn_spec( )); StringM::::default() }); + + if let ScSpecTypeDef::Hash(_) = type_ { + if ident != "__check_auth" { + errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, + since there is no guarantee the received input is from a secure hash function. + If you still intend to use a hash with such a guarantee, please use `ByteN`")); + } + } + ScSpecFunctionInputV0 { doc: "".try_into().unwrap(), name, From f394cfe55982c817bc87939f399ffd934b66c113 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Tue, 16 Apr 2024 21:08:15 -0400 Subject: [PATCH 05/21] tighen the allowed usage of `Hash` --- soroban-sdk-macros/src/derive_enum.rs | 9 ++++- soroban-sdk-macros/src/derive_fn.rs | 37 +++++++----------- soroban-sdk-macros/src/derive_spec_fn.rs | 13 +------ soroban-sdk-macros/src/derive_struct.rs | 8 +++- soroban-sdk-macros/src/derive_struct_tuple.rs | 7 +++- soroban-sdk-macros/src/lib.rs | 5 ++- soroban-sdk-macros/src/map_type.rs | 39 ++++++++++++------- 7 files changed, 63 insertions(+), 55 deletions(-) diff --git a/soroban-sdk-macros/src/derive_enum.rs b/soroban-sdk-macros/src/derive_enum.rs index c7eb592e8..c952fcea7 100644 --- a/soroban-sdk-macros/src/derive_enum.rs +++ b/soroban-sdk-macros/src/derive_enum.rs @@ -85,6 +85,7 @@ pub fn derive_type_enum( into_xdr, } = map_tuple_variant( path, + &vis, enum_ident, &case_num_lit, &case_name_str_lit, @@ -333,6 +334,7 @@ fn map_empty_variant( fn map_tuple_variant( path: &Path, + vis: &Visibility, enum_ident: &Ident, case_num_lit: &Literal, case_name_str_lit: &Literal, @@ -343,9 +345,14 @@ fn map_tuple_variant( errors: &mut Vec, ) -> VariantTokens { let spec_case = { + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let field_types = fields .iter() - .map(|f| match map_type(&f.ty) { + .map(|f| match map_type(&f.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index 76104d690..f0247b788 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -1,3 +1,4 @@ +use crate::map_type::map_type; use itertools::MultiUnzip; use proc_macro2::TokenStream as TokenStream2; use quote::{format_ident, quote}; @@ -6,22 +7,31 @@ use syn::{ punctuated::Punctuated, spanned::Spanned, token::{Colon, Comma}, - Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, Type, TypePath, TypeReference, + Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, ReturnType, Type, TypePath, + TypeReference, }; #[allow(clippy::too_many_arguments)] -pub fn derive_fn( +pub fn derive_pub_fn( crate_path: &Path, call: &TokenStream2, ident: &Ident, attrs: &[Attribute], inputs: &Punctuated, + output: &ReturnType, trait_ident: Option<&Ident>, client_ident: &str, ) -> Result { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); + let allow_hash = ident == "__check_auth"; + if let ReturnType::Type(_, ty) = output { + if let Err(e) = map_type(ty, allow_hash) { + errors.push(e); + } + } + // Prepare the env input. let env_input = inputs.first().and_then(|a| match a { FnArg::Typed(pat_type) => { @@ -55,28 +65,9 @@ pub fn derive_fn( .enumerate() .map(|(i, a)| match a { FnArg::Typed(pat_ty) => { - if ident != "__check_auth" { - let mut ty = &*pat_ty.ty; - if let Type::Reference(TypeReference { elem, .. }) = ty { - ty = elem; - } - if let Type::Path(TypePath { - path: syn::Path { segments, .. }, - .. - }) = ty - { - if segments.last().map_or(false, |s| s.ident == "Hash" && !s.arguments.is_none()) { - errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, - since there is no guarantee the received input is from a secure hash function. - If you still intend to use a hash with such a guarantee, please use `ByteN`")); - } else { - () - } - } else { - () - } + if let Err(e) = map_type(&pat_ty.ty, allow_hash) { + errors.push(e); } - let ident = format_ident!("arg_{}", i); let arg = FnArg::Typed(PatType { attrs: vec![], diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index 86cc12290..1d7db9c85 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -61,7 +61,7 @@ pub fn derive_fn_spec( errors.push(Error::new(a.span(), "argument not supported")); "".to_string() }; - match map_type(&pat_type.ty) { + match map_type(&pat_type.ty, ident == "__check_auth") { Ok(type_) => { let name = name.try_into().unwrap_or_else(|_| { const MAX: u32 = 30; @@ -71,15 +71,6 @@ pub fn derive_fn_spec( )); StringM::::default() }); - - if let ScSpecTypeDef::Hash(_) = type_ { - if ident != "__check_auth" { - errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, - since there is no guarantee the received input is from a secure hash function. - If you still intend to use a hash with such a guarantee, please use `ByteN`")); - } - } - ScSpecFunctionInputV0 { doc: "".try_into().unwrap(), name, @@ -109,7 +100,7 @@ pub fn derive_fn_spec( // Prepare the output. let spec_result = match output { - ReturnType::Type(_, ty) => vec![match map_type(ty) { + ReturnType::Type(_, ty) => vec![match map_type(ty, false) { Ok(spec) => spec, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_struct.rs b/soroban-sdk-macros/src/derive_struct.rs index a35f9d9c5..2b51e3b66 100644 --- a/soroban-sdk-macros/src/derive_struct.rs +++ b/soroban-sdk-macros/src/derive_struct.rs @@ -25,7 +25,11 @@ pub fn derive_type_struct( ) -> TokenStream2 { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); - + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let fields = &data.fields; let field_count_usize: usize = fields.len(); let (spec_fields, field_idents, field_names, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields @@ -43,7 +47,7 @@ pub fn derive_type_struct( errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {MAX}", field_name.len()))); StringM::::default() }), - type_: match map_type(&field.ty) { + type_: match map_type(&field.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_struct_tuple.rs b/soroban-sdk-macros/src/derive_struct_tuple.rs index a41fe2a4a..a6b6ccdd8 100644 --- a/soroban-sdk-macros/src/derive_struct_tuple.rs +++ b/soroban-sdk-macros/src/derive_struct_tuple.rs @@ -24,6 +24,11 @@ pub fn derive_type_struct_tuple( let fields = &data.fields; let field_count_usize: usize = fields.len(); + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let (field_specs, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields .iter() @@ -36,7 +41,7 @@ pub fn derive_type_struct_tuple( let field_spec = ScSpecUdtStructFieldV0 { doc: docs_from_attrs(&field.attrs).try_into().unwrap(), // TODO: Truncate docs, or display friendly compile error. name: field_name.try_into().unwrap_or_else(|_| StringM::default()), - type_: match map_type(&field.ty) { + type_: match map_type(&field.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/lib.rs b/soroban-sdk-macros/src/lib.rs index 048bc5f4b..29d471401 100644 --- a/soroban-sdk-macros/src/lib.rs +++ b/soroban-sdk-macros/src/lib.rs @@ -18,7 +18,7 @@ use derive_client::{derive_client_impl, derive_client_type}; use derive_enum::derive_type_enum; use derive_enum_int::derive_type_enum_int; use derive_error_enum_int::derive_type_error_enum_int; -use derive_fn::{derive_contract_function_registration_ctor, derive_fn}; +use derive_fn::{derive_contract_function_registration_ctor, derive_pub_fn}; use derive_spec_fn::derive_fn_spec; use derive_struct::derive_type_struct; use derive_struct_tuple::derive_type_struct_tuple; @@ -231,12 +231,13 @@ pub fn contractimpl(metadata: TokenStream, input: TokenStream) -> TokenStream { .map(|m| { let ident = &m.sig.ident; let call = quote! { ::#ident }; - derive_fn( + derive_pub_fn( &crate_path, &call, ident, &m.attrs, &m.sig.inputs, + &m.sig.output, trait_ident, &client_ident, ) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index 3d0b6c6ee..fd234e0ff 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -9,7 +9,7 @@ use syn::{ }; #[allow(clippy::too_many_lines)] -pub fn map_type(t: &Type) -> Result { +pub fn map_type(t: &Type, allow_hash: bool) -> Result { match t { Type::Path(TypePath { qself: None, @@ -61,8 +61,8 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Result(Box::new(ScSpecTypeResult { - ok_type: Box::new(map_type(ok)?), - error_type: Box::new(map_type(err)?), + ok_type: Box::new(map_type(ok, allow_hash)?), + error_type: Box::new(map_type(err, allow_hash)?), }))) } "Option" => { @@ -74,7 +74,7 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Option(Box::new(ScSpecTypeOption { - value_type: Box::new(map_type(t)?), + value_type: Box::new(map_type(t, allow_hash)?), }))) } "Vec" => { @@ -86,7 +86,7 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Vec(Box::new(ScSpecTypeVec { - element_type: Box::new(map_type(t)?), + element_type: Box::new(map_type(t, allow_hash)?), }))) } "Map" => { @@ -98,8 +98,8 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Map(Box::new(ScSpecTypeMap { - key_type: Box::new(map_type(k)?), - value_type: Box::new(map_type(v)?), + key_type: Box::new(map_type(k, allow_hash)?), + value_type: Box::new(map_type(v, allow_hash)?), }))) } "BytesN" => { @@ -113,14 +113,21 @@ pub fn map_type(t: &Type) -> Result { Ok(ScSpecTypeDef::BytesN(ScSpecTypeBytesN { n })) } "Hash" => { - let n = match args.as_slice() { - [GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?, - [..] => Err(Error::new( + if allow_hash { + let n = match args.as_slice() { + [GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?, + [..] => Err(Error::new( + t.span(), + "incorrect number of generic arguments, expect one for BytesN", + ))?, + }; + Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) + } else { + Err(Error::new( t.span(), - "incorrect number of generic arguments, expect one for BytesN", - ))?, - }; - Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) + "Hash cannot be used in contexts where there is no guarantee it comes from a secure hash function", + )) + } } _ => Err(Error::new( angle_bracketed.span(), @@ -132,10 +139,12 @@ pub fn map_type(t: &Type) -> Result { } } Type::Tuple(TypeTuple { elems, .. }) => { + let map_type_reject_hash = + |t: &Type| -> Result { map_type(t, false) }; Ok(ScSpecTypeDef::Tuple(Box::new(ScSpecTypeTuple { value_types: elems .iter() - .map(map_type) + .map(map_type_reject_hash) .collect::, Error>>()? // TODO: Implement conversion to VecM from iters to omit this collect. .try_into() .map_err(|e| { From a4eef4a073bd6be9a919b533c8ae70cfa5b645ea Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:26:38 +1000 Subject: [PATCH 06/21] tweak doc --- soroban-sdk/src/crypto.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 12d0dfe87..c4d87f77b 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -5,12 +5,12 @@ use crate::{ TryFromVal, Val, }; -/// A wrapper type for a cryptographic hash. +/// A Bytes generated by a cryptographic hash function. /// /// This struct is designed to be used in contexts where a hash value generated /// by a secure cryptographic function is required. It can only be constructed /// via secure manners, i.e. output from a secure hash function, or received -/// from the host (e.g. via `CustomAccountInterface`) +/// from the Soroban Env (e.g. via [`CustomAccountInterface`][crate::auth::CustomAccountInterface]) pub struct Hash(BytesN); impl Hash { From 279f33d82588aeab52d392283b6407472f51b2cf Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:42:59 +1000 Subject: [PATCH 07/21] fix type name --- soroban-sdk-macros/src/map_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index fd234e0ff..f79e28ed7 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -118,7 +118,7 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { [GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?, [..] => Err(Error::new( t.span(), - "incorrect number of generic arguments, expect one for BytesN", + "incorrect number of generic arguments, expect one for Hash", ))?, }; Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) From 709c8e3c388154c08a9c2286399094b348e891b2 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 13:43:10 +1000 Subject: [PATCH 08/21] remove double negatives from compile error --- soroban-sdk-macros/src/map_type.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index f79e28ed7..866e37eaf 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -125,7 +125,7 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { } else { Err(Error::new( t.span(), - "Hash cannot be used in contexts where there is no guarantee it comes from a secure hash function", + "Hash can only be used in contexts where there is a guarantee that the hash has been sourced from a secure cryptographic hash function", )) } } From 7f2f6688efe5dbd0fd984f4ca6befd17838065bf Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 20:44:04 +1000 Subject: [PATCH 09/21] disallow hash on contracttypes --- soroban-sdk-macros/src/derive_enum.rs | 9 +-------- soroban-sdk-macros/src/derive_struct_tuple.rs | 7 +------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/soroban-sdk-macros/src/derive_enum.rs b/soroban-sdk-macros/src/derive_enum.rs index c952fcea7..f52a448b2 100644 --- a/soroban-sdk-macros/src/derive_enum.rs +++ b/soroban-sdk-macros/src/derive_enum.rs @@ -85,7 +85,6 @@ pub fn derive_type_enum( into_xdr, } = map_tuple_variant( path, - &vis, enum_ident, &case_num_lit, &case_name_str_lit, @@ -334,7 +333,6 @@ fn map_empty_variant( fn map_tuple_variant( path: &Path, - vis: &Visibility, enum_ident: &Ident, case_num_lit: &Literal, case_name_str_lit: &Literal, @@ -345,14 +343,9 @@ fn map_tuple_variant( errors: &mut Vec, ) -> VariantTokens { let spec_case = { - let allow_hash = if let Visibility::Public(_) = vis { - false - } else { - true - }; let field_types = fields .iter() - .map(|f| match map_type(&f.ty, allow_hash) { + .map(|f| match map_type(&f.ty, false) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_struct_tuple.rs b/soroban-sdk-macros/src/derive_struct_tuple.rs index a6b6ccdd8..5a41870cc 100644 --- a/soroban-sdk-macros/src/derive_struct_tuple.rs +++ b/soroban-sdk-macros/src/derive_struct_tuple.rs @@ -24,11 +24,6 @@ pub fn derive_type_struct_tuple( let fields = &data.fields; let field_count_usize: usize = fields.len(); - let allow_hash = if let Visibility::Public(_) = vis { - false - } else { - true - }; let (field_specs, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields .iter() @@ -41,7 +36,7 @@ pub fn derive_type_struct_tuple( let field_spec = ScSpecUdtStructFieldV0 { doc: docs_from_attrs(&field.attrs).try_into().unwrap(), // TODO: Truncate docs, or display friendly compile error. name: field_name.try_into().unwrap_or_else(|_| StringM::default()), - type_: match map_type(&field.ty, allow_hash) { + type_: match map_type(&field.ty, false) { Ok(t) => t, Err(e) => { errors.push(e); From d835f4c135515945c90b8eecca9177f5a8b90dba Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 20:44:16 +1000 Subject: [PATCH 10/21] map hash in contract fns to bytesn --- soroban-sdk-macros/src/map_type.rs | 4 ++-- soroban-spec-rust/src/types.rs | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index 866e37eaf..d0f7e8a27 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -1,7 +1,7 @@ use stellar_xdr::curr as stellar_xdr; use stellar_xdr::{ ScSpecTypeBytesN, ScSpecTypeDef, ScSpecTypeMap, ScSpecTypeOption, ScSpecTypeResult, - ScSpecTypeTuple, ScSpecTypeUdt, ScSpecTypeVec, ScSpectTypeHash, + ScSpecTypeTuple, ScSpecTypeUdt, ScSpecTypeVec, }; use syn::{ spanned::Spanned, Error, Expr, ExprLit, GenericArgument, Lit, Path, PathArguments, PathSegment, @@ -121,7 +121,7 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { "incorrect number of generic arguments, expect one for Hash", ))?, }; - Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) + Ok(ScSpecTypeDef::BytesN(ScSpecTypeBytesN { n })) } else { Err(Error::new( t.span(), diff --git a/soroban-spec-rust/src/types.rs b/soroban-spec-rust/src/types.rs index 51add9eb4..3cd42e6d8 100644 --- a/soroban-spec-rust/src/types.rs +++ b/soroban-spec-rust/src/types.rs @@ -175,9 +175,8 @@ pub fn generate_type_ident(spec: &ScSpecTypeDef) -> TokenStream { let n = Literal::u32_unsuffixed(b.n); quote! { soroban_sdk::BytesN<#n> } } - ScSpecTypeDef::Hash(b) => { - let n = Literal::u32_unsuffixed(b.n); - quote! { soroban_sdk::Hash<#n> } + ScSpecTypeDef::Hash(_) => { + todo!("Hash type is unsupported, remove this arm when https://github.com/stellar/stellar-xdr/pull/184 is merged") } ScSpecTypeDef::Udt(u) => { let ident = format_ident!("{}", u.name.to_utf8_string().unwrap()); From 4c656c3fdb63f6a8846ae710f5aa6a7ce888ce2e Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 21:03:12 +1000 Subject: [PATCH 11/21] disallow on contracttypes --- soroban-sdk-macros/src/derive_struct.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/soroban-sdk-macros/src/derive_struct.rs b/soroban-sdk-macros/src/derive_struct.rs index 2b51e3b66..32dbcdc07 100644 --- a/soroban-sdk-macros/src/derive_struct.rs +++ b/soroban-sdk-macros/src/derive_struct.rs @@ -25,11 +25,6 @@ pub fn derive_type_struct( ) -> TokenStream2 { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); - let allow_hash = if let Visibility::Public(_) = vis { - false - } else { - true - }; let fields = &data.fields; let field_count_usize: usize = fields.len(); let (spec_fields, field_idents, field_names, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields @@ -47,7 +42,7 @@ pub fn derive_type_struct( errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {MAX}", field_name.len()))); StringM::::default() }), - type_: match map_type(&field.ty, allow_hash) { + type_: match map_type(&field.ty, false) { Ok(t) => t, Err(e) => { errors.push(e); From f20782970a628597d0f150592ccff2c4455d98dd Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 21:17:41 +1000 Subject: [PATCH 12/21] allow hash in return types always but map to bytes --- soroban-sdk-macros/src/derive_fn.rs | 11 +---------- soroban-sdk-macros/src/lib.rs | 1 - 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index f0247b788..c0c92296c 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -7,8 +7,7 @@ use syn::{ punctuated::Punctuated, spanned::Spanned, token::{Colon, Comma}, - Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, ReturnType, Type, TypePath, - TypeReference, + Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, Type, TypePath, TypeReference, }; #[allow(clippy::too_many_arguments)] @@ -18,20 +17,12 @@ pub fn derive_pub_fn( ident: &Ident, attrs: &[Attribute], inputs: &Punctuated, - output: &ReturnType, trait_ident: Option<&Ident>, client_ident: &str, ) -> Result { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); - let allow_hash = ident == "__check_auth"; - if let ReturnType::Type(_, ty) = output { - if let Err(e) = map_type(ty, allow_hash) { - errors.push(e); - } - } - // Prepare the env input. let env_input = inputs.first().and_then(|a| match a { FnArg::Typed(pat_type) => { diff --git a/soroban-sdk-macros/src/lib.rs b/soroban-sdk-macros/src/lib.rs index 29d471401..1de2e7b04 100644 --- a/soroban-sdk-macros/src/lib.rs +++ b/soroban-sdk-macros/src/lib.rs @@ -237,7 +237,6 @@ pub fn contractimpl(metadata: TokenStream, input: TokenStream) -> TokenStream { ident, &m.attrs, &m.sig.inputs, - &m.sig.output, trait_ident, &client_ident, ) From a67d4b78a9fe1db82a9cdbd2876057a9c626d5b3 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 21:18:07 +1000 Subject: [PATCH 13/21] dont allow hash in container types --- soroban-sdk-macros/src/map_type.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index d0f7e8a27..840e99a5d 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -61,8 +61,8 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { ))?, }; Ok(ScSpecTypeDef::Result(Box::new(ScSpecTypeResult { - ok_type: Box::new(map_type(ok, allow_hash)?), - error_type: Box::new(map_type(err, allow_hash)?), + ok_type: Box::new(map_type(ok, false)?), + error_type: Box::new(map_type(err, false)?), }))) } "Option" => { @@ -74,7 +74,7 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { ))?, }; Ok(ScSpecTypeDef::Option(Box::new(ScSpecTypeOption { - value_type: Box::new(map_type(t, allow_hash)?), + value_type: Box::new(map_type(t, false)?), }))) } "Vec" => { @@ -86,7 +86,7 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { ))?, }; Ok(ScSpecTypeDef::Vec(Box::new(ScSpecTypeVec { - element_type: Box::new(map_type(t, allow_hash)?), + element_type: Box::new(map_type(t, false)?), }))) } "Map" => { @@ -98,8 +98,8 @@ pub fn map_type(t: &Type, allow_hash: bool) -> Result { ))?, }; Ok(ScSpecTypeDef::Map(Box::new(ScSpecTypeMap { - key_type: Box::new(map_type(k, allow_hash)?), - value_type: Box::new(map_type(v, allow_hash)?), + key_type: Box::new(map_type(k, false)?), + value_type: Box::new(map_type(v, false)?), }))) } "BytesN" => { From 9a9c241b615784e0450784623e44e5922e2f976e Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 21:18:44 +1000 Subject: [PATCH 14/21] allow hash only as first argument in __check_auth --- soroban-sdk-macros/src/derive_fn.rs | 6 ++++++ soroban-sdk-macros/src/derive_spec_fn.rs | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index c0c92296c..93a654b5a 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -56,9 +56,15 @@ pub fn derive_pub_fn( .enumerate() .map(|(i, a)| match a { FnArg::Typed(pat_ty) => { + // If fn is a __check_auth implementation, allow the first argument, + // signature_payload of type Bytes (32 size), to be a Hash. + let allow_hash = ident == "__check_auth" && i == 0; + + // Error if the type of the fn is not mappable. if let Err(e) = map_type(&pat_ty.ty, allow_hash) { errors.push(e); } + let ident = format_ident!("arg_{}", i); let arg = FnArg::Typed(PatType { attrs: vec![], diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index 1d7db9c85..16ed3f9da 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -53,7 +53,8 @@ pub fn derive_fn_spec( let spec_args: Vec<_> = inputs .iter() .skip(if env_input.is_some() { 1 } else { 0 }) - .map(|a| match a { + .enumerate() + .map(|(i, a)| match a { FnArg::Typed(pat_type) => { let name = if let Pat::Ident(pat_ident) = *pat_type.pat.clone() { pat_ident.ident.to_string() @@ -61,7 +62,12 @@ pub fn derive_fn_spec( errors.push(Error::new(a.span(), "argument not supported")); "".to_string() }; - match map_type(&pat_type.ty, ident == "__check_auth") { + + // If fn is a __check_auth implementation, allow the first argument, + // signature_payload of type Bytes (32 size), to be a Hash. + let allow_hash = ident == "__check_auth" && i == 0; + + match map_type(&pat_type.ty, allow_hash) { Ok(type_) => { let name = name.try_into().unwrap_or_else(|_| { const MAX: u32 = 30; From cbf1aeb5068d797fe22cdf5f16bd02ab6a93d524 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 21:20:25 +1000 Subject: [PATCH 15/21] allow hash in all return values --- soroban-sdk-macros/src/derive_spec_fn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index 16ed3f9da..431ee28bc 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -106,7 +106,7 @@ pub fn derive_fn_spec( // Prepare the output. let spec_result = match output { - ReturnType::Type(_, ty) => vec![match map_type(ty, false) { + ReturnType::Type(_, ty) => vec![match map_type(ty, true) { Ok(spec) => spec, Err(e) => { errors.push(e); From ca6c3097785c0cc49b576037f0749ce7f9eabc49 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 22:28:38 +1000 Subject: [PATCH 16/21] add a test vector that creates an account --- Cargo.lock | 7 + Cargo.toml | 19 +- tests/account/Cargo.toml | 18 ++ tests/account/src/lib.rs | 63 +++++ tests/account/test_snapshots/test/test.1.json | 246 ++++++++++++++++++ 5 files changed, 335 insertions(+), 18 deletions(-) create mode 100644 tests/account/Cargo.toml create mode 100644 tests/account/src/lib.rs create mode 100644 tests/account/test_snapshots/test/test.1.json diff --git a/Cargo.lock b/Cargo.lock index 076fc4d23..22a041063 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1383,6 +1383,13 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "test_account" +version = "21.0.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "test_add_i128" version = "21.0.0" diff --git a/Cargo.toml b/Cargo.toml index 8bf0bc292..4c4670dd5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,24 +8,7 @@ members = [ "soroban-spec-rust", "soroban-ledger-snapshot", "soroban-token-sdk", - "tests/empty", - "tests/empty2", - "tests/add_u64", - "tests/add_i128", - "tests/add_u128", - "tests/import_contract", - "tests/invoke_contract", - "tests/udt", - "tests/contract_data", - "tests/events", - "tests/logging", - "tests/errors", - "tests/alloc", - "tests/auth", - "tests/fuzz", - "tests/multiimpl", - "tests/workspace_contract", - "tests/workspace_lib", + "tests/*", ] [workspace.package] diff --git a/tests/account/Cargo.toml b/tests/account/Cargo.toml new file mode 100644 index 000000000..ec3699307 --- /dev/null +++ b/tests/account/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "test_account" +version.workspace = true +authors = ["Stellar Development Foundation "] +license = "Apache-2.0" +edition = "2021" +publish = false +rust-version.workspace = true + +[lib] +crate-type = ["cdylib"] +doctest = false + +[dependencies] +soroban-sdk = {path = "../../soroban-sdk"} + +[dev-dependencies] +soroban-sdk = {path = "../../soroban-sdk", features = ["testutils"]} diff --git a/tests/account/src/lib.rs b/tests/account/src/lib.rs new file mode 100644 index 000000000..bce7c204b --- /dev/null +++ b/tests/account/src/lib.rs @@ -0,0 +1,63 @@ +#![no_std] +use soroban_sdk::{ + auth::Context, auth::CustomAccountInterface, contract, contracterror, contractimpl, + crypto::Hash, Env, Vec, +}; + +#[contracterror] +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub enum Error { + Fail = 1, +} + +#[contract] +pub struct Contract; + +#[contractimpl] +impl CustomAccountInterface for Contract { + type Error = Error; + type Signature = (); + #[allow(non_snake_case)] + fn __check_auth( + _env: Env, + _signature_payload: Hash<32>, + _signatures: (), + _auth_contexts: Vec, + ) -> Result<(), Error> { + Ok(()) + } +} + +#[cfg(test)] +mod test { + use soroban_sdk::{ + contract, + testutils::{MockAuth, MockAuthInvoke}, + Env, IntoVal, + }; + + use crate::Contract; + + #[contract] + struct TestContract; + + #[test] + fn test() { + let e = Env::default(); + let test_contract_id = e.register_contract(None, TestContract); + let contract_id = e.register_contract(None, Contract); + + e.set_auths(&[MockAuth { + address: &contract_id, + invoke: &MockAuthInvoke { + contract: &test_contract_id, + fn_name: "", + args: ().into_val(&e), + sub_invokes: &[], + }, + } + .into()]); + + e.as_contract(&test_contract_id, || contract_id.require_auth()); + } +} diff --git a/tests/account/test_snapshots/test/test.1.json b/tests/account/test_snapshots/test/test.1.json new file mode 100644 index 000000000..cf124af58 --- /dev/null +++ b/tests/account/test_snapshots/test/test.1.json @@ -0,0 +1,246 @@ +{ + "generators": { + "address": 2, + "nonce": 1 + }, + "auth": [ + [ + [ + "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + { + "function": { + "contract_fn": { + "contract_address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "function_name": "", + "args": [] + } + }, + "sub_invocations": [] + } + ] + ] + ], + "ledger": { + "protocol_version": 21, + "sequence_number": 0, + "timestamp": 0, + "network_id": "0000000000000000000000000000000000000000000000000000000000000000", + "base_reserve": 0, + "min_persistent_entry_ttl": 4096, + "min_temp_entry_ttl": 16, + "max_entry_ttl": 6312000, + "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": null + } + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": null + } + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": { + "ledger_key_nonce": { + "nonce": 1 + } + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": { + "ledger_key_nonce": { + "nonce": 1 + } + }, + "durability": "temporary", + "val": "void" + } + }, + "ext": "v0" + }, + 6311999 + ] + ], + [ + { + "contract_code": { + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_code": { + "ext": "v0", + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "code": "" + } + }, + "ext": "v0" + }, + 4095 + ] + ] + ] + }, + "events": [ + { + "event": { + "ext": "v0", + "contract_id": "0000000000000000000000000000000000000000000000000000000000000001", + "type_": "diagnostic", + "body": { + "v0": { + "topics": [ + { + "symbol": "fn_call" + }, + { + "bytes": "0000000000000000000000000000000000000000000000000000000000000002" + }, + { + "symbol": "__check_auth" + } + ], + "data": { + "vec": [ + { + "bytes": "5eae5e76e50f6cd05ea99bc99c9e19a8cbee2c497eefd008eb25392f8057f876" + }, + "void", + { + "vec": [ + { + "vec": [ + { + "symbol": "Contract" + }, + { + "map": [ + { + "key": { + "symbol": "args" + }, + "val": { + "vec": [] + } + }, + { + "key": { + "symbol": "contract" + }, + "val": { + "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM" + } + }, + { + "key": { + "symbol": "fn_name" + }, + "val": { + "symbol": "" + } + } + ] + } + ] + } + ] + } + ] + } + } + } + }, + "failed_call": false + }, + { + "event": { + "ext": "v0", + "contract_id": "0000000000000000000000000000000000000000000000000000000000000002", + "type_": "diagnostic", + "body": { + "v0": { + "topics": [ + { + "symbol": "fn_return" + }, + { + "symbol": "__check_auth" + } + ], + "data": "void" + } + } + }, + "failed_call": false + } + ] +} \ No newline at end of file From b51e75aeea506a1a509891e58d95b6b7421df8c9 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Mon, 22 Apr 2024 22:36:55 +1000 Subject: [PATCH 17/21] make pub crate because cfg test essentially makes it pub crate anyway --- soroban-sdk/src/crypto.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index c4d87f77b..fe5b2f0e2 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -19,7 +19,7 @@ impl Hash { /// This is intended for test-only, since `Hash` type is only meant to be /// constructed via secure manners. #[cfg(test)] - pub fn from_bytes(bytes: BytesN) -> Self { + pub(crate) fn from_bytes(bytes: BytesN) -> Self { Self(bytes) } } From 48590dc9b969498eedcdc08510998dd7357c5b36 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Mon, 22 Apr 2024 14:32:04 -0400 Subject: [PATCH 18/21] Add more comments to the `Hash` type --- soroban-sdk/src/crypto.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index fe5b2f0e2..7ff015519 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -5,12 +5,18 @@ use crate::{ TryFromVal, Val, }; -/// A Bytes generated by a cryptographic hash function. +/// A Bytes generated by a cryptographic hash function. This struct is designed +/// to be used in contexts where a hash value generated by a secure +/// cryptographic function is required. /// -/// This struct is designed to be used in contexts where a hash value generated -/// by a secure cryptographic function is required. It can only be constructed -/// via secure manners, i.e. output from a secure hash function, or received -/// from the Soroban Env (e.g. via [`CustomAccountInterface`][crate::auth::CustomAccountInterface]) +/// **__Note:_** this struct is used as a special annotation of `Bytes` in +/// certain internally-defined interfaces: output of hash functions, input hash +/// to ECDSA, `__check_auth`. In these scenarios, it is guaranteed the enclosed +/// bytes is the output of a secure hash function. It cannot be used as a +/// contract type (either stored as user data, or as an argument or return type +/// of a contract function). If your contract interface requires to work with a +/// hash, you need to use `Bytes`/`BytesN` and make sure it comes from a secure +/// hash function. pub struct Hash(BytesN); impl Hash { From dea95659eab4e5da9d2ed237859ebc64771c4905 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Tue, 23 Apr 2024 07:46:12 +1000 Subject: [PATCH 19/21] tweak doc --- soroban-sdk/src/crypto.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 7ff015519..ee451f88a 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -5,18 +5,19 @@ use crate::{ TryFromVal, Val, }; -/// A Bytes generated by a cryptographic hash function. This struct is designed -/// to be used in contexts where a hash value generated by a secure -/// cryptographic function is required. +/// A BytesN generated by a cryptographic hash function. /// -/// **__Note:_** this struct is used as a special annotation of `Bytes` in -/// certain internally-defined interfaces: output of hash functions, input hash -/// to ECDSA, `__check_auth`. In these scenarios, it is guaranteed the enclosed -/// bytes is the output of a secure hash function. It cannot be used as a -/// contract type (either stored as user data, or as an argument or return type -/// of a contract function). If your contract interface requires to work with a -/// hash, you need to use `Bytes`/`BytesN` and make sure it comes from a secure -/// hash function. +/// The `Hash` type contains a `BytesN` and can only be constructed in +/// contexts where the value has been generated by a secure cryptographic +/// function. As a result, the type is only found as a return value of calling +/// [`sha256`][Crypto::sha256], [`keccak256`][Crypto::keccak256], or via +/// implementing [`CustomAccountInterface`][crate::auth::CustomAccountInterface] +/// since the `__check_auth` is guaranteed to receive a hash from a secure +/// cryptographic hash function as its first parameter. +/// +/// **__Note:_** A Hash should not be used with storage, since no guarantee can +/// be made about the Bytes stored as to whether they were infact from a secure +/// cryptographic hash function. pub struct Hash(BytesN); impl Hash { From f2de11b2ecb01183aef406bc4e514bd7366fa33e Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Tue, 23 Apr 2024 11:20:16 +1000 Subject: [PATCH 20/21] disallow creating hash from val unless via contract fn --- soroban-sdk-macros/src/derive_fn.rs | 2 +- soroban-sdk/src/crypto.rs | 5 ++- soroban-sdk/src/lib.rs | 5 +++ .../src/try_from_val_for_contract_fn.rs | 39 +++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 soroban-sdk/src/try_from_val_for_contract_fn.rs diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index 93a654b5a..8b347892f 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -80,7 +80,7 @@ pub fn derive_pub_fn( }); let call = quote! { <_ as #crate_path::unwrap::UnwrapOptimized>::unwrap_optimized( - <_ as #crate_path::TryFromVal<#crate_path::Env, #crate_path::Val>>::try_from_val( + <_ as #crate_path::TryFromValForContractFn<#crate_path::Env, #crate_path::Val>>::try_from_val_for_contract_fn( &env, &#ident ) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index ee451f88a..27fb0acbb 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -55,10 +55,11 @@ impl Into<[u8; N]> for Hash { } } -impl TryFromVal for Hash { +#[allow(deprecated)] +impl crate::TryFromValForContractFn for Hash { type Error = ConversionError; - fn try_from_val(env: &Env, v: &Val) -> Result { + fn try_from_val_for_contract_fn(env: &Env, v: &Val) -> Result { Ok(Hash(BytesN::::try_from_val(env, v)?)) } } diff --git a/soroban-sdk/src/lib.rs b/soroban-sdk/src/lib.rs index 8348091ec..80fd76284 100644 --- a/soroban-sdk/src/lib.rs +++ b/soroban-sdk/src/lib.rs @@ -756,6 +756,11 @@ pub use env::SymbolStr; #[doc(hidden)] pub use env::VecObject; +mod try_from_val_for_contract_fn; +#[doc(hidden)] +#[allow(deprecated)] +pub use try_from_val_for_contract_fn::TryFromValForContractFn; + #[doc(hidden)] #[deprecated(note = "use storage")] pub mod data { diff --git a/soroban-sdk/src/try_from_val_for_contract_fn.rs b/soroban-sdk/src/try_from_val_for_contract_fn.rs new file mode 100644 index 000000000..b6ee027c4 --- /dev/null +++ b/soroban-sdk/src/try_from_val_for_contract_fn.rs @@ -0,0 +1,39 @@ +//! TryFromValForContractFn is an internal trait that is used by code generated +//! for the export of contract functions. The generated code calls the trait to +//! convert incoming Val's into their respective SDK types. +//! +//! The trait has a blanket implementation for all types that already implement +//! TryFromVal<_, Val>. +//! +//! The trait exists primarily to allow some special types, e.g. +//! [`crate::crypto::Hash`], to be used as inputs to contract functions without +//! otherwise being createable from a Val via the public TryFromVal trait, and +//! therefore not storeable. +//! +//! For types that can be used and converted everywhere, implementing TryFromVal +//! is most appropriate. For types that should only be used and converted to as +//! part of contract function invocation, then this trait is appropriate. + +use crate::{env::internal::Env, Error, TryFromVal}; +use core::fmt::Debug; + +#[doc(hidden)] +#[deprecated( + note = "TryFromValForContractFn is an internal trait and is not safe to use or implement" +)] +pub trait TryFromValForContractFn: Sized { + type Error: Debug + Into; + fn try_from_val_for_contract_fn(env: &E, v: &V) -> Result; +} + +#[doc(hidden)] +#[allow(deprecated)] +impl TryFromValForContractFn for U +where + U: TryFromVal, +{ + type Error = U::Error; + fn try_from_val_for_contract_fn(e: &E, v: &T) -> Result { + U::try_from_val(e, v) + } +} From 6729c3a47e00fbf2649505ca38bfcfca267cc8d1 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Tue, 23 Apr 2024 11:34:14 +1000 Subject: [PATCH 21/21] add a few helper fns --- soroban-sdk/src/crypto.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/soroban-sdk/src/crypto.rs b/soroban-sdk/src/crypto.rs index 27fb0acbb..a65babf35 100644 --- a/soroban-sdk/src/crypto.rs +++ b/soroban-sdk/src/crypto.rs @@ -1,8 +1,8 @@ //! Crypto contains functions for cryptographic functions. use crate::{ - env::internal, unwrap::UnwrapInfallible, Bytes, BytesN, ConversionError, Env, IntoVal, - TryFromVal, Val, + env::internal, env::internal::BytesObject, unwrap::UnwrapInfallible, Bytes, BytesN, + ConversionError, Env, IntoVal, TryFromVal, Val, }; /// A BytesN generated by a cryptographic hash function. @@ -29,6 +29,34 @@ impl Hash { pub(crate) fn from_bytes(bytes: BytesN) -> Self { Self(bytes) } + + /// Returns a [`BytesN`] containing the bytes in this hash. + #[inline(always)] + pub fn to_bytes(&self) -> BytesN { + self.0.clone() + } + + /// Returns an array containing the bytes in this hash. + #[inline(always)] + pub fn to_array(&self) -> [u8; N] { + self.0.to_array() + } + + pub fn as_val(&self) -> &Val { + self.0.as_val() + } + + pub fn to_val(&self) -> Val { + self.0.to_val() + } + + pub fn as_object(&self) -> &BytesObject { + self.0.as_object() + } + + pub fn to_object(&self) -> BytesObject { + self.0.to_object() + } } impl IntoVal for Hash {