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

feat(ifl-774): asset constructor use public address #3863

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

jowparks
Copy link
Contributor

Summary

The signature for Asset constructor was taking spending key, but it immediately converted it to public address in napi method. Having this take the public address instead allows us to calculate RawTransaction size deterministically without the spending key. Which means we can calculate size of a transaction with a view only wallet.

Testing Plan

Tests pass

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Any user of the ecosystem that is constructing an Asset manually will break. I think it is unlikely anyone is doing this yet.

[x] Yes

@jowparks jowparks requested a review from a team as a code owner April 28, 2023 18:54
@jowparks jowparks force-pushed the joe/ifl-774/asset-constructor-public-address branch from a601146 to fb12b3e Compare April 28, 2023 18:55
@jowparks jowparks changed the base branch from master to staging April 28, 2023 18:55
@jowparks jowparks force-pushed the joe/ifl-774/asset-constructor-public-address branch from fb12b3e to 2f4ba91 Compare April 28, 2023 19:01
@jowparks jowparks merged commit 621650a into staging Apr 28, 2023
@jowparks jowparks deleted the joe/ifl-774/asset-constructor-public-address branch April 28, 2023 21:18
@dguenther dguenther mentioned this pull request May 9, 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

Successfully merging this pull request may close these issues.

2 participants