Skip to content

add tecMPT_ISSUANCE_NOT_FOUND code#33

Merged
gregtatcam merged 3 commits intogregtatcam:feature/mpt-v1from
shawnxie999:fix-issuance-code
Sep 9, 2024
Merged

add tecMPT_ISSUANCE_NOT_FOUND code#33
gregtatcam merged 3 commits intogregtatcam:feature/mpt-v1from
shawnxie999:fix-issuance-code

Conversation

@shawnxie999
Copy link
Copy Markdown

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@shawnxie999 shawnxie999 changed the title add issuance not found code add tecMPT_ISSUANCE_NOT_FOUND code Sep 6, 2024

mptAlice.create({.ownerCount = 1, .holderCount = 0});

mptAlice.authorize({.account = &bob});
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't need to authorize.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think we should keep the test as it is, I added another test case where the holder didn't authorize

}
else
return tecINTERNAL;
return tecMPT_ISSUANCE_NOT_FOUND;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If a sender is not the issuer, need to check if mpt issuance doesn't exist. I.e., if it doesn't exist then the error is tecMPT_ISSUANCE_NOT_FOUND and if mpt token doesn't exist then the error is tecNO_AUTH.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Could you clarify? The code currently does what you have described.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If a sender is not the issuer then we are getting MPToken object. If this object doesn't exist then tecNO_AUTH is returned. This is not correct error if MPTokenIssuance object doesn't exist. I think we should check at the start of the function if MPTokenIssuance doesn't exist and return tecMPT_ISSUANCE_NOT_FOUND.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i moved the check to the start of function

@gregtatcam gregtatcam merged commit 8fd18a5 into gregtatcam:feature/mpt-v1 Sep 9, 2024
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