KIPs icon indicating copy to clipboard operation
KIPs copied to clipboard

KIP-0019: coin-v6 and fungible-v3

Open rsoeldner opened this issue 2 years ago • 12 comments

Rendered version: https://github.com/kadena-io/KIPs/blob/b49dbbcf021cab75e71c6e15cf08c2ef23b2db0c/kip-0019.md

TODO:

  • [x] Timebox proposal
  • [ ] Check gas consumption in coin-v6.repl

  • To see the specific tasks where the Asana app for GitHub is being used, see below:
    • https://app.asana.com/0/0/1204463139564860

rsoeldner avatar May 02 '23 07:05 rsoeldner

I'd suggest to update the create-account to have a creation-key-set to encourage users to create their k:account with a different keyset compared to the keysets used to sign for. This being that if your private key gets compromised that was used to create the k:account you risk getting your account squatted on chains you haven't created your account yet.

So something along the lines of:

(defun create-account(creation-keyset:guard sign-keyset:guard)
  (let ((account (create-principal creation-keyset)))
    (enforce-keyset create-keyset)
    (enforce-keyset sign-keyset)
    (insert coin-table account
      { balance: 0.0
      , guard: sign-keyset })))

EnoF avatar May 04 '23 09:05 EnoF

I have a big concern about the fungible-v3 interface.

I believe that this will break all contracts relying on coin used with a modref. Like existing DEXs.

I am wrong ?

Is it worth upgrading fungible-v2 -> funglibe-v3 just for adding some not-essential FV tests and removing (rotate ).

Furthermore (rotate ) can still be disabled in coin-v6 while keeping funglible-v2

CryptoPascal31 avatar Jun 16 '23 14:06 CryptoPascal31

@CryptoPascal31 Interfaces are not upgradeable by design, so that's the main cause of all of this kerfuffle.

emilypi avatar Jun 16 '23 16:06 emilypi

@CryptoPascal31 Interfaces are not upgradeable by design, so that's the main cause of all of this kerfuffle.

Yes I know..

I maybe wrong but that's why, I think that changing the interface implemented by coin must be avoided if not absolutely necessary. All contracts using coin as a modref will be broken.

And even worse, DEX contracts will need to be rewritten in an "hybrid way" to be able to handle both interfaces: funglible-v2 for old tokens and fungible-v3 for coin => Nightmare

CryptoPascal31 avatar Jun 16 '23 19:06 CryptoPascal31

This cannot go through in its current state. A blockchain should never break previously deployed contracts. Think of the financial implications of this. People and developers will lose trust in your network if you cause this kind of damage. You cannot expect everyone to upgrade their contracts to "keep working". This change to fungible-v3 could cause major headaches to the ecosystem. Thank you Pascal for pointing this out.

Thanos420NoScope avatar Jun 18 '23 00:06 Thanos420NoScope

Hey guys do you think it will be possible to wrap tokens with some fungiblev2->fungiblev3 wrapper contract or something similar so the code won't break and this way everyone's happy? Another option is to make the opposite way so we can downgrade coin to v2 in some places like dexes, so the rotation will be asserted as not implemented or something.. I'm not sure that we need to break everything in the current state.

michaeldoron59 avatar Jun 19 '23 10:06 michaeldoron59

@CryptoPascal31 @Thanos420NoScope @michaeldoron59 thank you for your valuable feedback. We are discussing the mentioned points to make sure these concerns are adequately addressed.

rsoeldner avatar Jun 19 '23 10:06 rsoeldner

This cannot go through in its current state. A blockchain should never break previously deployed contracts. Think of the financial implications of this. People and developers will lose trust in your network if you cause this kind of damage. You cannot expect everyone to upgrade their contracts to "keep working". This change to fungible-v3 could cause major headaches to the ecosystem. Thank you Pascal for pointing this out.

We will not break fungible-v2 nor remove it from coin-v6. We are aware of how breaking this is to the ecosystem, and it's important the DEXs keep working.

I just want to point out that the KIP is precisely for these discussions. We discuss these things internally as well, but in an effort to bring about more transparency, we're simply moving a lot of these discussions out to a public forum.

jmcardon avatar Jun 19 '23 19:06 jmcardon

We will not break fungible-v2 nor remove it from coin-v6.

When I look at the files attached in this PR, coin-v6 is not implementing fungible-v2 anymore but fungible-v3.

I may be wrong, but in my understanding, if deployed as it is, this will technically break all DEXs after the next fork.

Do you agree @jmcardon ?

CryptoPascal31 avatar Jun 19 '23 21:06 CryptoPascal31

We will not break fungible-v2 nor remove it from coin-v6.

When I look at the files attached in this PR, coin-v6 is not implementing fungible-v2 anymore but fungible-v3.

I may be wrong, but in my understanding, if deployed as it is, this will technically break all DEXs after the next fork.

Do you agree @jmcardon ?

IMO it will just break future lp creations, but not old ones. but yeah indeed it's still a massive headache to the whole ecosystem.

michaeldoron59 avatar Jun 20 '23 17:06 michaeldoron59

IMO it will just break future lp creations, but not old ones. but yeah indeed it's still a massive headache to the whole ecosystem.

No it won't work even for old pairs. For example kdSwap public functions expect modrefs as arguments implementing fungible-v2 But with the current proposal, coin will be a fungible-v3 and Pact will refuse to call a swap function because of types mismatch.

CryptoPascal31 avatar Jun 21 '23 14:06 CryptoPascal31

IMO it will just break future lp creations, but not old ones. but yeah indeed it's still a massive headache to the whole ecosystem.

No it won't work even for old pairs. For example kdSwap public functions expect modrefs as arguments implementing fungible-v2 But with the current proposal, coin will be a fungible-v3 and Pact will refuse to call a swap function because of types mismatch.

I can confirm. As is it will not work for both old and new pairs in all the DEXs. If coin will be updated implementing fungible-v3 it will affect all the functionalities across the DEXs (swap, add/remove liquidity etc).

Looking at the differences between fungible-v2 and fungible-v3, honestly I think it's not worth the effort just for add a couple of FV tests and remove the rotate function from the interface.

If the main reason is removing the rotate function I think the strategy to consider in this case it's just to disable the function in the coin contract.

peppinho89 avatar Jun 22 '23 17:06 peppinho89

#50 has done similarly but without nearly as much breakage.

edmundnoble avatar Oct 16 '24 14:10 edmundnoble