-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Token escrow amendment updates/fixes #3336
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
Conversation
bc48af5 to
ece4883
Compare
ece4883 to
7cdf35f
Compare
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.
Very good, necessary improvements, but I did notice a couple places that still need to be changed, and noted several more that you might want to adjust (but are not necessary).
docs/references/http-websocket-apis/public-api-methods/account-methods/account_channels.md
Outdated
Show resolved
Hide resolved
docs/references/http-websocket-apis/public-api-methods/account-methods/account_channels.md
Outdated
Show resolved
Hide resolved
docs/references/http-websocket-apis/public-api-methods/account-methods/account_channels.md
Show resolved
Hide resolved
docs/references/protocol/ledger-data/ledger-entry-types/mptokenissuance.md
Outdated
Show resolved
Hide resolved
docs/references/protocol/ledger-data/ledger-entry-types/escrow.md
Outdated
Show resolved
Hide resolved
|
@maria-robobug Could you also update the detailed section on https://xrpl.org/resources/known-amendments#tokenescrow? |
Co-authored-by: Rome Reginelli <[email protected]>
Co-authored-by: Rome Reginelli <[email protected]>
…-methods/account_channels.md Co-authored-by: Rome Reginelli <[email protected]>
…-methods/account_channels.md Co-authored-by: Rome Reginelli <[email protected]>
Co-authored-by: Rome Reginelli <[email protected]>
b9aa951 to
76d7880
Compare
76d7880 to
e646e10
Compare
docs/references/http-websocket-apis/public-api-methods/account-methods/account_channels.md
Outdated
Show resolved
Hide resolved
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.
Mostly good, got a few small changes requested.
|
|
||
| {% admonition type="info" name="Note" %}If the escrow has an expiration time and isn't successfully finished before then, the escrow becomes expired. An expired escrow remains in the ledger until an `EscrowCancel` transaction cancels it, destroying the `Escrow` object and returning the escrowed XRP or fungible tokens to the sender.{% /admonition %} | ||
| {% admonition type="info" name="Note" %} | ||
| If the escrow has an expiration time and isn't successfully finished before then, the escrow becomes expired. An expired escrow remains in the ledger until an `EscrowCancel` transaction cancels it, returning the escrowed funds to the sender. |
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.
Can an EscrowCancel transaction come from anybody?
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.
From what I understand yes, any account can submit an EscrowCancel transaction.
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.
Yes, EscrowFinish and EscrowCancel transactions can both be sent by anyone as long as the conditions are met (to finish or cancel the escrow, respectively)
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.
Unless I missed it elsewhere in the doc, it might be worth mentioning that either party can finish the escrow. Not a blocker, though.
3c56eed to
3136af7
Compare
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.
LGTM
#3260, #3187