rest: introduce transaction resolve and simulation endpoints #19029

bmwill · 2024-08-19T14:38:17Z

Description

This stack of commits introduces a transaction resolve and simulation endpoint to the REST api service.

More documentation, usage examples as well as a stabilized format for an unresolved transaction JSON payload still need to be done/decided.

At a high level the resolve api will allow someone to provide a partially filled in PTB and have the Fullnode:

select versions
mutability of the objects
budget determination
fill in reference gas price
basic gas selection/gas smashing

Test plan

How did you test the new or updated feature?

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

vercel · 2024-08-19T14:38:28Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 10, 2024 1:00am

3 Skipped Deployments

Name	Status	Preview	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview	Oct 10, 2024 1:00am
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview	Oct 10, 2024 1:00am
sui-typescript-docs	⬜️ Ignored (Inspect)	Visit Preview	Oct 10, 2024 1:00am

vercel · 2024-08-19T14:38:29Z

@bmwill is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

hayes-mysten

Are there examples of the JSON format somewhere I can look at? I can't recall where we landed on standardizing the JSON format. I know we definitely wanted to align the JSON format between the GraphQL API and the TS SDK, but were less sure about how this would work for the rest API. I think it would be good to check in about what differences we have now that there is an actual implementation, and see if we want to align things more before releasing this. Using this will be a bit more complicated if the formats are different between the GraphQL/TS SDK and the rest API

hayes-mysten · 2024-08-19T17:08:56Z

crates/sui-e2e-tests/tests/rest.rs

