-
Notifications
You must be signed in to change notification settings - Fork 103
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
feat(marketplace): escrow sell order credits #878
Changes from all commits
501ed4d
cb104de
0ed55f4
1f901eb
ad01737
d9576a8
cf7f9d3
06f65b7
70d0d40
c7b4bdb
3ab6cf8
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.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,6 +195,9 @@ message BatchBalance { | |
|
||
// retired is the retired amount of credits | ||
string retired = 4; | ||
|
||
// escrowed is the amount of credits locked up in escrow for the marketplace. | ||
string escrowed = 5; | ||
} | ||
|
||
// BatchSupply tracks the supply of a credit batch | ||
|
@@ -220,4 +223,8 @@ message BatchSupply { | |
// cancelled, effectively undoing the issuance. The sum of total_amount and | ||
// amount_cancelled will always equal the original credit issuance amount. | ||
string cancelled_amount = 4; | ||
|
||
// escrowed_amount is the amount of credits in the batch that have been | ||
// locked up in escrow for use in the marketplace. | ||
string escrowed_amount = 5; | ||
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. Do we really need to have a separate supply here? I guess maybe that's a useful invariant? 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. it seemed right to have it as the credits aren't necessarily tradable nor retired. if anything, it might be useful to be able to query a batch to see how many credits are in the market? |
||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 escrowed there right user facing name? Maybe
in_sell_orders
?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 there a specific concern you have regarding users and the word
escrowed
? i felt like it was a good fit -in_sell_orders
seems a bit verboseThere 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.
I could go either way.
in_sell_orders
is more specific and potentially more user friendly but provided the supporting documentation,escrowed
seems ok to me as well.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.
How about you make a call @ryanchristo
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.
Ok,
escrowed
it is. More aligned withtradable
andretired
.