Skip to content

Conversation

@0xLeif
Copy link
Contributor

@0xLeif 0xLeif commented Nov 29, 2025

Empty strings for url, unitName, and assetName were being encoded as "au": "", "un": "", "an": "" in MessagePack instead of being omitted. Algorand's canonical encoding requires empty/default values to be omitted entirely, causing signature verification failures.

Added !isEmpty checks to properly omit empty strings from encoding.

@0xLeif 0xLeif requested a review from Copilot November 29, 2025 21:32
Empty strings for url, unitName, and assetName were being encoded
as "au": "", "un": "", "an": "" in MessagePack instead of being
omitted. Algorand's canonical encoding requires empty/default values
to be omitted entirely, causing signature verification failures.

Added !isEmpty checks to properly omit empty strings from encoding.
@0xLeif 0xLeif force-pushed the fix/empty-string-encoding branch from 092bd34 to 7713013 Compare November 29, 2025 21:32
Copilot finished reviewing on behalf of 0xLeif November 29, 2025 21:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical encoding bug in AssetCreateTransaction where empty strings for optional asset parameters were being included in the MessagePack encoding instead of being omitted. Algorand's canonical encoding requires empty/default values to be entirely omitted, and the presence of empty string fields was causing signature verification failures.

  • Added !isEmpty checks to conditionally omit empty strings from encoding
  • Applied the fix to all three optional string fields: unitName, assetName, and url

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +131 to 139
if let unitName = assetParams.unitName, !unitName.isEmpty {
apar["un"] = .string(unitName)
}
if let assetName = assetParams.assetName {
if let assetName = assetParams.assetName, !assetName.isEmpty {
apar["an"] = .string(assetName)
}
if let url = assetParams.url {
if let url = assetParams.url, !url.isEmpty {
apar["au"] = .string(url)
}
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

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

The new behavior for empty strings (unitName, assetName, url) should be covered by tests. Consider adding a test case that creates an AssetCreateTransaction with empty strings for these optional fields and verifies they are properly omitted from the encoded output.

Copilot uses AI. Check for mistakes.
@0xLeif 0xLeif merged commit 3c7c7f2 into main Nov 29, 2025
2 checks passed
@0xLeif 0xLeif deleted the fix/empty-string-encoding branch November 29, 2025 21:38
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