+                    initial_shared_version: None,
+                    mutable: None,
+                },
+                UnresolvedInputArgument::ImmutableOrOwned(UnresolvedObjectReference {


I was trying to track down our original conversations around modeling these inputs, but something thats missing here is the ability to model inputs where we don't know if they are Immutable/Owned/Shared/Receiving, which will be important to support before this can be used by the sdk

Yeah i do how notes from our previous conversation about just wanting to provide the objectid and no other info. we can definitely do that. Do you see there being value in being able to specify these other options? as in allowing you to hint that this should be an owned object/shared objects or would it be simpler to not carry that information and have the server handle it entirely?

What we do in the typescript SDK is to just use a single type that supports providing some of the data: https://sdk.mystenlabs.com/typedoc/interfaces/_mysten_wallet_standard.UnresolvedObjectArg.html

crates/sui-e2e-tests/tests/rest.rs

crates/sui-rest-api/src/client/sdk.rs

crates/sui-rest-api/src/transactions/resolve.rs

amnn

This is looking great, thanks @bmwill -- sorry it took so long for me to get around to reviewing it. High level thoughts:

Do we need both resolve and simulate at the REST level? It seems like simulate is offering a strict subset of the features that resolve offers today.
There are some tweaks required to get input resolution working w.r.t. shared object deletion. Hayes pointed out one (commands other than MoveCall) and I pointed out the other (shared object can be passed by value now).
Recording that we had a conversation about a form of input resolution that does basic type inference and BCS serialization where necessary.

crates/sui-types/src/transaction_executor.rs

crates/sui-e2e-tests/tests/rest.rs

amnn · 2024-09-04T11:03:44Z

crates/sui-rest-api/src/transactions/execution.rs

+) -> Result<ResponseContent<TransactionSimulationResponse>> {
+    let executor = state.ok_or_else(|| anyhow::anyhow!("No Transaction Executor"))?;
+
+    if transaction.gas_payment.objects.is_empty() {


Is it necessarily an issue to simulate a transaction without a gas payment? I thought I saw the authority codebase handled that by generating a mock gas object.

Yes, so the main reason to support a simulate_transaction api thats different from the resolve one is that the input format is different. Resolve takes a json payload that may have holes in it while this one takes a fully formed Transaction and explicitly doesn't try to fill in any holes. I'll add documentation to that affect.

crates/sui-rest-api/src/transactions/resolve.rs

amnn · 2024-09-04T11:18:52Z

crates/sui-rest-api/src/transactions/resolve.rs

+    {
+        let package = reader
+            .inner()
+            .get_object(&(move_call.package.into()))?


Will this read objects from RocksDB serially, or is it doing something more clever behind the scenes? If the latter, is there a way we can perform all the fetches concurrently, and is that even worth doing?

Yes reads are done sequentially I'm not overly concerned with perf on this part though, but we can easily optimize later

We can look at integrating sui-package-resolver here if we want to improve this. I think we will need to do something like that anyway to handle type inference on pure types etc.

crates/sui-rest-api/src/transactions/resolve.rs

stefan-mysten · 2024-10-09T22:59:29Z

crates/sui-core/src/authority.rs

+        if transaction.kind().is_system_tx() {
+            return Err(SuiError::UnsupportedFeatureError {
+                error: "simulate does not support system transactions".to_string(),
+            });
+        }


Shouldn't this be the first condition in the function, before loading the epoch?

crates/sui-e2e-tests/tests/rest.rs

crates/sui-rest-api/src/transactions/execution.rs

crates/sui-rest-api/src/transactions/resolve.rs

stefan-mysten · 2024-10-09T23:39:08Z

crates/sui-rest-api/src/transactions/resolve.rs

+                            .ok_or_else(|| {
+                                RestError::new(
+                                    axum::http::StatusCode::BAD_REQUEST,
+                                    "unable to find function",


would it be useful to specify which function is not found in which command?

That's a good point, i'll see what i can do about errors

stefan-mysten · 2024-10-09T23:53:53Z

crates/sui-rest-api/src/transactions/resolve.rs

+
+/// Given an particular input argument, find all of its uses.
+///
+/// The returned iterator contains all commands where the arguement is used an an optional index


Suggested change

/// The returned iterator contains all commands where the arguement is used an an optional index

/// The returned iterator contains all commands where the argument is used and an optional index

crates/sui-rest-api/src/transactions/resolve.rs

stefan-mysten · 2024-10-10T00:04:11Z

crates/sui-rest-api/src/transactions/resolve.rs

+                .ok()
+                .map(|coin| (object.compute_object_reference(), coin.value()))
+        })
+        .take(max_gas_payment_objects as usize);


if the gas coins with sufficient gas are above this threshold, wouldn't this condition potentially miss them?

yes if your account is just full of dust this could be an issue, but i did want to try to have some bound on how many objects we'd read in the db, at least to start. This is something we could definitely tweak if needed

stefan-mysten · 2024-10-10T00:06:41Z

crates/sui-rest-api/src/transactions/resolve.rs

+
+    // If the user didn't provide a budget we need to run a quick simulation in order to calculate
+    // a good estimated budget to use
+    let budget = if let Some(user_provided_budget) = user_provided_budget {


Should have a check that the provided budget is > than the gas price?

For now we can assume if someone set a budget themselves they know what they are doing. If they request a simulation they'd get the out of gas error as well.

stefan-mysten · 2024-10-10T00:08:50Z

crates/sui-rest-api/src/transactions/resolve.rs

+    reference_gas_price: u64,
+    max_gas_budget: u64,
+    unresolved_transaction: UnresolvedTransaction,
+) -> Result<TransactionData> {


Noob question: could we have an API that returns sui-sdk-types::types::Transaction as well, or would be better to convert this TransactionData into a Transaction where I'd need this, just bcs-serialize-deserialize the types?

this function is an internal implementation detail so i wouldn't worry about the type here. If you look at the REST entry point you'll find that it does return the the sdk types version of things

Yes, that's why I was a bit confused 😅

stefan-mysten · 2024-10-10T00:16:49Z

crates/sui-core/src/authority.rs

+            self.get_backing_package_store().as_ref(),
+        )?;
+
+        let (input_objects, receiving_objects) = self.input_loader.read_objects_for_signing(


Naive question: we want this read_objects_for_signing and not read_objects_for_execution because at signing step a lot of checks are being done?

That and the other one requires locks and is actually used for real execution, not simulation.

stefan-mysten

Had a pass, overall looks good to me and it was incredibly useful to put some pieces together in my mind regarding going from a unresolved tx to a resolved one, including seeing how the Unresolved_[X] types are being used.

I hope @amnn has some good feedback as well.

amnn

LGTM too -- most of my comments are reserved for the follow-ups!

#6929) # Description of change - Upstream range: [v1.35.4, v1.36.2) - Port the following Sui's commits which are contained in the single PR MystenLabs/sui#19029 - MystenLabs/sui@bf9ac99 - MystenLabs/sui@ea27dff - MystenLabs/sui@066a6b6 - Descriptions from commits - transaction-executor: add simulate_transaction trait method - rest: introduce transaction simulation endpoint - rest: introduce transaction resolve endpoint - Notes - The extra field `trace_builder_opt` of `execute_transaction_to_effects` was introduced in #6354 - Because the `iota-sdk-types` does not exit, use `iota-sdk2` instead - Remove stable/unstable description in openapi.json from #6922 ## Links to any relevant issues Part of #3990 ## Type of change - Enhancement (a non-breaking change which adds functionality) --------- Co-authored-by: jkrvivian <[email protected]>

bmwill requested review from Jordan-Mysten and hayes-mysten August 19, 2024 14:38

bmwill requested review from amnn, emmazzz, joyqvq, mystenmark, patrickkuo, stefan-mysten, suiwombat and wlmyng as code owners August 19, 2024 14:38

vercel bot deployed to Preview – sui-docs August 19, 2024 14:42 View deployment

bmwill force-pushed the rest branch from 180c422 to 115eb81 Compare August 19, 2024 14:47

vercel bot deployed to Preview – sui-docs August 19, 2024 14:51 View deployment

hayes-mysten reviewed Aug 19, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/resolve.rs Outdated Show resolved Hide resolved

bmwill force-pushed the rest branch from 115eb81 to 9c145a5 Compare August 20, 2024 20:38

vercel bot deployed to Preview – sui-docs August 20, 2024 20:44 View deployment

bmwill force-pushed the rest branch from 9c145a5 to 0ab784c Compare August 21, 2024 18:53

vercel bot deployed to Preview – sui-docs August 21, 2024 18:57 View deployment

amnn reviewed Sep 4, 2024

View reviewed changes

bmwill force-pushed the rest branch from 0ab784c to ce5ead5 Compare October 2, 2024 14:08

vercel bot deployed to Preview – sui-docs October 2, 2024 14:10 View deployment

bmwill force-pushed the rest branch from ce5ead5 to 41eea51 Compare October 2, 2024 14:39

vercel bot deployed to Preview – sui-docs October 2, 2024 14:42 View deployment

Emadalshamery mentioned this pull request Oct 9, 2024

https://github.com/MystenLabs/sui/pull/19029#discussion_r1793565637 Emadalshamery/emtr#12

Open

bmwill force-pushed the rest branch from 41eea51 to 5ca17be Compare October 9, 2024 15:47

vercel bot deployed to Preview – sui-docs October 9, 2024 15:49 View deployment

bmwill force-pushed the rest branch from 5ca17be to 3e1c131 Compare October 9, 2024 16:08

bmwill force-pushed the rest branch from f3c19c2 to 0759fe4 Compare October 9, 2024 17:34

vercel bot deployed to Preview – sui-docs October 9, 2024 17:36 View deployment

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-e2e-tests/tests/rest.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/execution.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/execution.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/resolve.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/resolve.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 9, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/resolve.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 10, 2024

View reviewed changes

crates/sui-rest-api/src/transactions/resolve.rs Show resolved Hide resolved

stefan-mysten reviewed Oct 10, 2024

View reviewed changes

stefan-mysten approved these changes Oct 10, 2024

View reviewed changes

bmwill added 3 commits October 9, 2024 19:58

transaction-executor: add simulate_transaction trait method

0c7be8f

rest: introduce transaction simulation endpoint

4eaa7fa

rest: introduce transaction resolve endpoint

f001ebf

bmwill force-pushed the rest branch from 0759fe4 to f001ebf Compare October 10, 2024 00:58

vercel bot deployed to Preview – sui-docs October 10, 2024 01:00 View deployment

bmwill merged commit 066a6b6 into MystenLabs:main Oct 10, 2024
45 checks passed

bmwill deleted the rest branch October 10, 2024 02:00

amnn reviewed Oct 10, 2024

View reviewed changes

jkrvivian mentioned this pull request Apr 2, 2025

Port upstream node team related changes [mainnet-v1.32.2..mainnet-v1.36.2) iotaledger/iota#3990

Closed

12 tasks

bingyanglin mentioned this pull request May 21, 2025

upstream(node): Introduce transaction resolve and simulation endpoints iotaledger/iota#6929

Merged

	/// The returned iterator contains all commands where the arguement is used an an optional index
	/// The returned iterator contains all commands where the argument is used and an optional index

rest: introduce transaction resolve and simulation endpoints #19029

rest: introduce transaction resolve and simulation endpoints #19029

Uh oh!

Conversation

bmwill commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test plan

Release notes

Uh oh!

vercel bot commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Aug 19, 2024

Uh oh!

hayes-mysten left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amnn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefan-mysten Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

bmwill commented Aug 19, 2024 •

edited

Loading

vercel bot commented Aug 19, 2024 •

edited

Loading

stefan-mysten Oct 10, 2024 •

edited

Loading

stefan-mysten left a comment •

edited

Loading