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 add(Re)Issuance API #88

Open
tiero opened this issue Nov 28, 2022 · 7 comments
Open

improve add(Re)Issuance API #88

tiero opened this issue Nov 28, 2022 · 7 comments
Assignees

Comments

@tiero
Copy link
Member

tiero commented Nov 28, 2022

At the moment we force uset to specify one address/amount for each asset issued and token. We should allow to issue and reissue asset to multiple outputs in the same tx.

the API could either change:

  1. accepting an array of recipients (address/script & amount in addInIssuance & addInReissuance
  2. having the user manually add the asset & token outputs, the addInIssuance & addInReissuance only care about adding entropy & assetAssetBlinder?

Toughts @altafan @louisinger

@tiero
Copy link
Member Author

tiero commented Nov 29, 2022

I would go for the second approach: is a bit counter-intuitive the fact the addIn(Re)Issuance methods also add outputs: better to let the user add the asset & token output, eventually if user forgots, we should throw an instructive errore message

@altafan
Copy link
Collaborator

altafan commented Nov 29, 2022

The API is designed like this to prevent the user to make mistakes when adding outputs. This stands ONLY for issuances:
to generate the correct asset hash of the reissuance token, the user would have to use the correct bool value that expresses whether the issuance is blinded or not.

I personally prefer the first approach, because I consider adding an issuance/resissuance to a tx not just adding stuff to inputs, but also adding some new output.

Now that we are discussing this, I think the correct name for these APIs are addIssuance and addReissuance instead of addInIssuance and addInReissuance, without any specific reference to inputs. We are still in time to make this change if we go for 1)

@tiero
Copy link
Member Author

tiero commented Nov 29, 2022

prevent the user to make mistakes when adding outputs

What if we give some convenience method out of the Pset class that introspect the input and generates the right asset?

some pseudocode?

const updater = new Updater(ptx)
updater.addInput({ reissuanceOpts: { tokenBlinder, entropy, intialBlindedIssuance }})
updater.addOutput({ asset: ptx.issuance.asset })
updater.addOutput({ asset: ptx.issuance.token })

@altafan
Copy link
Collaborator

altafan commented Nov 30, 2022

Don't really like this solution mostly because there's no asset filed in the input issuance.

Another option could be to have convenience methods AddIssuance/AddReissuance (those already existing), and AddInIssuance/AddInReissuance to add only the input part of the issuance, leaving the output one to the user, like experienced users, what do you think?

@tiero
Copy link
Member Author

tiero commented Dec 1, 2022

Let's go for AddIssuance/AddReissuance as a method of the updater, where you can pass an array of assetOutputs & outputs? We basically phase out assetAddress & amountAddress + tokenAddress & tokenAmount

updater.addReissuance({
  assetOutputs,
  tokenOutputs
})

@altafan
Copy link
Collaborator

altafan commented Dec 1, 2022

Agreed.

@altafan
Copy link
Collaborator

altafan commented Dec 1, 2022

cc @louisinger

TLDR: let's rename addInIssuance | addInReissuance to addIssuance | addReissuance and expect an array of asset/token outputs defined as [{address, amount}] instead of assetAddress | amountAddress and tokenAddress | tokenAmount in the respective args.

@altafan altafan assigned louisinger and unassigned altafan May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants