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

[pallet-revive] last call return data API #5779

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

xermicus
Copy link
Member

@xermicus xermicus commented Sep 19, 2024

This PR introduces 2 new syscalls: return_data_size and return_data_copy, resembling the semantics of the EVM RETURNDATASIZE and RETURNDATACOPY opcodes.

The ownership of ExecReturnValue (the return data) has moved to the Frame. This allows implementing the new contract API functionality in ext with no additional copies. Returned data is passed via contract memory, memory is (will be) metered, hence the amount of returned data can not be statically known, so we should avoid storing copies of the returned data if we can. By moving the ownership of the exectuables return value into the Frame struct we achieve this.

A zero-copy implementation of those APIs would be technically possible without that internal change by making the callsite in the runtime responsible for moving the returned data into the frame after any call. However, resetting the stored output needs to be handled in ext, since plain transfers will not affect the stored return data (and we don't want to handle this special call case inside the runtime API). This has drawbacks:

  • It can not be tested easily in the mock.
  • It introduces an inconsistency where resetting the stored output is handled in ext, but the runtime API is responsible to store it back correctly after any calls made. Instead, with ownership of the data in Frame, both can be handled in a single place. Handling both in fn run() is more natural and leaves less room for runtime API bugs.

The returned output is reset each time before running any executable in a nested stack. This change should not incur any overhead to the overall memory usage as only the returned data from the last executed frame will be kept around at any time.

xermicus and others added 7 commits September 18, 2024 21:11
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
@xermicus xermicus added R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts. labels Sep 19, 2024
xermicus and others added 9 commits September 20, 2024 10:04
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: xermicus <cyrill@parity.io>
@xermicus xermicus marked this pull request as ready for review September 20, 2024 16:17
@xermicus
Copy link
Member Author

bot fmt

@command-bot
Copy link

command-bot bot commented Sep 20, 2024

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 5-936aad13-b847-43cd-8d31-72960ecb926d to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 20, 2024

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7398794/artifacts/download.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant