-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add asset flag to rvasps #124
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ message Transaction { | |
Account beneficiary = 2; // Target described by wallet address or email of beneficiary | ||
float amount = 3; // amount of the transaction | ||
string timestamp = 4; // timestamp of completion on the account provider side | ||
string envelope_id = 5; // envelope ID from TRISA (not included between TRISA peers) | ||
string envelope_id = 5; // envelope ID from TRISA (not included between TRISA peers) | ||
string identity = 6; // identity payload from TRISA (not included between TRISA peers) | ||
TransactionState state = 7; // state of the transaction | ||
} | ||
|
@@ -70,6 +70,7 @@ message TransferRequest { | |
string account = 1; // email address or crypto wallet of the account to debit | ||
string beneficiary = 2; // email address or crypto wallet to look up beneficiary with | ||
float amount = 3; // amount to transfer to the beneficiary (will be truncated to 2 decimal points) | ||
string asset_type = 8; // the type of virtual asset for multi-asset chains (optional) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we move this to the bottom for clarity? Also, if we're saying it's an optional field here, should it not have a default value in the CLI? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had it after amount to keep it in line with the order in the trisa repo protobuf definition but I'll move it to the bottom. Good catch on the optional tag, I added that before I put the default value in the CLI. |
||
string originating_vasp = 4; // common name of the originating VASP for demo UI error handling (optional) | ||
string beneficiary_vasp = 5; // common name of the beneficiary VASP for demo UI error handling or external demo lookup (optional if external_demo is false) | ||
bool check_beneficiary = 6; // if set, confirm that the beneficiary wallet belongs to the beneficiary VASP (optional) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the asset type supposed to be a shorthand (e.g. BTC, USD, ETH)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transfer requests I looked at have the whole word. I don't know if the TRISA whitepaper makes a recommendation, that would be something to check into.