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

contracts: Add event field names #9896

Merged
3 commits merged into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 17 additions & 8 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,10 @@ where
}

// Deposit an instantiation event.
deposit_event::<T>(vec![], Event::Instantiated(self.caller().clone(), account_id));
deposit_event::<T>(
vec![],
Event::Instantiated { deployer: self.caller().clone(), contract: account_id },
);
}

Ok(output)
Expand Down Expand Up @@ -942,10 +945,10 @@ where
)?;
ContractInfoOf::<T>::remove(&frame.account_id);
E::remove_user(info.code_hash, &mut frame.nested_meter)?;
Contracts::<T>::deposit_event(Event::Terminated(
frame.account_id.clone(),
beneficiary.clone(),
));
Contracts::<T>::deposit_event(Event::Terminated {
contract: frame.account_id.clone(),
beneficiary: beneficiary.clone(),
});
Ok(())
}

Expand Down Expand Up @@ -997,7 +1000,7 @@ where
fn deposit_event(&mut self, topics: Vec<T::Hash>, data: Vec<u8>) {
deposit_event::<Self::T>(
topics,
Event::ContractEmitted(self.top_frame().account_id.clone(), data),
Event::ContractEmitted { contract: self.top_frame().account_id.clone(), data },
);
}

Expand Down Expand Up @@ -1662,7 +1665,10 @@ mod tests {
Storage::<Test>::code_hash(&instantiated_contract_address).unwrap(),
dummy_ch
);
assert_eq!(&events(), &[Event::Instantiated(ALICE, instantiated_contract_address)]);
assert_eq!(
&events(),
&[Event::Instantiated { deployer: ALICE, contract: instantiated_contract_address }]
);
});
}

Expand Down Expand Up @@ -1751,7 +1757,10 @@ mod tests {
Storage::<Test>::code_hash(&instantiated_contract_address).unwrap(),
dummy_ch
);
assert_eq!(&events(), &[Event::Instantiated(BOB, instantiated_contract_address)]);
assert_eq!(
&events(),
&[Event::Instantiated { deployer: BOB, contract: instantiated_contract_address }]
);
});
}

Expand Down
49 changes: 22 additions & 27 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,49 +369,44 @@ pub mod pallet {
#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
/// Contract deployed by address at the specified address. \[deployer, contract\]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacogr I've removed this \[deployer, contract\], but is that used by polkadot.js currently for the field names?

Copy link
Contributor

@jacogr jacogr Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it not used at all - it purely appears in the md documentation generated from the metadata, aka https://polkadot.js.org/docs/.

Copy link
Member

@shawntabrizi shawntabrizi Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will find this annotations sprinkled everywhere. If moving to this field name syntax, I think we can clean them all up :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was a temporary solution until the events have named fields. More in PR #6684

Instantiated(T::AccountId, T::AccountId),
/// Contract deployed by address at the specified address.
Instantiated { deployer: T::AccountId, contract: T::AccountId },

/// Contract has been removed.
/// \[contract, beneficiary\]
///
/// # Params
///
/// - `contract`: The contract that was terminated.
/// - `beneficiary`: The account that received the contracts remaining balance.
///
/// # Note
///
/// The only way for a contract to be removed and emitting this event is by calling
/// `seal_terminate`.
Terminated(T::AccountId, T::AccountId),
Terminated {
/// The contract that was terminated.
contract: T::AccountId,
/// The account that received the contracts remaining balance
beneficiary: T::AccountId,
},

/// Code with the specified hash has been stored. \[code_hash\]
CodeStored(T::Hash),
/// Code with the specified hash has been stored.
CodeStored { code_hash: T::Hash },

/// Triggered when the current schedule is updated.
/// \[version\]
///
/// # Params
///
/// - `version`: The version of the newly set schedule.
ScheduleUpdated(u32),
ScheduleUpdated {
/// The version of the newly set schedule.
version: u32,
},

/// A custom event emitted by the contract.
/// \[contract, data\]
///
/// # Params
///
/// - `contract`: The contract that emitted the event.
/// - `data`: Data supplied by the contract. Metadata generated during contract compilation
/// is needed to decode it.
ContractEmitted(T::AccountId, Vec<u8>),
ContractEmitted {
/// The contract that emitted the event.
contract: T::AccountId,
/// Data supplied by the contract. Metadata generated during contract compilation
/// is needed to decode it.
data: Vec<u8>,
},

/// A code with the specified hash was removed.
/// \[code_hash\]
///
/// This happens when the last contract that uses this code hash was removed.
CodeRemoved(T::Hash),
CodeRemoved { code_hash: T::Hash },
}

#[pallet::error]
Expand Down
24 changes: 16 additions & 8 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,20 +478,25 @@ fn instantiate_and_call_and_deposit_event() {
},
EventRecord {
phase: Phase::Initialization,
event: Event::Contracts(crate::Event::CodeStored(code_hash.into())),
event: Event::Contracts(crate::Event::CodeStored {
code_hash: code_hash.into()
}),
topics: vec![],
},
EventRecord {
phase: Phase::Initialization,
event: Event::Contracts(crate::Event::ContractEmitted(
addr.clone(),
vec![1, 2, 3, 4]
)),
event: Event::Contracts(crate::Event::ContractEmitted {
contract: addr.clone(),
data: vec![1, 2, 3, 4]
}),
topics: vec![],
},
EventRecord {
phase: Phase::Initialization,
event: Event::Contracts(crate::Event::Instantiated(ALICE, addr.clone())),
event: Event::Contracts(crate::Event::Instantiated {
deployer: ALICE,
contract: addr.clone()
}),
topics: vec![],
},
]
Expand Down Expand Up @@ -764,12 +769,15 @@ fn self_destruct_works() {
},
EventRecord {
phase: Phase::Initialization,
event: Event::Contracts(crate::Event::CodeRemoved(code_hash)),
event: Event::Contracts(crate::Event::CodeRemoved { code_hash }),
topics: vec![],
},
EventRecord {
phase: Phase::Initialization,
event: Event::Contracts(crate::Event::Terminated(addr.clone(), DJANGO)),
event: Event::Contracts(crate::Event::Terminated {
contract: addr.clone(),
beneficiary: DJANGO
}),
topics: vec![],
},
],
Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/wasm/code_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ where
Some(module) => increment_64(&mut module.refcount),
None => {
*existing = Some(prefab_module);
Contracts::<T>::deposit_event(Event::CodeStored(code_hash))
Contracts::<T>::deposit_event(Event::CodeStored { code_hash })
},
});
}
Expand Down Expand Up @@ -170,7 +170,7 @@ where
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
<PristineCode<T>>::remove(code_hash);
Contracts::<T>::deposit_event(Event::CodeRemoved(code_hash))
Contracts::<T>::deposit_event(Event::CodeRemoved { code_hash })
}

/// Increment the refcount panicking if it should ever overflow (which will not happen).
Expand Down