Skip to content

Conversation

@linh2931
Copy link
Contributor

@linh2931 linh2931 commented Mar 18, 2025

This PR implements the remaining sync call host functions. Basic sync call protocol level functionalities are in place.

Unit test basic_params_return_value_passing shows a complete end to end sync call in WASM code using the host functions in the following steps

  1. making a sync call,
  2. passing arguments,
  3. saving return value,
  4. retrieving return value

Resolves #1215

@linh2931 linh2931 requested review from heifner and spoonincode March 18, 2025 00:00
@spoonincode
Copy link
Contributor

afaict we aren't enforcing max_sync_call_depth yet even in this PR. It feels like at least one of these PRs should have that enforcement? Since this one is at least enforcing max_sync_call_data_size now

@linh2931
Copy link
Contributor Author

afaict we aren't enforcing max_sync_call_depth yet even in this PR. It feels like at least one of these PRs should have that enforcement? Since this one is at least enforcing max_sync_call_data_size now

I plan to enforce max_sync_call_depth after the refactor of apply_context is done (#1261, my next task), since that will change code structure. This PR focuses on the three host functions. I will create a task issue.

Copy link
Contributor

@spoonincode spoonincode left a comment

Choose a reason for hiding this comment

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

I might have missed it, but I think it would be worthwhile to have tests that have a 0 length call data, and 0 length call return data.

Also I see we're following the same pattern as action return data that makes it impossible to discern between an action that returned nothing and an action that returned a 0 length value. Consistency is good; just pointing it out.

Base automatically changed from sync_call_limits to sync_call March 31, 2025 19:31
@linh2931
Copy link
Contributor Author

linh2931 commented Mar 31, 2025

length

Good idea! I added #1297 for more tests in a future PR.

Right now I have get_call_data_less_memory_test for destination size 0 or less than required size.

@linh2931 linh2931 merged commit 0694e8b into sync_call Mar 31, 2025
36 checks passed
@linh2931 linh2931 deleted the params_return_value branch March 31, 2025 21:55
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.

4 participants