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

Change of issuer: locked for old properties? #293

Open
dexX7 opened this issue Dec 9, 2014 · 8 comments
Open

Change of issuer: locked for old properties? #293

dexX7 opened this issue Dec 9, 2014 · 8 comments

Comments

@dexX7
Copy link
Member

dexX7 commented Dec 9, 2014

With #257 it is possible to change the issuer or maintainer of a property, however it is currently undefined, if this ability is only granted for managed properties or all properties, including those that were not created via tx 54: New Property with Managed Number of Tokens.

So the question is: should it be restricted to managed properties or available for every property?

@dacoinminster
Copy link
Contributor

The "issuer" has no special privileges for non-managed properties currently, but that may not always be true.

I'd suggest this change should be usable with any property, even though it currently only has an effect for managed properties.

@dacoinminster
Copy link
Contributor

p.s. clarifying pull request welcome :)

@dexX7
Copy link
Member Author

dexX7 commented Dec 9, 2014

Actually it's the other way around and right now there no check in place to restrict a change of issuer for "old" properties (created with tx 50, tx 51).

Because one might say this is a retroactive change, I think it's worth to discuss, if this is correct behavior.

The security aspect is two sided, and this is addressed @ #294 as well: there is a benefit, if there is the ability to transfer ownership, even in restrospective, but on the other hand: if a key got compromised and an asset was locked right from the beginning, there is no risk to begin with.

@dacoinminster
Copy link
Contributor

I think this is correct behavior. Transferring ownership for a TX50 or TX51
coin currently does not allow the attacker to do ANYTHING.

If private key is compromised for a managed property, the attacker can
ALREADY do bad things (e.g. issue new coins). Transferring ownership to
another address doesn't really help the attacker that I can see . . .

On Tue, Dec 9, 2014 at 9:15 AM, dexX7 notifications@github.com wrote:

Actually it's the other way around and right now there no check in
place to restrict a change of issuer for "old" properties (created with tx
50, tx 51).

Because one might say this is a retroactive change, I think it's worth to
discuss, if this is correct behavior.

The security aspect is two sided, and this is addressed @ #294
#294 as well: there is a
benefit, if there is the ability to transfer ownership, even in
restrospective, but on the other hand: if a key got compromised and an
asset was locked right from the beginning, there is no risk to begin with.


Reply to this email directly or view it on GitHub
#293 (comment).

@dexX7
Copy link
Member Author

dexX7 commented Dec 9, 2014

... does not allow the attacker to do ANYTHING.

Oh well, that's actually right. :)

The only case that I'm not able to predict at the moment is a transfer of ownership during an ongoing crowdsale, but if I recall correctly, then it shouldn't have an effect.

@zathras-crypto
Copy link
Contributor

Great point @dexx, we'd have to model what the behaviour would be if the
issuer was changed mid-crowdsale but I think we should restrict this anyway

  • changing address half way through a crowdsale is at least by some
    capacity going to leave people sending tokens to the old address and
    getting nothing in return - interesting case...

On Wed, Dec 10, 2014 at 6:31 AM, dexX7 notifications@github.com wrote:

... does not allow the attacker to do ANYTHING.

Oh well, that's actually right. :)

The only case that I'm not able to predict at the moment is a transfer of
ownership during an ongoing crowdsale, but if I recall correctly, then it
shouldn't have an effect.


Reply to this email directly or view it on GitHub
#293 (comment).

@dexX7
Copy link
Member Author

dexX7 commented Dec 11, 2014

Meh, ... the behavior is not transparent for the end user at the moment.

  • Transfer of ownership in general, and during a crowdsale is possible.
  • The transfer transaction is valid and reflected by gettransaction_MP, but property = "0" is shown.
  • After the transfer of ownership all three calls, namely getactivecrowdsales_MP, getcrowdsale_MP, getproperty_MP, show the new owner as "issuer" without a reference to the old one. That's not optimal, but sort of expected. I'd prefer a full history of ownership changes.
  • Sending tokens to the old owner grants token.
  • Sending tokens to the new owner grants no token.

So it's a bit mixed at the moment and the RPC interface does not reflect the actual token movements.

I tend to lock tx 50, tx 51 per default and allow ownership changes only for managed properties, in particular to respect history, so to speak, and to avoid behavior as described - at least for now. My other thread about "locking" properties probably already hints I'm not against the idea of stateful properties and the topic in general. ;)

@dexX7
Copy link
Member Author

dexX7 commented Mar 14, 2015

Quick note to catch up on this, given that a change of issuer during a crowdsale is deactivated now, there is actually an use case in combination with #282 (Accepting not-yet-issued propeties in a crowdsale).

#282 allows to create a trustless token swap mechanism, where:

  • crowdsale A offers tokens X for Y
  • crowdsale B offers tokens Y for X

There is, however, still some risk involved, namely that a crowdsale issuer may manually close a crowdsale.

If changing the issuer during a crowdsale were enabled, it would be possible to willingly give up any control by changing the issuer to an address that (likely) no one controls, which would remove that risk completely.

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