Helpful error messages if user confuses abandonclaim/abandonsupport

I think first of all those commands will work regardless of what you are abandoning (abandonclaim should work on supports, abandonsupport should work on claims) , the code is exactly the same, except for the isminefilter which is not used actually. Which begs the question if we need two commands to do the same thing.

Second , txid should not be used here as multiple claims could in theory share the same txid (and have different vouts). So this command could end up abandoning claims that you want to keep.. Ideally we'd want to use claimId here... or at least txid + vout

I'm reviewing this, and realized that there are some other issues, with the abandonclaim and supportclaim RPC command ... I think first of all those commands will work regardless of what you are abandoning (abandonclaim should work on supports, abandonsupport should work on claims) , the code is exactly the same, except for the isminefilter which is not used actually. Which begs the question if we need two commands to do the same thing. Second , txid should not be used here as multiple claims could in theory share the same txid (and have different vouts). So this command could end up abandoning claims that you want to keep.. Ideally we'd want to use claimId here... or at least txid + vout

sonatagreen commented

2016-08-24 20:48:33 +02:00

(Migrated from github.com)

I don't think abandonclaim and abandonsupport ought to be equivalent; I think it's a useful bit of syntactic salt. It might be possible to DRY out some redundant code, though.

Using claimId instead of txid is a good idea.

I don't think abandonclaim and abandonsupport ought to be equivalent; I think it's a useful bit of syntactic salt. It might be possible to DRY out some redundant code, though. Using claimId instead of txid is a good idea.

kaykurokawa commented

2016-08-26 17:00:06 +02:00

(Migrated from github.com)

Nice, I will take a look int the next couple of days, kind of strange that the travis build failed though.. doesn't seem to be related to the patch

jimmykiselak commented

2016-08-27 00:37:45 +02:00

(Migrated from github.com)

Unless there is some particularly good UX reason to use claimId, this should use txhash and nOut.

jimmykiselak commented

2016-08-30 05:44:43 +02:00

(Migrated from github.com)

I've changed my mind. Txhash and nOut should be used regardless of any ux concerns.

jimmykiselak commented

2016-08-30 05:53:41 +02:00

(Migrated from github.com)

Reasoning is that it's possible to have multiple txouts ( even in the same tx) with identical claimIDs. They could be valid supports, invalid updates, or a valid update and any number of invalid updates. In all of those cases it should be possible to uniquely specify the txout to spend.

kaykurokawa commented

2016-08-30 19:37:49 +02:00

(Migrated from github.com)

I agree that we should use txhash and nout. On a side note, it looks like lbryum (and therefore lbrynet ) has this same problem where only txhash is used to specify what to abandon.

I also don't see the purpose of having two separate function abandonclaim and abandonsupport if we are going this route. It seems like abandon is sufficient.

I agree that we should use txhash and nout. On a side note, it looks like lbryum (and therefore lbrynet ) has this same problem where only txhash is used to specify what to abandon. I also don't see the purpose of having two separate function abandonclaim and abandonsupport if we are going this route. It seems like abandon is sufficient.

jobevers commented

2016-10-18 18:39:48 +02:00

(Migrated from github.com)

Should this be closed?

kaykurokawa commented

2016-11-22 00:24:00 +01:00

(Migrated from github.com)

Closing, will need to revisit when we are re writing the lbrycrd rpc calls but not high priority right now.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Helpful error messages if user confuses abandonclaim/abandonsupport #41

Pull request closed