Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contract level messages #4370

Merged

Conversation

alsrdn
Copy link
Contributor

@alsrdn alsrdn commented Oct 20, 2023

Contract level messages are a facility that enable a smart contract to emit a message while executing, which eventually gets sent on the event stream and is visible from outside the network.

This PR introduces 2 new FFIs that allow contracts to manage message topics and emit messages.
Messages are sent out through DeployProcessed SSE events.

Alexandru Sardan and others added 29 commits September 22, 2023 16:11
Add a new `Key::MessageTopic` variant under which message digests will
be written to global state for messages emitted by contracts.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
casper-types: add a new `Key` variant for contract level messages
Added 2 new FFIs that enable contracts to register message topics and
emit human readable string messages.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
… new block

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Add the message topic names along with the hashes of their names to the
addressable entity to enable easy discovery.
Also migrate topics to new contract on upgrade.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Also re-organize the types used for contract level messages in multiple
modules and refactor naming for better consistency.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
…the SSE

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Remove unnecessary trap error, fix comments and add new test case for
checking topic name limits.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
…nts-alt

ee: add support for emitting contract level messages
Add test for checking that messages topics are carried over to an
upgraded contract.
Fix check for not allowing session code to register message topics and
add test.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Now the `Message` key has the largest serialized length so the test
needs to be updated.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
…sages

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Implemented a mechanism to increase the gas cost for emitting messages
linearly for each execution.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Adjust the expected costs for the faucet and the host function
metrics contract since the code size has increased due to the addition
of the contract messages code. Also the size of the addressable entity
stored value has increased because of the stored topic names.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
The `AddressableEntity` record now includes the name of the message
topics so the fixture needs to be adjusted.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
When emitting messages, return an error if there's no space left in the
topic.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Also add convenience methods to convert contract message related stored
values to their inner types.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Don't increase the cost if the message was not emitted.
Also simplify config emit message cost config.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
…nts-alt

mechanism to increase gas cost for each message emitted within one execution
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Separate contract messages from the execution results; we generally
don't want to store messages since they are supposed to be ephemeral.
Making them part of the exec results will make them get stored
automatically in the node DB with the exec results.

Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Alexandru Sardan added 2 commits October 30, 2023 18:25
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Copy link
Collaborator

@mpapierski mpapierski left a comment

Choose a reason for hiding this comment

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

Couple of notes regarding costs, and few nits, but overall looks good 👍

)))));
}

let topic_name = self.string_from_mem(topic_name_ptr, topic_name_size)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know we're not consistent at all with regards to this but I have suggestion that we perhaps don't deserialize a string here, but instead, pass the string directly from Wasm. The host already has access to all the data, and you just need to pass pointer to the string i.e.

// wasm
fn foo(s: &str) {
  ext_ffi::foo(s.as_ptr(), s.len());
}
// host
self.checked_memory_slice(key_ptr as usize, key_size as usize, |data| {
  std::str::from_utf8(data).map(ToOwned::to_owned)
})?

This should make it cheaper for the users as additional serialization, deserialization, etc, adds to the gas costs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed serialization in 53483e1 although didn't use checked_memory_slice as it's not available yet on the feat-2.0 branch. Will do a follow-up commit after.

)))));
}

let topic_name = self.string_from_mem(topic_name_ptr, topic_name_size)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed serialization in 53483e1 although didn't use checked_memory_slice as it's not available yet on the feat-2.0 branch. Will do a follow-up commit after.

&scratch_state,
metrics.clone(),
state_root_hash,
deploy_hash,
result,
)?;
execution_results.push((deploy_hash, deploy_header, execution_result));
execution_results.push((deploy_hash, deploy_header, execution_result, messages));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be changed into a named struct at this point?

Copy link
Contributor Author

@alsrdn alsrdn Nov 3, 2023

Choose a reason for hiding this comment

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

Changed in 53483e1

