-
Notifications
You must be signed in to change notification settings - Fork 3
feat(ffi): expose TransactionBuilder::{get_named_result(), named_command()} #274
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
base: sdk-bindings
Are you sure you want to change the base?
Conversation
…and()}; add tx_command_results example
7459299
to
ad484b4
Compare
pub fn split_coins(coin: Arc<PTBArgument>, amounts: Vec<Arc<PTBArgument>>) -> Self { | ||
Self(PTBCommandKind::SplitCoins { coin, amounts }) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These probably need a new prefix, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't really like that named_command
is exposed at all. Why would you do this rather than just calling the builder methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The builder methods currently doesn't allow to provide results from previous commands
We could change the params to make use of the PTBArgument
trait, but not sure if that's better, because just providing numbers like this .split_coins(coin, [1000, 2000, 3000])
wouldn't work anymore, you would have to specify them as u64 .split_coins(coin, [1000u64, 2000, 3000])
and if you read the fn signature it's not clear at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would look like this, and we probably should adjust all the builder methods to that then 8ec9483
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I see. This is definitely a bit of a weird case. And using PTBArgument means you can pass a lot of stuff that would be invalid :c Still, I guess the system allows you to make mistakes in any command if you don't know what you're doing, so maybe it's not such a big issue 🤷♀️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you prefer the params with the PTBArgument
trait?
Also added tx_command_results examples for #233