-
Notifications
You must be signed in to change notification settings - Fork 116
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
Updated verification API for DEx #35
Conversation
I suggest a deadline of 3-4 days for this pull request. |
As discussed, supported. |
Aside from a pluralization nitpick looks good. Just one question, regarding the Purchase Offers. Quoted from the spec,
The spec doesn't mention the outcome if you send more bitcoins than the SO requires and the SO doesn't have any more Mastercoins available. I might have missed it, but the assumption then is that those BTC are lost? Is this an assumption left to the reader? |
That's how I read it as well. The BTC will have gone to the seller. Not sure if J.R. meant something else there. |
The extra BTC would be lost in that case. That's a hypothetical for Thanks
|
That sounds right. Thanks for the clarification. |
@zathras-crypto Do you want to send a PR in for your suggestion on clarifying the Class A parsing? |
Yep, that's all coming - apologies in advance for the long post incoming
|
I modified the example to use 2 and 3. Good point. As for your other comments; I think the name changes belong in a different PR. The whole spec is full of these terms so they should be redone properly and all together. I will wait 12 more hours in case something else comes up otherwise I'm merging tomorrow morning. |
@maran 12 hours X 12 = 6 days :) |
Updated verification API for DEx
We need some more tools in the verification API in order to keep automatic consensus going. With these added fields we will be able to determine which transaction is breaking consensus when it happens.