Skip to content

Commit

Permalink
Raise error if struct field name is greater than 30 chars (#1239)
Browse files Browse the repository at this point in the history
### What
Raise compile error if struct field name is greater than 30 chars,
instead of 32 chars.

### Why
As it happens the contract spec restricts the length of the struct field
name to 30 characters. This is an oversight. The type is a StringM<30>,
but we should have made it a ScSymbol in the XDR so that it picked up
the max length of a Symbol. We did this with other parts of the spec,
but not this one it seems.

It's probably too late to change the type in a backwards compatible way,
at least now. And there isn't significant value in trying to fix that
because struct fields over 30 characters long but still under 32
characters long are rare.

The code change introduces a constant in the code that matches the const
generic expected on the StringM type. This probably looks a little odd
but is done to introduce a compiler check/guarantee that the length of
the value we print out can never diverge from the max size of the
StringM that stores the value. If the XDR ever changes to use a larger
value, this code in the SDK will fail to compile until we update it.

Close #1228

### Other Problems This Creates

This discrepancy does create some problems. It's possible using the raw
`Map` type to store key-value pairs where the keys are `Symbol`s up to
32-bytes long, that cannot be transferred into a `contracttype` because
they cannot have field names greater than 30-bytes long. This is quite
an edge case, and so unlikely to be a problem someone encounters, but it
is unfortunate.

### Example

#### Before

No error. An empty field name is written to spec XDR.

#### After

```
error: struct field name is too long: 31, max is 30
  --> tests/empty/src/lib.rs:16:9
   |
16 |     pub a234567890123456789001234567890: u64,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
  • Loading branch information
leighmcculloch authored Mar 5, 2024
1 parent 7c5e216 commit d492a17
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 12 deletions.
11 changes: 5 additions & 6 deletions soroban-sdk-macros/src/derive_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use syn::{Attribute, DataStruct, Error, Ident, Path, Visibility};
use stellar_xdr::curr as stellar_xdr;
use stellar_xdr::{
ScSpecEntry, ScSpecTypeDef, ScSpecUdtStructFieldV0, ScSpecUdtStructV0, StringM, WriteXdr,
SCSYMBOL_LIMIT,
};

use crate::{doc::docs_from_attrs, map_type::map_type, DEFAULT_XDR_RW_LIMITS};
Expand Down Expand Up @@ -37,13 +36,13 @@ pub fn derive_type_struct(
let field_ident = field.ident.as_ref().unwrap();
let field_name = field_ident.to_string();
let field_idx_lit = Literal::usize_unsuffixed(field_num);

if field_name.len() > SCSYMBOL_LIMIT as usize {
errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {}", field_name.len(), SCSYMBOL_LIMIT)));
}
let spec_field = ScSpecUdtStructFieldV0 {
doc: docs_from_attrs(&field.attrs).try_into().unwrap(), // TODO: Truncate docs, or display friendly compile error.
name: field_name.clone().try_into().unwrap_or_else(|_| StringM::default()),
name: field_name.clone().try_into().unwrap_or_else(|_| {
const MAX: u32 = 30;
errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {MAX}", field_name.len())));
StringM::<MAX>::default()
}),
type_: match map_type(&field.ty) {
Ok(t) => t,
Err(e) => {
Expand Down
8 changes: 4 additions & 4 deletions soroban-sdk/src/tests/contract_udt_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub struct Udt {
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[contracttype]
pub struct UdtWithLongName {
pub this_is_a_very_long_name_1234567: u64,
pub this_is_a_very_long_name_12345: u64,
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
Expand All @@ -40,7 +40,7 @@ impl Contract {
}

pub fn add_udt_with_long_name(a: UdtWithLongName, b: UdtWithLongName) -> u64 {
a.this_is_a_very_long_name_1234567 + b.this_is_a_very_long_name_1234567
a.this_is_a_very_long_name_12345 + b.this_is_a_very_long_name_12345
}
}

Expand All @@ -61,10 +61,10 @@ fn test_long_names_functional() {
let contract_id = env.register_contract(None, Contract);

let a = UdtWithLongName {
this_is_a_very_long_name_1234567: 1_000_000_000_000,
this_is_a_very_long_name_12345: 1_000_000_000_000,
};
let b = UdtWithLongName {
this_is_a_very_long_name_1234567: 5_000_000_000_000,
this_is_a_very_long_name_12345: 5_000_000_000_000,
};
assert_eq!(
ContractClient::new(&env, &contract_id).add_udt_with_long_name(&a, &b),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"map": [
{
"key": {
"symbol": "this_is_a_very_long_name_1234567"
"symbol": "this_is_a_very_long_name_12345"
},
"val": {
"u64": 1000000000000
Expand All @@ -108,7 +108,7 @@
"map": [
{
"key": {
"symbol": "this_is_a_very_long_name_1234567"
"symbol": "this_is_a_very_long_name_12345"
},
"val": {
"u64": 5000000000000
Expand Down

0 comments on commit d492a17

Please sign in to comment.