Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
KIP-0019: coin-v6 and fungible-v3 #43
base: master
Are you sure you want to change the base?
KIP-0019: coin-v6 and fungible-v3 #43
Changes from 2 commits
e91886c
4d0cff8
b590ada
e651a76
a6d47f0
b49dbbc
7310398
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I highly recommend not to remove
rotate
as this is one of our unique selling points over other chains where the account/address is directly tied to the public/private keypair. If your private key get's compromised on such chain you are forced to move all existing assets to a new address(keypair).Kadena's chain has the unique property that our address is not tied to the keypair (or keyset in our case). So being able to rotate is a huge benefit. Allowing users to keep the same address, but changing their keyset to sign with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @EnoF here that rotation is unique to the Pact language in that you can rotate the underlying governance of a particular account in the coin contract, but there is a distinction here that needs to be made:
The removal of
rotate
will not remove the ability for accounts to have rotatable governance. This is a misconception. It will remove the ability to arbitrarily rotate account governance from any guard to any guard. However, the feature still remains that if one uses a keyset reference and stores akeyset-ref-guard
, then keys are rotateable viadefine-keyset
. Since keysets are namespaced, users may unambiguously name and store guards they expect to rotate, and the principal account type associated with them is ther:
protocol, so they may be stored under unique account addresses as well.So the question here that needs to be answered is this: should guards be arbitrarily rotatable, or should rotation be restricted to when it makes sense?
rotate
makes any crooss-chain transaction possibly unsound in its presence: funds can be locked up if any call torotate
occurs in between the burn and creation steps on the two chains. That makes for a terrible UX! I don't think restricting rotation to just builtin Pact primitives would create the same burden of UX because there already exists a recourse for users that care about rotation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an addendum: also, keep in mind that
rotate
breaks the invariants for principal account protocols. This is extremely important! Whileenforce-reserved
in the coin contract requires thatk:
accounts be well-formed when doing any useful operation in the coin contract, one can still callrotate
on principal accounts, effectively bricking them until they are rotated back to the correct governance defined by the principal. There are only negatives here: principals don't work as intended, rotate doesn't provide any value, and we're going to be restrictingcreate-account
so that users can only interact withcoin
if they have a principal account protocol in play. The existence ofrotate
in this case only introduces surface area for griefing, with no benefits.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood this correctly, this means that users should be encouraged to create accounts like:
And then they will be able to rotate said keyset like:
Which then makes the
coin.transfer-crosschain
reference the defined keyset rather than the value of the keyset and therefore you get the SPV to become sound? If this is correct we should probably some more "encouraging documentation" around this for new devs. At least to me the use ofdefine-keyset
did not ring a bell to be used in this way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EnoF close! You understood correctly, but for the interest of the KIP process, there are some details to iron out. Namely, let's look at the call you've written:
This would not be correct because the account name for an
r:
protocol account would ber:<my-keyset>
, while the keyset ref guard would be called to wrap around a particular predefined keyset. In fact, the call to creating a rotatable account would be something like this in general:The call to
create-principal
creates anr:
-principal account name much in the same sense ask:
. They look liker:<some keyset name>
. And yes, the underlying keyset is fully rotateable via calls like:Yes, exactly. If the keyset is defined on the target chain, then the keys may rotate underneath it, but the keyset name reference is a static string, which does not change. We have documentation for this in the principal account section of the readthedocs, but I'll work on writing up a concrete tutorial/introduction and tie it in with this proposal to ease everyone's minds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emilypi I tried creating an account using a
keyset-ref-guard
,which requires me to use
define-keyset
. I was hoping to define it like:In the hopes that the keyset would be guarded like a
k:account
would be.This is not possible because you need to be on a namespace to use
define-keyset
.Which means that I would use either
free
oruser
, my keyset will not be guarded.Then I thought maybe I could use
ns-create-principal-namespace
to solve this problem:This should prevent your keyset ref from being squatted, but it is quite a bit of boilerplate.
I think it would be a good compromise between UX/DX and security if we would
upgrade the
create-account
to auto define a keyset in a way that only the originalkeyset can sign for. (i.e.
k:pubkey
thepubkey
needs to sign for the initialdefine-keyset
). Maybe this wouldn't work on thedefine-keyset
, but we could maybeintroduce something like
ks.create-principal-keyset
that would do this for you.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EnoF I see your point using
create-principal-namespace
and I thinkdefine-keyset
could implicitly use the current namespace. Maybe there are good reasons for explicitly writing the NS @emilypi ?For later reference, repl script using the KIP-0019 infrastructure showing key-rotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that
free.robert
can be squated by anyone on a different chain. So you could have definedfree.robert
with your keyset on chain 1, while I would definefree.robert
on chain 2 with a different keyset.So to prevent this from happening, we need to have
define-keyset
enforce the initial keyset used to create the definition, i.e.k:pubkey
or a different principal that could enforce such requirement.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be nice here to explain why the funds would be locked. i.e. "the cross chain transfer is permanently tied to a given guard, and rotating the keys of the coin contract makes that saved guard invalid."
It may also be good here to explain the recommended workaround: migrating to keyset references before the update to coin contract.