-
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
tx=20: add Action byte for New, Update, Cancel #41
Conversation
To simplify updating or cancelling a Sell offer, and to prevent an update from generating an unintended new Sell offer if the previous one had already been accepted.
Good idea adding a byte to the offer request. What about backwards compatibility with the test transactions we have done so far? |
I need the devs to weigh in on this one, especially in regards to the effect of adding this byte for backwards compatibility. I'm pretty sure this is the best solution from a user-experience perspective. |
Quick clarification - what would be envisaged for action = 2 (update) with a saleamount of 0 - same behaviour as action=3 (cancel?) |
It seems reasonable that a 0 saleamount means cancel. That begs the
and what does 0 mean in each case? We need to answer this type of question for all the transactions, e.g. is Marv Schneider On Tue, Jan 21, 2014 at 6:17 PM, zathras-crypto notifications@github.comwrote:
|
I would prefer to invalidate 0 amount transactions / time limit and perhaps even transaction fee.
This pull would invalidate all existing selling offers, which I think won't matter anyway since they are only in test. Adding legacy support (build defaulting to action=0) is probably not worth it. My 2 cents :) |
Perhaps another approach to this would be to
Commit could probably also be incorporated into any message making it final. (but maybe it is better to have a separate commit message as well) So if I issue as sale offer of 1 MSC and say I make this offer valid for 10 blocks, then anybody can respond to this offer by issuing a buy bid (using the same transaction ID) that was posted in the offer. Any such reply received within the 10 blocks would be presented to the seller in his GUI. In fact, a seller may be presented with multiple offers to chose from. He will obviously chose the one with highest bid. the price of the "bid" and his "ask" most likely will not match. So he can adjust his offer to cross with the highest bidder and issue an updated sell order with updated price (using the same transaction ID) and also issue a commit message as well to close the transaction. A seller should also be able to specifically allocate his sell to multiple buyers. Or even increase the amount he is selling if amount of the bids exceeds the amount he offered for sale and he happens to have more coins to sell. If prices for bid and ask do not cross within the specified time frame , then transaction expire. Absence of a commit message also invalidate any started transaction. The negative to this approach is that an extra "commit" message is needed which further increase the cost of the transaction. |
Bitmaster - thanks & FYI your suggested process is a significant deviation from current DEx process and would require we re-evaluate our approach from a holistic PoV - it should also be noted that including a reference transaction ID adds significantly to the payload size (it would triple it from 33 bytes to 97 bytes). As discussed in the last sync I think this topic is complex and deserves a real-time discussion. IRC is fine or Skype perhaps. Taking a stab at timezones how does 20:00 GMT 25/01 suit? |
Also +1 re Tachikoma's comments on 0 value timelimits (for payment) - I concur that could be open to abuse (perpetually locked in 'pending payment') - continuing with that logic an upper bound would also be appropriate - a timelimit of 999999999 is just as problematic. |
I can make time on 25/01 around 20:00 GMT to have a discussion about this. I think it's important @marv-engine is there as well since he has been very vocal about this issue so far. Could that time work for you? |
Speaking of FIX. The protocol has so called: Cancel-and-Replace message which is both : cancel of the previous order and updating the order with new information. Could be relevant here as a model. http://www.onixs.biz/fix-dictionary/4.2/msgType_G_71.html |
I am away during the weekend on 25th. Also 20 GMT is 4 am for me. :-) One more question on the protocol encoding in response to tathras: I may be mis-understanding some basics on bitcoin chain encoding reqs maybe? But otherwise, I don't understand why are we using so many bytes for everything. Also, could we use something like Google Buffers to encode the entire message into a compact binary form which then could be stored onto a block-chain? |
Why are we using 32 bit integers instead of 16 (or 8) bit? That'd be one for J.R. to answer, I wouldn't want to speak for him :) |
Additionally since the concept of bids has been raised, my thoughts on how this would be achieved without deviating from the current model too much:
Note: Assuming appropriate confirmations between messages It's rough - been a long day. May be a glaring hole there so feel free to tear it apart, it's just brain dump :) |
I didn't notice that the pull request specified a 32-bit integer (sorry). Interesting idea about posting bids in the block chain. My main worry would Anti-spam fees are a whole different conversation. We need them for smart I'd suggest that bids in the order book are probably out-of-scope for now, I'm probably not going to be able to join the conference call (I am VERY Thanks! On Thu, Jan 23, 2014 at 1:11 AM, zathras-crypto notifications@github.comwrote:
|
I did make the action 1 byte:
Marv Schneider On Thu, Jan 23, 2014 at 12:53 PM, dacoinminster notifications@github.comwrote:
|
That's what I thought. Sorry for the confusion. I used 32-bit integers On Thu, Jan 23, 2014 at 10:03 AM, Marv Schneider
|
So @marv-engine are we on for discussing this 20:00GMT 25/01 or would you prefer to try and arrange a time where @B1tMaster can join? |
Sorry, we should schedule this for a time that works for everyone who's interested, but ASAP. I'm generally available. If you want to be in the live mtg, let us know when you can make it. If you're comfortable just stating your position, please do that. This discussion has grown beyond just the Action byte (New, Update, Cancel) I proposed. Can we make a decision on just that part now and address the other suggested enhancements afterwards? Also we need to be clear about:
|
I replied by adding a comment to the PR. It says: Sorry, we should schedule this for a time that works for everyone who's This discussion has grown beyond just the Action byte (New, Update, Cancel) Also we need to be clear about:
Marv Schneider On Sat, Jan 25, 2014 at 12:36 AM, zathras-crypto
|
Hey guys, Can we have a an example series of transactions that triggers this race condition? I can't seem to find one anywhere. Ie user A does X, then user B does Y, then Z condition occurs etc. This helps with clarity. Re the additional byte, I'm in split minds & supportive if it's fundamentally necessary. Tachikoma has raised the possibility of invalidating all previous DEx messages. Since it's only test we can get away with this but what is the impact on branding (if any) - does it give the impression we don't have a clear direction/strategy if we state we're invalidating all DEx messages done so far? Or is this a non-issue? If we did go ahead with this method:
Thanks |
Proposed behavior:
Re: invalidating previous DEx messages - that's one reason why I proposed Marv Schneider On Mon, Jan 27, 2014 at 5:44 PM, zathras-crypto notifications@github.comwrote:
|
Thanks Marv & this is exactly what I was looking for. This really helps - I could never quite understand what the problem was to begin with & that's probably because what you've described I don't see as a problem at all :) Current behaviour:
Feedback:
TL:DR; I don't see a race condition. Am I the only one in this mindset? Tachikoma/Bitoy/Graz what do you guys think? Thanks :) |
There still is a race condition, it's just moved to the actual purchase.
On Tue, Jan 28, 2014 at 3:27 AM, zathras-crypto notifications@github.comwrote:
|
Apologies to labor this but I'm still not seeing the issue, inline:
For clarity, I'm not against an extra byte to explicitly state transaction intent, so if I'm the only one posing this position and everyone else doesn't see what I'm talking about, don't let me hold you up. With that said, I always prefer to understand the reasoning for something and preferably would like to identify the exact circumstances that we need to protect against Thanks :) On Tue, Jan 28, 2014 at 7:41 PM, Maran notifications@github.com wrote:
|
Just to be clear on this: The problem we are trying to solve is that User A created a new Purchase So if you wanted to sell 10MSC, you now might end up selling 10MSC twice Right? On Tue, Jan 28, 2014 at 10:00 AM, zathras-crypto
|
Yeah I think I've got you - thanks - where you say purchase offer you're meaning sell offer right? So let's just break this down with example block numbers (again if I'm being pedantic feel free to curse :P) BLOCK1 - Confirmed tx where seller offers 10MSC for 1BTC If the seller broadcasts a update-sell between 2 and 3, it's invalid because the funds are already reserved and the sell is still open. No new sell is created. Just my view :) |
Yeah that's exactly right. It would be easier to let the client be responsible for this. I'm just not On Tue, Jan 28, 2014 at 10:23 AM, zathras-crypto
|
Scratch that. My apologies. If the seller broadcasts an update-sell between block 2 and 3, AND it makes it into block 3, AND it is ordered after the BTC payment - boom, update becomes new sell (from the perspective of a sell message broadcast prior to the previous sell closing). Again should be handled client side - ie clients shouldn't let you broadcast a change sell if all the funds are already reserved - but do concur we can't rely on that (trust wise) - people can broadcast whatever they like - and we should have a set way of dealing with whatever is broadcast. So we rely (trust) on the behaviour that is dictated by the protocol:
The protocol will always behave the same way & it's up to clients to interpret that and allow users to interact with it (and potentially stop users broadcasting transactions they didn't mean to if that's a concern). Other than a bad client tricking a user into selling twice is there anything else in play here? Thanks for taking time to discuss mate :) |
I don't think so but I let's see what Marv has to say about our recent On Tue, Jan 28, 2014 at 10:43 AM, zathras-crypto
|
Cool - I'll just add my PoV on 'bad clients'. If a client is bad, you're (to steal a quote) attached to another object by an inclined plane, wrapped helically around an axis. ie - screwed. You're giving it the authority to sign transactions on your behalf, there's no need for trickery it could just send your balance anywhere it wanted. For broadcasted transactions, as long as the protocol handles these messages in a defined manner, it'll handle them this way regardless of user intent. EDIT: for clarity |
Zathras, Maran,
Marv Schneider On Tue, Jan 28, 2014 at 5:07 AM, zathras-crypto notifications@github.comwrote:
|
I'd like to settle this in our conference call in 45 minutes. I hope you There is no way for implementations to smoothly handle this problem without On Tue, Jan 28, 2014 at 8:23 AM, Marv Schneider notifications@github.comwrote:
|
Sounds like this was settled on the call. Time to merge? |
Oh no - did that discussion take place while I was buying the skypeout stuff to dial in to the second number? Could one of the devs just briefly summarize the conclusion for me? |
Zathras, Marv Schneider On Tue, Jan 28, 2014 at 4:55 PM, zathras-crypto notifications@github.comwrote:
|
Fair enough. Plan for existing transactions? |
Every transaction definition now includes a backward compatible version number, to accommodate changes to the definitions.
Added explanation about reading 1st byte to determine message version number. Various other spelling corrections, clean up. Still more tx definitions to complete.
several questions embedded in the text, in bold.
e.g. for updating/canceling sell & buy offers
Curious about how this treats transactions that are tiny (or, perhaps, bundled microdonations that are confirmed by an entity who is the receiver). |
Zathras: I believe these changes are backward-compatible with old Marv: This can't be auto-merged because of conflicts. Can you pull from the All: We do need to get rid of the inline questions. I hope to look at them On Wed, Feb 5, 2014 at 1:11 PM, ABIS notifications@github.com wrote:
|
These changes were included via a different pull request |
To simplify updating or cancelling a Sell offer, and to prevent an update from generating an unintended new Sell offer if the previous one had already been accepted.
Version # and Doc History need to be updated.