let execution_results_checksum =
compute_execution_results_checksum(execution_results.iter().map(|(_, _, result)| result))?;
let execution_results_checksum = compute_execution_results_checksum(
execution_results.iter().map(|(_, _, result, _)| result),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Then you could hide the complexity

Suggested change
execution_results.iter().map(|(_, _, result, _)| result),
execution_results.into_iter().map(|execution_result| execution_result.result),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 53483e1

return Err(ApiError::InvalidArgument);
}

let (topic_name_ptr, topic_name_size, _bytes) = contract_api::to_ptr(topic_name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my other comment regarding passing strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 53483e1

}

let (topic_name_ptr, topic_name_size, _bytes) = contract_api::to_ptr(topic_name);
let (operation_ptr, operation_size, _bytes) = contract_api::to_ptr(operation);
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be fair, looking at the definition this could be passed as a number to the host - unless you plan to add more variants with fields etc. in the future. Adding #[derive(ToPrimitive, FromPrimitive)] to MessageTopicOperation could help with convenience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, now we could pass it as a plain number, and it would probably work for a new operation like Remove. Though my thinking was that if we're going to support more complex operations in the future like Rename, or to allow selection of the checksum algorithm we would need space for some parameters.

}

fn is_zero(&self) -> bool {
self.bit.is_zero()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use let OpcodeCosts { bit, add, .. } = self etc just so we wouldn't accidentally forget to update this if it grows new fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 53483e1

types/src/contract_messages/messages.rs Show resolved Hide resolved
Comment on lines +958 to +963
if let Self::AddressableEntity(PackageKindTag::SmartContract, _) = self {
return true;
}

false
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit

Suggested change
if let Self::AddressableEntity(PackageKindTag::SmartContract, _) = self {
return true;
}
false
}
matches!(self, Self::AddressableEntity(PackageKindTag::SmartContract, _))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other implementations follow the same (e.g. is_bid_addr_key, is_dictionary_key etc) follow the same style so I would prefer to keep it like this.

Alexandru Sardan added 2 commits November 3, 2023 15:20
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
Signed-off-by: Alexandru Sardan <alexandru@casperlabs.io>
@alsrdn
Copy link
Contributor Author

alsrdn commented Nov 3, 2023

bors r+

casperlabs-bors-ng bot added a commit that referenced this pull request Nov 3, 2023
4370: Contract level messages r=alsrdn a=alsrdn

Contract level messages are a facility that enable a smart contract to emit a message while executing, which eventually gets sent on the event stream and is visible from outside the network.

This PR introduces 2 new FFIs that allow contracts to manage message topics and emit messages.
Messages are sent out through `DeployProcessed` SSE events.

Co-authored-by: Alexandru Sardan <alexandru@casperlabs.io>
Co-authored-by: Joe Sacher <321623+sacherjj@users.noreply.github.com>
Co-authored-by: Luca B <93586856+bradjohnl@users.noreply.github.com>
Copy link
Contributor

Build failed:

@alsrdn
Copy link
Contributor Author

alsrdn commented Nov 3, 2023

bors merge

casperlabs-bors-ng bot added a commit that referenced this pull request Nov 3, 2023
4370: Contract level messages r=alsrdn a=alsrdn

Contract level messages are a facility that enable a smart contract to emit a message while executing, which eventually gets sent on the event stream and is visible from outside the network.

This PR introduces 2 new FFIs that allow contracts to manage message topics and emit messages.
Messages are sent out through `DeployProcessed` SSE events.

Co-authored-by: Alexandru Sardan <alexandru@casperlabs.io>
Co-authored-by: Joe Sacher <321623+sacherjj@users.noreply.github.com>
Co-authored-by: Luca B <93586856+bradjohnl@users.noreply.github.com>
Copy link
Contributor

Build failed:

@alsrdn
Copy link
Contributor Author

alsrdn commented Nov 3, 2023

bors r+

Copy link
Contributor

Build succeeded:

@casperlabs-bors-ng casperlabs-bors-ng bot merged commit fd01a5a into casper-network:feat-2.0 Nov 3, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants