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

Add Call Endpoint #708

Merged
merged 11 commits into from
Mar 11, 2022
Merged

Add Call Endpoint #708

merged 11 commits into from
Mar 11, 2022

Conversation

arun-koshy
Copy link
Contributor

This is the fifth of a series of PR's that will implement the client service API described as part of the GDC Eng Deliverables

The endpoints that are included in this PR are:

Call: Execute a Move call transaction by calling the specified function in the module of the given package. Arguments are passed in and type will be inferred from function signature. Gas usage is capped by the gas_budget.

curl --location --request POST 'http://127.0.0.1:5000/call'
--header 'Content-Type: application/json'
--data-raw '{
"sender": "A7099E1F43A47D50488179D02E45DD63FFEAC8E9",
"packageObjectId": "0x2",
"module": "ObjectBasics",
"function": "create",
"args": [
100,
"0xA7099E1F43A47D50488179D02E45DD63FFEAC8E9"
],
"gasObjectId": "0F3A9F01B2C950DE335244EB8DA5DAD558F4038F",
"gasBudget": 2000
}'

@666lcz
Copy link
Contributor

666lcz commented Mar 8, 2022

Thanks for working on this! Can we also test passing by objects: https://github.com/MystenLabs/fastnft/blob/c7eb3f784cb7ea50bae21f4d0cc5b3956e7ab936/sui_programmability/framework/sources/ObjectBasics.move#L29?

@arun-koshy
Copy link
Contributor Author

@oxade
Copy link
Contributor

oxade commented Mar 9, 2022

Please link an issue for unit tests

@arun-koshy
Copy link
Contributor Author

Please link an issue for unit tests

#635

@arun-koshy
Copy link
Contributor Author

Thanks for working on this! Can we also test passing by objects:
https://github.com/MystenLabs/fastnft/blob/c7eb3f784cb7ea50bae21f4d0cc5b3956e7ab936/sui_programmability/framework/sources/ObjectBasics.move#L29

?

Screen Shot 2022-03-08 at 10 40 03 AM

@666lcz just in case this image was not clear, transferring objects via Move Call works! This is the curl request to do it yourself (it will hit the hosted version we are using for geniteam)

curl --location --request POST 'http://3.229.149.13:5000/call'
--header 'Content-Type: application/json'
--data-raw '{
"sender": "09818AAC3EDF9CF9B006B70C36E7241768B26386",
"packageObjectId": "0x2",
"module": "ObjectBasics",
"function": "transfer",
"args": [
"0x{{object_id}}",
"0x{{to_address}}"
],
"gasObjectId": "0000000000000000000000000000000000000003",
"gasBudget": 2000
}'

sui/src/rest_server.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@666lcz 666lcz left a comment

Choose a reason for hiding this comment

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

Overall looks good to me! Just a few nits for error code and writing shorter and reusable functions

sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
sui/src/rest_server.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

AFAiCT, this is pretty much done, thanks!
Left a nit if you're up for one more pass.

Comment on lines 1037 to 1043
let mut effects = Vec::new();
for (obj, _) in transaction_effects.created {
let mut effect = HashMap::new();
effect.insert("id".to_string(), obj.0.to_string());
effect.insert("version".to_string(), format!("{:?}", obj.1));
effect.insert("object_digest".to_string(), format!("{:?}", obj.2));
for ((object_id, sequence_number, object_digest), _) in transaction_effects.created {
let effect = get_effect(wallet_context, object_id, sequence_number, object_digest)
.await
.map_err(|error| error)?;
effects.push(effect);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This is basically:

let effects = transaction_effects.created.map(|((object_id, sequence_number, object_digest), _)| {
    get_effect(wallet_context, object_id, sequence_number, object_digest)
    .await
    .map_err(|error| error.into())
}.collect::<Result<Vec<_>,_>()?;

But my point isn't the refactoring to combinators, it's about the use of an auxiliary private function (e.g. in this file) to not repeat this so many times.

What I'd love to see here and in other places is:

let effects = my_auxiliary_fn(some_kind_of_tx_effects).await?;

if my_auxiliary_fn contains a for loop, I'll live :)

@arun-koshy arun-koshy merged commit ecc7a96 into main Mar 11, 2022
@arun-koshy arun-koshy deleted the arun/call branch March 11, 2022 08:43
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