[10/x][programmable transactions] Add migration and builder #8769
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| @@ -0,0 +1,244 @@ | |||
| // Copyright (c) Mysten Labs, Inc. | |||
There was a problem hiding this comment.
Any suggestions where this should live?
There was a problem hiding this comment.
If it's only temporary, I don't think we should worry too hard about this -- but this doesn't seem like a bad place for it since SingleTransactionKind, and ProgrammableTransaction are both in a sibling module.
There was a problem hiding this comment.
Having said that, you may want to re-use the builder without the migration logic in the SDK? cc @patrickkuo
| Self::ProgrammableTransaction(pt) => { | ||
| Either::Right(Either::Left(pt.shared_input_objects())) | ||
| } | ||
| _ => Either::Right(Either::Right(iter::empty())), |
There was a problem hiding this comment.
We should implement all dynamic dispatch like this
| @@ -1586,10 +1609,10 @@ impl TransactionData { | |||
| self.kind.shared_input_objects() | |||
| } | |||
|
|
|||
| pub fn move_calls(&self) -> Vec<&MoveCall> { | |||
| pub fn legacy_move_calls(&self) -> Vec<&MoveCall> { | |||
There was a problem hiding this comment.
Can we add a document here explaining what's legacy about these move calls, and why you may not want to use this anymore, after programmable transactions takes over the world (i.e. what goes wrong in that case, if you keep using this function).
| @@ -0,0 +1,244 @@ | |||
| // Copyright (c) Mysten Labs, Inc. | |||
There was a problem hiding this comment.
If it's only temporary, I don't think we should worry too hard about this -- but this doesn't seem like a bad place for it since SingleTransactionKind, and ProgrammableTransaction are both in a sibling module.
| amounts, | ||
| }) => self.pay(coins, recipients, amounts)?, | ||
| SingleTransactionKind::PaySui(_) | SingleTransactionKind::PayAllSui(_) => { | ||
| anyhow::bail!( |
There was a problem hiding this comment.
Is this an existing limitation of PaySui and PayAllSui (that they can't exist in a batch) or a new limitation from programmable transactions?
There was a problem hiding this comment.
You cannot map PaySui into a SingleTransacitonKind, but only TransactionData because the coins need to be in the gas vector with gas smashing
| @@ -0,0 +1,244 @@ | |||
| // Copyright (c) Mysten Labs, Inc. | |||
There was a problem hiding this comment.
Having said that, you may want to re-use the builder without the migration logic in the SDK? cc @patrickkuo
| ) -> anyhow::Result<()> { | ||
| let mut coins = coins.into_iter(); | ||
| let Some(coin) = coins.next() | ||
| else { |
There was a problem hiding this comment.
I think this is the RFC formatting for let else
There was a problem hiding this comment.
There was a problem hiding this comment.
I think the RFC formatting for this let-else would be
let Some(coin) = coins.next() else {
anyhow::bail!("coins vector is empty");
};According to this section
There was a problem hiding this comment.
Although I've got to say, the rules as written in the RFC are super ambiguous in general, and I for one welcome our rustfmt overloads, whenever they choose to impose their iron will on let-else. Until then, I don't have a strong opposition to this formatting, it just scanned strangely for me.
- Added a programmable transaction builder - APIs are there mostly for migration
Description
Adds a programmable transaction builder. APIs are there mostly for migration.
Builds on #8767
Test Plan
Not yet in use
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes