Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow duplicate topics in smart contract events #13065

Merged
merged 3 commits into from
Jan 11, 2023
Merged

Allow duplicate topics in smart contract events #13065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a99f52
Removed `has_duplicates` check form the `deposit_event`.
xgreenx Jan 4, 2023
fb372a3
Merge branch 'master' of https://github.com/paritytech/substrate into…
Jan 10, 2023
812d075
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts
Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions frame/contracts/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,8 +816,6 @@ pub mod pallet {
RandomSubjectTooLong,
/// The amount of topics passed to `seal_deposit_events` exceeds the limit.
TooManyTopics,
/// The topics passed to `seal_deposit_events` contains at least one duplicate.
DuplicateTopics,
/// The chain does not provide a chain extension. Calling the chain extension results
/// in this error. Note that this usually shouldn't happen as deploying such contracts
/// is rejected.
Expand Down
56 changes: 32 additions & 24 deletions frame/contracts/src/wasm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1988,15 +1988,15 @@ mod tests {
assert!(mock_ext.gas_meter.gas_left().ref_time() > 0);
}

const CODE_DEPOSIT_EVENT_MAX_TOPICS: &str = r#"
const CODE_DEPOSIT_EVENT_DUPLICATES: &str = r#"
(module
(import "seal0" "seal_deposit_event" (func $seal_deposit_event (param i32 i32 i32 i32)))
(import "env" "memory" (memory 1 1))

(func (export "call")
(call $seal_deposit_event
(i32.const 32) ;; Pointer to the start of topics buffer
(i32.const 161) ;; The length of the topics buffer.
(i32.const 129) ;; The length of the topics buffer.
(i32.const 8) ;; Pointer to the start of the data buffer
(i32.const 13) ;; Length of the buffer
)
Expand All @@ -2005,37 +2005,44 @@ mod tests {

(data (i32.const 8) "\00\01\2A\00\00\00\00\00\00\00\E5\14\00")

;; Encoded Vec<TopicOf<T>>, the buffer has length of 161 bytes.
(data (i32.const 32) "\14"
;; Encoded Vec<TopicOf<T>>, the buffer has length of 129 bytes.
(data (i32.const 32) "\10"
"\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01"
"\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02"
"\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03"
"\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04"
"\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05")
"\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01"
"\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04")
)
"#;

/// Checks that the runtime traps if there are more than `max_topic_events` topics.
/// Checks that the runtime allows duplicate topics.
#[test]
fn deposit_event_max_topics() {
fn deposit_event_duplicates_allowed() {
let mut mock_ext = MockExt::default();
assert_ok!(execute(CODE_DEPOSIT_EVENT_DUPLICATES, vec![], &mut mock_ext,));

assert_eq!(
execute(CODE_DEPOSIT_EVENT_MAX_TOPICS, vec![], MockExt::default(),),
Err(ExecError {
error: Error::<Test>::TooManyTopics.into(),
origin: ErrorOrigin::Caller,
})
mock_ext.events,
vec![(
vec![
H256::repeat_byte(0x01),
H256::repeat_byte(0x02),
H256::repeat_byte(0x01),
H256::repeat_byte(0x04)
],
vec![0x00, 0x01, 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe5, 0x14, 0x00]
)]
);
}

const CODE_DEPOSIT_EVENT_DUPLICATES: &str = r#"
const CODE_DEPOSIT_EVENT_MAX_TOPICS: &str = r#"
(module
(import "seal0" "seal_deposit_event" (func $seal_deposit_event (param i32 i32 i32 i32)))
(import "env" "memory" (memory 1 1))

(func (export "call")
(call $seal_deposit_event
(i32.const 32) ;; Pointer to the start of topics buffer
(i32.const 129) ;; The length of the topics buffer.
(i32.const 161) ;; The length of the topics buffer.
(i32.const 8) ;; Pointer to the start of the data buffer
(i32.const 13) ;; Length of the buffer
)
Expand All @@ -2044,22 +2051,23 @@ mod tests {

(data (i32.const 8) "\00\01\2A\00\00\00\00\00\00\00\E5\14\00")

;; Encoded Vec<TopicOf<T>>, the buffer has length of 129 bytes.
(data (i32.const 32) "\10"
;; Encoded Vec<TopicOf<T>>, the buffer has length of 161 bytes.
(data (i32.const 32) "\14"
"\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01"
"\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02\02"
"\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01\01"
"\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04")
"\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03\03"
"\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04\04"
"\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05\05")
)
"#;

/// Checks that the runtime traps if there are duplicates.
/// Checks that the runtime traps if there are more than `max_topic_events` topics.
#[test]
fn deposit_event_duplicates() {
fn deposit_event_max_topics() {
assert_eq!(
execute(CODE_DEPOSIT_EVENT_DUPLICATES, vec![], MockExt::default(),),
execute(CODE_DEPOSIT_EVENT_MAX_TOPICS, vec![], MockExt::default(),),
Err(ExecError {
error: Error::<Test>::DuplicateTopics.into(),
error: Error::<Test>::TooManyTopics.into(),
origin: ErrorOrigin::Caller,
})
);
Expand Down
18 changes: 1 addition & 17 deletions frame/contracts/src/wasm/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2085,15 +2085,6 @@ pub mod env {
data_ptr: u32,
data_len: u32,
) -> Result<(), TrapReason> {
fn has_duplicates<T: Ord>(items: &mut Vec<T>) -> bool {
items.sort();
// Find any two consecutive equal elements.
items.windows(2).any(|w| match &w {
&[a, b] => a == b,
_ => false,
})
}

let num_topic = topics_len
.checked_div(sp_std::mem::size_of::<TopicOf<E::T>>() as u32)
.ok_or("Zero sized topics are not allowed")?;
Expand All @@ -2102,7 +2093,7 @@ pub mod env {
return Err(Error::<E::T>::ValueTooLarge.into())
}

let mut topics: Vec<TopicOf<<E as Ext>::T>> = match topics_len {
let topics: Vec<TopicOf<<E as Ext>::T>> = match topics_len {
0 => Vec::new(),
_ => ctx.read_sandbox_memory_as_unbounded(memory, topics_ptr, topics_len)?,
};
Expand All @@ -2112,13 +2103,6 @@ pub mod env {
return Err(Error::<E::T>::TooManyTopics.into())
}

// Check for duplicate topics. If there are any, then trap.
// Complexity O(n * log(n)) and no additional allocations.
// This also sorts the topics.
if has_duplicates(&mut topics) {
return Err(Error::<E::T>::DuplicateTopics.into())
}

let event_data = ctx.read_sandbox_memory(memory, data_ptr, data_len)?;

ctx.ext.deposit_event(topics, event_data);
Expand Down
Loading