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

Improve batching API #3883

Open
grarco opened this issue Oct 3, 2024 · 0 comments
Open

Improve batching API #3883

grarco opened this issue Oct 3, 2024 · 0 comments
Labels

Comments

@grarco
Copy link
Contributor

grarco commented Oct 3, 2024

Depends on #3901 for the last part.

We currently expose from the SDK the following function to batch transactions together.

namada/crates/sdk/src/tx.rs

Lines 2887 to 2889 in 418ef64

pub fn build_batch(
mut txs: Vec<(Tx, SigningTxData)>,
) -> Result<(Tx, Vec<SigningTxData>)> {

There are some issues with the API of this function. First, we require a collections of Txs but what we need, for every inner tx, is just:

  • The TxCommitments
  • A Vec<Section>

The second issue is that the resulting tx will have the Header of the first transaction in the provided argument, but this is unclear to the caller. We should instead add another argument to provide a specific Header (also because from the previous point we won't have any header anymore).

Finally, the function avoids returning duplicated SigningTxData instances to prevent the caller from attaching duplicated signatures. Unfortunately, every SigningTxData also carries the wrapper signer, so the caller might still end up signing the wrapper transaction more than once. This is not an issue so long as the intended gas payer has signed the wrapper, but it unnecessarily increase the gas cost of the batch. We should try to improve this even though it's not completely clear to me if there's anything we can do from within this function, especially because the sign function explicitly requires SigningTxData (we could pass in an extra argument to the function to opt out of wrapper signing or we could make it accept a new type).

Some changes to add_inner_tx will also be required.

The changes proposed here are breaking but we might provide them in a non-breaking fashion by marking as deprecated the current involved functions and providing some new ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant