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

Implement transaction dependencies estimation #701

Closed
digorithm opened this issue Dec 20, 2022 · 8 comments · Fixed by #954
Closed

Implement transaction dependencies estimation #701

digorithm opened this issue Dec 20, 2022 · 8 comments · Fixed by #954
Assignees
Labels
feat Issue is a feature

Comments

@digorithm
Copy link
Member

Similar to the Rust SDK, the TS SDK should be able to estimate the transaction dependencies (contract input set and variable outputs).

This removes the guesswork of setting the variable outputs and contract input set when performing a call.

@digorithm digorithm added the feat Issue is a feature label Dec 20, 2022
@Dhaiwat10
Copy link
Member

I will try having a go at this one @digorithm

@Dhaiwat10 Dhaiwat10 self-assigned this Jan 17, 2023
@Dhaiwat10
Copy link
Member

Dhaiwat10 commented Jan 18, 2023

After doing some digging around this, I found this function in our codebase: addMissingVariables. It does more or less what the estimate_tx_dependencies function from our Rust SDK does, and it is automatically called by several of TS SDK's functions like sendTransaction, call & simulate. So I think the TS SDK already adds in any missing transaction dependencies for you. I also found this unit test titled 'Automatically adds variableOutputs` which, I believe, tests this behaviour.

Do you think we still need to create another function? I might be missing something so please correct me if I'm wrong 😅

cc @digorithm

@luizstacio
Copy link
Member

We currently have this solution working by simulating the tx. @digorithm @Dhaiwat10, if is now possible to know missing dependencies ahead of time without the need to call the tx then the work, is to replace the current solution.

@Dhaiwat10
Copy link
Member

Dhaiwat10 commented Apr 12, 2023

@luizstacio sorry, just seeing this when browsing the todo list. I missed your response among my Github notifications. You mentioned replacing the current solution. Why do we need to do that? Is the current solution not good enough? Or are we specifically looking for a solution that does not involve having to simulate the tx? Thanks.

@digorithm
Copy link
Member Author

Unfortunately, we still have to simulate the transaction to be able to estimate the transaction dependencies.

So, is this feature live or not? I can't seem to find the documentation for it yet!

@Dhaiwat10
Copy link
Member

Dhaiwat10 commented Apr 21, 2023

@digorithm I would say yes the feature is live, but documentation around it is missing.

Our addMissingVariables function does the same things that the estimate_tx_dependencies in the Rust SDK does.

@digorithm
Copy link
Member Author

digorithm commented Apr 21, 2023

@digorithm I would say yes the feature is live, but documentation around it is missing.

Our addMissingVariables function does the same things that the estimate_tx_dependencies in the Rust SDK does.

Ah, I see. Then I'll ask you two things: Can we rename addMissingVariables to estimateTxDependencies? Rationale: We should unify the nomenclature/terminology between the two SDKs. Please let me know what you think. The other thing: let's add this stuff to our docs; this is something users will often look for!

@Dhaiwat10
Copy link
Member

@digorithm

  1. Yes, I agree with the renaming suggestion because of the point you raised, and the fact that the addMissingVariables also adds any missing external contract IDs and not just missing variable outputs.
  2. Will add docs for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment