Skip to content
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

Cancel-everything: "Property identifier does not exist (Want)" #218

Closed
dexX7 opened this issue Nov 29, 2014 · 7 comments
Closed

Cancel-everything: "Property identifier does not exist (Want)" #218

dexX7 opened this issue Nov 29, 2014 · 7 comments

Comments

@dexX7
Copy link

dexX7 commented Nov 29, 2014

Via RPC interface:

  1. A1 starts with 50.0 TMSC, 50.0 TDiv1 and 0 TIndiv1
  2. A1 offers 50.0 TMSC for 500 TIndiv1
  3. A1 cancels everything in the main ecosystem (0.0 TDiv1, 9000 of a non existing test property)

This fails with the message "Property identifier does not exist (Want)".

Related test lines: test_cancel_everything_values.py#L173-L203

This is due to the input validation checks, but in the case of cancel-everything amounts and property identifiers should be ignored, as long as both properties are in the same ecosystem.

@zathras-crypto
Copy link

Same comment as other, cancel everything ambiguous.

  • CANCEL EVERYTHING = "Cancel every sell offer"
  • CANCEL EVERYTHING = "Cancel every sell offer in the ecosystem matching the supplied pair"

First option is coded now AFAIK.

@dexX7
Copy link
Author

dexX7 commented Nov 29, 2014

Either way (with or without ecosystem limitation), there should not be a property id validation, I think.

Edit: similar to how some other fields are ignored, such as amounts that can be zero. (...)

@ghost
Copy link

ghost commented Dec 1, 2014

can you provide a use case for why this behavior needs to be added? if you have an active trade, its very easy to just supply the proper propertyid to make the RPC call return successfully

Additionally as Zathras mentioned depending on future implementation changes this will need to be checked (if cancel_all is applied per-ecosystem rather than cross-ecosystem)

@dexX7
Copy link
Author

dexX7 commented Dec 1, 2014

I have no explicit use case, but everything in the Mastercoin "system" is build around the concept of two seperated ecosystems that can't interact with each other. A "cancel-everything" that is not limited to one ecosystem breaks this.

@dexX7
Copy link
Author

dexX7 commented Dec 1, 2014

Oh, sorry, wrong thread.

Additionally as Zathras mentioned depending on future implementation changes this will need to be checked (if cancel_all is applied per-ecosystem rather than cross-ecosystem)

I do not suggest do ignore property identifiers completely, but it should not matter, if they are valid. Similar to how amounts are ignored in the case of "cancel-pair" and "cancel-everything", there should be no validation of the property values. But if "cancel-everything" is indeed limited to one ecosystem, then the property values should be used to determine the ecosystem - until there is an explicit field, as suggested here: OmniLayer/spec#276 (comment)

Two scenarios:

  1. If "cancel-everything" is not limited to an ecosystem:
  • amount values should be ignored
  • property values should be ignored
  1. If "cancel-everything" is limited to an ecosystem:
  • amount values should be ignored
  • property values are used to determine the ecosystem
  • contradicting values should not be allowed (e.g. MSC, SomeTestEcosystemProperty)
  • it could be acceptable, if only one property value is given and the other is zero

@dacoinminster
Copy link

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

@dexX7
Copy link
Author

dexX7 commented Dec 9, 2014

Resolved with PR #224 and #226.

@dexX7 dexX7 closed this as completed Dec 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants