Skip to content

Commit 808e774

Browse files
mootz12mootz12
committed
Fix contractevent vec data format to preserve field declaration order (#1680)
### What Only sort contractevent fields when building events where the data format is `map`. ### Why For contractevents with a data_format of vec, sorting the fields caused the data to be published in a different order than what the event defined, which is not expected. Closes #1679 ### Known limitations N/A
1 parent 98c46cd commit 808e774

4 files changed

Lines changed: 304 additions & 13 deletions

File tree

‎soroban-sdk-macros/src/derive_event.rs‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -229,17 +229,12 @@ fn derive_impls(args: &ContractEventArgs, input: &DeriveInput) -> Result<TokenSt
229229
let data_params = params
230230
.iter()
231231
.filter(|p| p.location == ScSpecEventParamLocationV0::Data)
232-
.sorted_by_key(|p| p.name.to_string()) // must be sorted for map_new_from_slices
233232
.collect::<Vec<_>>();
234233
let data_params_count = data_params.len();
235234
let data_idents = data_params
236235
.iter()
237236
.map(|p| format_ident!("{}", p.name.to_string()))
238237
.collect::<Vec<_>>();
239-
let data_strs = data_idents
240-
.iter()
241-
.map(|i| i.to_string())
242-
.collect::<Vec<_>>();
243238
let data_to_val = match args.data_format {
244239
DataFormat::SingleValue if data_params_count == 0 => quote! {
245240
#path::Val::VOID.to_val()
@@ -258,14 +253,26 @@ fn derive_impls(args: &ContractEventArgs, input: &DeriveInput) -> Result<TokenSt
258253
#({ let v: #path::Val = self.#data_idents.into_val(env); v },)*
259254
).into_val(env)
260255
},
261-
DataFormat::Map => quote! {
262-
use #path::{EnvBase,IntoVal,unwrap::UnwrapInfallible};
263-
const KEYS: [&'static str; #data_params_count] = [#(#data_strs),*];
264-
let vals: [#path::Val; #data_params_count] = [
265-
#(self.#data_idents.into_val(env)),*
266-
];
267-
env.map_new_from_slices(&KEYS, &vals).unwrap_infallible().into()
268-
},
256+
DataFormat::Map => {
257+
// Must be sorted for map_new_from_slices
258+
let data_idents_sorted = data_params
259+
.iter()
260+
.sorted_by_key(|p| p.name.to_string())
261+
.map(|p| format_ident!("{}", p.name.to_string()))
262+
.collect::<Vec<_>>();
263+
let data_strs_sorted = data_idents_sorted
264+
.iter()
265+
.map(|i| i.to_string())
266+
.collect::<Vec<_>>();
267+
quote! {
268+
use #path::{EnvBase,IntoVal,unwrap::UnwrapInfallible};
269+
const KEYS: [&'static str; #data_params_count] = [#(#data_strs_sorted),*];
270+
let vals: [#path::Val; #data_params_count] = [
271+
#(self.#data_idents_sorted.into_val(env)),*
272+
];
273+
env.map_new_from_slices(&KEYS, &vals).unwrap_infallible().into()
274+
}
275+
}
269276
};
270277

271278
// Output.

‎soroban-sdk/src/tests/contract_event.rs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,46 @@ fn test_data_vec_no_data() {
405405
);
406406
}
407407

408+
#[test]
409+
fn test_data_vec_preserves_field_order() {
410+
let env = Env::default();
411+
412+
#[contract]
413+
pub struct Contract;
414+
let id = env.register(Contract, ());
415+
416+
// Fields are named so that alphabetical order differs from declaration order
417+
#[contractevent(data_format = "vec")]
418+
pub struct Deposit {
419+
#[topic]
420+
addr: Symbol,
421+
time: u64,
422+
amount: i128,
423+
}
424+
425+
let event = Deposit {
426+
addr: symbol_short!("user"),
427+
time: 1000u64,
428+
amount: 500i128,
429+
};
430+
env.as_contract(&id, || {
431+
event.publish(&env);
432+
});
433+
434+
let data: Val = (1000u64, 500i128).into_val(&env);
435+
let expected_event = xdr::ContractEvent {
436+
ext: xdr::ExtensionPoint::V0,
437+
type_: xdr::ContractEventType::Contract,
438+
contract_id: Some(id.contract_id()),
439+
body: xdr::ContractEventBody::V0(xdr::ContractEventV0 {
440+
topics: vec![&env, symbol_short!("deposit"), symbol_short!("user")].into(),
441+
data: xdr::ScVal::try_from_val(&env, &data).unwrap(),
442+
}),
443+
};
444+
assert_eq!(env.events().all(), std::vec![expected_event.clone()],);
445+
assert_eq!(event.to_xdr(&env, &id), expected_event);
446+
}
447+
408448
#[test]
409449
fn test_data_map() {
410450
let env = Env::default();
@@ -492,6 +532,56 @@ fn test_data_map_no_data() {
492532
);
493533
}
494534

535+
#[test]
536+
fn test_data_map_sorts_fields() {
537+
let env = Env::default();
538+
539+
#[contract]
540+
pub struct Contract;
541+
let id = env.register(Contract, ());
542+
543+
#[contractevent(data_format = "map")]
544+
pub struct Deposit {
545+
#[topic]
546+
addr: Symbol,
547+
time: u64,
548+
amount: i128,
549+
}
550+
551+
let event = Deposit {
552+
addr: symbol_short!("user"),
553+
time: 1000u64,
554+
amount: 500i128,
555+
};
556+
env.as_contract(&id, || {
557+
event.publish(&env);
558+
});
559+
560+
let data: Val = map![
561+
&env,
562+
(
563+
symbol_short!("amount"),
564+
<_ as IntoVal<Env, Val>>::into_val(&500i128, &env),
565+
),
566+
(
567+
symbol_short!("time"),
568+
<_ as IntoVal<Env, Val>>::into_val(&1000u64, &env),
569+
),
570+
]
571+
.into_val(&env);
572+
let expected_event = xdr::ContractEvent {
573+
ext: xdr::ExtensionPoint::V0,
574+
type_: xdr::ContractEventType::Contract,
575+
contract_id: Some(id.contract_id()),
576+
body: xdr::ContractEventBody::V0(xdr::ContractEventV0 {
577+
topics: vec![&env, symbol_short!("deposit"), symbol_short!("user")].into(),
578+
data: xdr::ScVal::try_from_val(&env, &data).unwrap(),
579+
}),
580+
};
581+
assert_eq!(env.events().all(), std::vec![expected_event.clone()],);
582+
assert_eq!(event.to_xdr(&env, &id), expected_event);
583+
}
584+
495585
#[test]
496586
fn test_ref_fields() {
497587
let env = Env::default();

‎soroban-sdk/test_snapshots/tests/contract_event/test_data_map_sorts_fields.1.json‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
{
2+
"generators": {
3+
"address": 1,
4+
"nonce": 0,
5+
"mux_id": 0
6+
},
7+
"auth": [
8+
[],
9+
[]
10+
],
11+
"ledger": {
12+
"protocol_version": 25,
13+
"sequence_number": 0,
14+
"timestamp": 0,
15+
"network_id": "0000000000000000000000000000000000000000000000000000000000000000",
16+
"base_reserve": 0,
17+
"min_persistent_entry_ttl": 4096,
18+
"min_temp_entry_ttl": 16,
19+
"max_entry_ttl": 6312000,
20+
"ledger_entries": [
21+
{
22+
"entry": {
23+
"last_modified_ledger_seq": 0,
24+
"data": {
25+
"contract_data": {
26+
"ext": "v0",
27+
"contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM",
28+
"key": "ledger_key_contract_instance",
29+
"durability": "persistent",
30+
"val": {
31+
"contract_instance": {
32+
"executable": {
33+
"wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
34+
},
35+
"storage": null
36+
}
37+
}
38+
}
39+
},
40+
"ext": "v0"
41+
},
42+
"live_until": 4095
43+
},
44+
{
45+
"entry": {
46+
"last_modified_ledger_seq": 0,
47+
"data": {
48+
"contract_code": {
49+
"ext": "v0",
50+
"hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
51+
"code": ""
52+
}
53+
},
54+
"ext": "v0"
55+
},
56+
"live_until": 4095
57+
}
58+
]
59+
},
60+
"events": [
61+
{
62+
"event": {
63+
"ext": "v0",
64+
"contract_id": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM",
65+
"type_": "contract",
66+
"body": {
67+
"v0": {
68+
"topics": [
69+
{
70+
"symbol": "deposit"
71+
},
72+
{
73+
"symbol": "user"
74+
}
75+
],
76+
"data": {
77+
"map": [
78+
{
79+
"key": {
80+
"symbol": "amount"
81+
},
82+
"val": {
83+
"i128": "500"
84+
}
85+
},
86+
{
87+
"key": {
88+
"symbol": "time"
89+
},
90+
"val": {
91+
"u64": "1000"
92+
}
93+
}
94+
]
95+
}
96+
}
97+
}
98+
},
99+
"failed_call": false
100+
}
101+
]
102+
}

‎soroban-sdk/test_snapshots/tests/contract_event/test_data_vec_preserves_field_order.1.json‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
{
2+
"generators": {
3+
"address": 1,
4+
"nonce": 0,
5+
"mux_id": 0
6+
},
7+
"auth": [
8+
[],
9+
[]
10+
],
11+
"ledger": {
12+
"protocol_version": 25,
13+
"sequence_number": 0,
14+
"timestamp": 0,
15+
"network_id": "0000000000000000000000000000000000000000000000000000000000000000",
16+
"base_reserve": 0,
17+
"min_persistent_entry_ttl": 4096,
18+
"min_temp_entry_ttl": 16,
19+
"max_entry_ttl": 6312000,
20+
"ledger_entries": [
21+
{
22+
"entry": {
23+
"last_modified_ledger_seq": 0,
24+
"data": {
25+
"contract_data": {
26+
"ext": "v0",
27+
"contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM",
28+
"key": "ledger_key_contract_instance",
29+
"durability": "persistent",
30+
"val": {
31+
"contract_instance": {
32+
"executable": {
33+
"wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
34+
},
35+
"storage": null
36+
}
37+
}
38+
}
39+
},
40+
"ext": "v0"
41+
},
42+
"live_until": 4095
43+
},
44+
{
45+
"entry": {
46+
"last_modified_ledger_seq": 0,
47+
"data": {
48+
"contract_code": {
49+
"ext": "v0",
50+
"hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
51+
"code": ""
52+
}
53+
},
54+
"ext": "v0"
55+
},
56+
"live_until": 4095
57+
}
58+
]
59+
},
60+
"events": [
61+
{
62+
"event": {
63+
"ext": "v0",
64+
"contract_id": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM",
65+
"type_": "contract",
66+
"body": {
67+
"v0": {
68+
"topics": [
69+
{
70+
"symbol": "deposit"
71+
},
72+
{
73+
"symbol": "user"
74+
}
75+
],
76+
"data": {
77+
"vec": [
78+
{
79+
"u64": "1000"
80+
},
81+
{
82+
"i128": "500"
83+
}
84+
]
85+
}
86+
}
87+
}
88+
},
89+
"failed_call": false
90+
}
91+
]
92+
}

0 commit comments

Comments
 (0)