mastercoin-MSC / mastercore

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

Add missing "cancel" checks, "cancel-everything" ecosystem limitation #223

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

... and cancel-everything that behaves as follows:

Consider the last part as debatable of course.

dexX7 commented 9 years ago

If desired, I can split this one up, but I was too tired yesterday. Sub parts:

Imho, related to the last part, there is consensus for the general idea. I'd want a limitation of "cancel-everything" to one ecosystem, Marv seems to support it too, as the comment linked in the other thread implies, Faiz supports it for consistency, zathras supports it as well. So if you don't hate it, let's do it. ;)

JR's objection was the work needed to adjust the code - which I tackled with this PR. I can push a PR to refine the spec on this as well and imho it's not a spec violation, but the spec is rather ambigious and not clearly defined in this context.

Opinions?

Edit: not to mention all the regtests that don't fail anymore. ;)

dacoinminster commented 9 years ago

heh. Nice. If the other devs don't object and you are willing to do the spec work too, this is okay by me.

On Wed, Dec 3, 2014 at 12:16 PM, dexX7 notifications@github.com wrote:

If desired, I can split this one up, but I was too tired yesterday. Sub parts:

  • flexible input validation of trade_MP to remove constraints related to the fields that are simply junk for some actions
  • don't always reject Bitcoin as property in any case, because the property field could just have been abused as filler and come with "zero" values
  • ensure sufficient values are provided for "cancel-at-price" and "cancel-currency-pair"
  • cross ecosystem behavior and support for "cancel-everything" limited to one ecosystem

Imho, related to the last part, there is consensus for the general idea. I'd want a limitation of "cancel-everything" to one ecosystem, Marv seems to support it too, as the comment linked in the other thread implies, Faiz supports it for consistency, zathras supports it as well. So if you don't hate it, let's do it. ;)

JR's objection was the work needed to adjust the code - which I tackled with this PR. I can push a PR to refine the spec on this as well and imho it's not a spec violation, but the spec is rather ambigious and not clearly defined in this context.

Opinions?

— Reply to this email directly or view it on GitHub https://github.com/mastercoin-MSC/mastercore/pull/223#issuecomment-65481924 .

dexX7 commented 9 years ago

@dacoinminster: PR pending: https://github.com/mastercoin-MSC/spec/pull/292

dexX7 commented 9 years ago

@m21: Consider this as finalized - unless you or anyone else has anything to add of course.. ;)

With #223 test support is back and all tests pass.

m21 commented 9 years ago

TBD after the tag