Skip to content

Commit

Permalink
Allow duplicate topics in smart contract events (paritytech#13065)
Browse files Browse the repository at this point in the history 
* Removed `has_duplicates` check form the `deposit_event`.
Removed the usage of `Error::DuplicateTopics`.

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_contracts

Co-authored-by: command-bot <>
  • Loading branch information
@xgreenx @ark0f
xgreenx authored and ark0f committed Feb 27, 2023
1 parent 8d9a8a6 commit ec8c480
Show file tree
Hide file tree
Showing 4 changed files with 972 additions and 978 deletions.
2 changes: 0 additions & 2 deletions frame/contracts/src/lib.rs
View file
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
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
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

0 comments on commit ec8c480

Please sign in to comment.