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

txnbuild: remove asset issuer requirement for allow_trust #1330

Closed
poliha opened this issue May 24, 2019 · 2 comments
Closed

txnbuild: remove asset issuer requirement for allow_trust #1330

poliha opened this issue May 24, 2019 · 2 comments
Assignees
Labels
txnbuild 2nd-generation transaction build library for Go SDK

Comments

@poliha
Copy link
Contributor

poliha commented May 24, 2019

txnbuild requires asset issuer for allow_trust operations. This is in contrast to other SDKs that require just the asset code.
This should be changed so that only the asset code is required. Ideally we would want to limit the change to only the allow trust operation because we require asset issuers for other operations such change_trust

@poliha poliha added the txnbuild 2nd-generation transaction build library for Go SDK label May 24, 2019
@ire-and-curses
Copy link
Member

This happens because txnbuild assumes assets must have an issuer, but the underlying XDR does not. It's an odd situation, because for some uses a partially configured asset is sufficient, but for others it is not.

There are several possible approaches we could take:

  1. We could modify allow_trust so that an Asset that was passed in without an issuer would be sufficient. At the moment this will fail here. One approach is to build an xdr.AllowTrustOpAsset directly, using only the code, rather than by converting first to xdr.Asset.
  2. Another option is to change txnbuild.Asset.ToXDR() so that converting to an XDR asset without an issuer is no longer an error (matching the underlying XDR's view). This weakens the checking in change trust. To get back to the current behaviour, we would then have to add validation for the presence of the issuer to that operation's construction.
  3. A third option is to change the allow_trust API to just require a code, since this is actually all that's needed. This would be a breaking API change.

@poliha
Copy link
Contributor Author

poliha commented Jun 20, 2019

Closed in #1439. Went with the first suggestion from above as this guarantees no changes to the current API and its effect is limited to only the AllowTrust operation

@poliha poliha closed this as completed Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
txnbuild 2nd-generation transaction build library for Go SDK
Projects
None yet
Development

No branches or pull requests

2 participants