search

mastercoin-MSC / mastercore

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

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

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

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 commented 9 years ago

Same comment as other, cancel everything ambiguous.

First option is coded now AFAIK.

dexX7 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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: https://github.com/mastercoin-MSC/spec/pull/276#issuecomment-60843476

Two scenarios:

1) If "cancel-everything" is not limited to an ecosystem:

2) If "cancel-everything" is limited to an ecosystem:

dacoinminster commented 9 years ago

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

dexX7 commented 9 years ago

Resolved with PR #224 and #226.