mastercoin-MSC / mastercoin-tools

GNU Affero General Public License v3.0
9 stars 12 forks source link

Edge cases: payment with multiple outputs in one tx, ambigious txs #52

Open dexX7 opened 10 years ago

dexX7 commented 10 years ago

Current logic:

If type is basic:

Source: https://github.com/mastercoin-MSC/mastercoin-tools/blob/f5d382fb33b2736101df415c310072c414dc6acc/msc_parse.py#L182

Payment is parsed here: https://github.com/mastercoin-MSC/mastercoin-tools/blob/f5d382fb33b2736101df415c310072c414dc6acc/msc_utils_parsing.py#L108

I think there may be some edge cases where a transaction is a payment, but with more than two outputs overall. This would currently slip through and be considered as class A type-ish transaction.

On a longer term I think it would be beneficial, if transactions are not parsed in a "this or that" way, but based on parsing priorities. What I mean is something like:

dacoinminster commented 10 years ago

I'm not sure I understand. Can you give an example in which the current implementation would cause a problem?

On Mon, May 5, 2014 at 9:29 AM, dexX7 notifications@github.com wrote:

Current logic:

  • In the case at least at least one multisig output exists, type is not basic.

If type is basic:

  • If number of outputs > 2: start class A parsing
  • Otherwise: start BTC payment parsing

Source: https://github.com/mastercoin-MSC/mastercoin-tools/blob/f5d382fb33b2736101df415c310072c414dc6acc/msc_parse.py#L182

Payment is parsed here: https://github.com/mastercoin-MSC/mastercoin-tools/blob/f5d382fb33b2736101df415c310072c414dc6acc/msc_utils_parsing.py#L108

I think there may be some edge cases where a transaction is a payment, but with more than two outputs overall. This would currently slip through and be considered as class A type-ish transaction.

On a longer term I think it would be beneficial, if transactions are not parsed in a "this or that" way, but based on parsing priorities. What I mean is something like:

  • Try parse as class B multisig transaction
  • If invalid, fallback and try to parse as class A transaction
  • If invalid, fallback and try to parse as Bitcoin payment

— Reply to this email directly or view it on GitHubhttps://github.com/mastercoin-MSC/mastercoin-tools/issues/52 .

dexX7 commented 10 years ago

Will post a probably more length thread on the spec soon, since this is a broader topic than I thought in the first place. What I meant here specifically: say I accept an offer and want to pay for it in BTC. The transaction looks like this:

Output 1: Exodus Output 2: Receiver, 3 BTC Output 3: Receiver, 5 BTC Output 4: Change

While I sent a payment of 8 BTC to the seller, I probably end up with nothing, because the transaction is not recognized, because only transactions with 2 or less (besides Exodus) outputs are considered as payment transaction.

dacoinminster commented 10 years ago

I think our answer to that is: don't do that :)

Keep in mind that we control the UI which generates these messages. Only somebody running an experiment would potentially make that mistake.

On Tue, May 6, 2014 at 4:14 PM, dexX7 notifications@github.com wrote:

Will post a probably more length thread on the spec soon, since this is a broader topic than I thought in the first place. What I meant here specifically: say I accept an offer and want to pay for it in BTC. The transaction looks like this:

Output 1: Exodus Output 2: Receiver, 3 BTC Output 3: Receiver, 5 BTC Output 4: Change

While I sent a payment of 8 BTC to the seller, I probably end up with nothing, because the transaction is not recognized, because only transactions with 2 or less (besides Exodus) outputs are considered as payment transaction.

— Reply to this email directly or view it on GitHubhttps://github.com/mastercoin-MSC/mastercoin-tools/issues/52#issuecomment-42371519 .