mastercoin-MSC / mastercore

mastercore info
mastercoin.org
MIT License
24 stars 11 forks source link

Cancel-everything: should only cancel all of one ecosystem #219

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

1) A1 starts with 50.0 MSC, 50.0 TMSC, 50 MIndiv1 and 50.0 TDiv1 2) A1 offers 50.0 MSC for 50.0 MDiv1 3) A1 offers 50.0 TMSC for 50 TIndiv1 4) A1 cancels everything in the test ecosystem (50.0 TMSC, 50 TIndiv1)

This should only cancel the offer in the test ecosystem, but it appears to cancel-everything and the scope is not limited to the given ecosystem.

Related test lines: test_cancel_everything_scope.py#L143-L540

zathras-crypto commented 9 years ago

Interesting point - spec is ambiguous on this - I actually agree with DexX on what I think is good behaviour but spec doesn't explicitly state that it is eco dependent, rather giving impression everything is cancelled regardless of anything else:

"CANCEL-EVERYTHING cancels all open orders for all currencies."

dexX7 commented 9 years ago

Hm. The reason I tend to believe it's limited to one ecosystem is the note about an explicit ecosystem field here:

https://github.com/mastercoin-MSC/spec/issues/284#issue-47442566

ghost commented 9 years ago

I think for consistency with not allowing cross-ecosystem trades in tx21, the cancel's should also only apply to one ecosystem (with cross-ecosystem behaviors being removed)

m21 commented 9 years ago

Then the spec needs to be changed to indicate that? I am fine either way. @marv-engine

dexX7 commented 9 years ago

It's currently undefined and ambigious.

dacoinminster commented 9 years ago

I think it is not limited to an ecosystem currently, and the spec should reflect that.

On Mon, Dec 1, 2014 at 9:49 AM, dexX7 notifications@github.com wrote:

It's currently undefined and ambigious.

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/issues/219#issuecomment-65104789 .

dexX7 commented 9 years ago

@dacoinminster: it's not a question, if it is limited to an ecosystem, but if it should be. Unless exceptional circumstances are given, current behavior should not dictate what ends up in the spec, but the other way around and adding such a limitation seems trivial and should not have any weight here.

I created https://github.com/mastercoin-MSC/spec/issues/290 and if you have an opinion on this, please go ahead.

dacoinminster commented 9 years ago

Well, changing the code is more than trivial - it affects not just the spec and code, but also RPC interface. When the code gets ahead of the spec I tend to go with the code unless there is an obvious problem with the way it was implemented. Perhaps this slightly violates the "sandbox" intention of Test MSC, but I'd point out that most testing is now taking place on testnet, so we probably shouldn't care.

That said, if the devs want to change this for some reason, I have no objection to changing the spec too.

I'll paste this comment in issue #290 too.

On Mon, Dec 1, 2014 at 12:55 PM, dexX7 notifications@github.com wrote:

@dacoinminster https://github.com/dacoinminster: it's not a question, if it is limited to an ecosystem, but if it should be. Unless exceptional circumstances are given, current behavior should not dictate what ends up in the spec, but the other way around and adding such a limitation seems trivial and should not have any weight here.

I created mastercoin-MSC/spec#290 https://github.com/mastercoin-MSC/spec/issues/290 and if you have an opinion on this, please go ahead.

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/issues/219#issuecomment-65133083 .

dexX7 commented 9 years ago

Addressed by #223.

marv-engine commented 9 years ago

I'm just seeing this now but my thinking is that all actions take place within an ecosystem, which had to be clearly indicated in the tx message.

On Monday, December 1, 2014, Michael notifications@github.com wrote:

Then the spec needs to be changed to indicate that? I am fine either way. @marv-engine https://github.com/marv-engine

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/issues/219#issuecomment-65104636 .

Marv Schneider VP, User Experience/Product Usability Engine, Inc. marv@engine.co