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

Simplify transaction assembly after new base features #120

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Aug 7, 2023

This releases v0.10.1.

The changelog is just this PR:

Fixed

  • The stellar-base dependency has been upgraded to fix a TypeScript bug (#665).
  • Bundle size has decreased by refactoring assembleTransaction to use new abstractions from stellar-base (#120).

@Shaptic Shaptic added bug Something isn't working soroban-scrum labels Aug 7, 2023
@Shaptic Shaptic added this to the Soroban Testnet Release milestone Aug 7, 2023
@Shaptic Shaptic requested review from paulbellamy and sreuland and removed request for paulbellamy August 7, 2023 18:39
});

switch (raw.operations[0].type) {
case "invokeHostFunction":
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

not looking through an IDE atm, what was the warning to ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh, I was hoping this would fly under the radar 🛸 the warning is that builder.operations is not defined in the TypeScript (because it's a private property), but this is the best/only way to remove operations from a TransactionBuilder. The correct approach, I think, would be to do something like { ignoreOperations: true } as an option to cloneFrom, but that would be a "feature request", so this is an acceptable workaround imo, especially given how much better this is than the existing solution anyway.

Copy link
Contributor

@sreuland sreuland Aug 7, 2023

Choose a reason for hiding this comment

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

got it. another option for consideration in future, rather than having operation aspects just specific to cloning, i've seen a few other builder patterns with a method to clear the builder state also, like builder.clearOperations() which then can setup for using builder.addOperation() again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment to clarify!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, that may be a cleaner approach! A reset button of sorts.

@Shaptic Shaptic merged commit c0ac961 into main Aug 7, 2023
4 checks passed
@Shaptic Shaptic deleted the simpler-assembly branch August 7, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working soroban-scrum
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants