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

Do not msgpack empty leases. #174

Merged
merged 3 commits into from
Feb 1, 2021
Merged

Conversation

jannotti
Copy link
Contributor

A lease of 32 zeros should not be msgpacked, or we get signature
errors, as the Go code does not pack it. Check and remove such leases
at Transaction construction time.

A lease of 32 zeros should not be msgpacked, or we get signature
errors, as the Go code does not pack it.  Check and remove such leases
at Transaction construction time.
@jasonpaulos
Copy link
Contributor

@jannotti thanks for working on this. Can you also extend this to work for the metadata_hash field of AssetConfigTxn? It's just like lease (32 byes, should omit if all zeros).

@jannotti
Copy link
Contributor Author

jannotti commented Feb 1, 2021

Sure. I meant to look into that and check metadata rules.

This doesn't change functionality, but makes it more clear that lease
and metadata_hash are handled the same way.  Must be 32 bytes, all
zero bytes is equivalent to not being present.

We should think about how to handle the Zero address, when specified
for rekey or close_to.  I believe it should be handled similarly.
@jannotti jannotti marked this pull request as ready for review February 1, 2021 19:14
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

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