From d6f734dd6ebe4b291624c90c75840062dabaf563 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Tue, 27 Aug 2024 13:12:34 -0400 Subject: [PATCH 1/4] Remove old registration flow (#1030) * Remove outdated test I thought this had already been removed, guess I missed it * Remove registration related propogation pallet code * Remove Registry pallet genesis config We don't pre-populate registered accounts anymore. We could do this again in the future, but the setup is a little tricker since we need to ensure that the network is jumpstarted before registration can happen. * Comment out old registration code in registry pallet I want to remove it to see where errors pop up, but still keep the old code visible as a reference. * Bump metadata * Remove unused functions in client * Get rid of `new_user` endpoint * Fix user registration test compilation * Inline DKG helper function * Remove `on_chain` flag from test CLI * Get rid of `register` benchmarks * Delete old code in `Registry` pallet * Remove "on_chain" suffix from registration related names * Move register extrinsic after jump start extrinsics * Remove split when getting signers from chain * Bump metadata * RustFmt * Get rid of old registration benchmark * Add `CHANGELOG` entry * Update breaking changes notes in `CHANGELOG` * Fix `test_signing_fails_if_wrong_participants_are_used` test * Remove irrelevant test * Remove outdated docs --- CHANGELOG.md | 9 +- crates/client/entropy_metadata.scale | Bin 209025 -> 207747 bytes crates/client/src/client.rs | 95 +------ crates/client/src/substrate.rs | 22 +- crates/client/src/tests.rs | 2 - crates/client/src/user.rs | 19 +- crates/test-cli/src/lib.rs | 6 +- crates/threshold-signature-server/src/lib.rs | 9 - .../src/user/api.rs | 97 +------ .../src/user/tests.rs | 101 ++----- .../tests/register_and_sign.rs | 2 - .../tests/sign_eth_tx.rs | 2 - node/cli/src/chain_spec/dev.rs | 28 +- node/cli/src/chain_spec/integration_tests.rs | 26 +- node/cli/src/service.rs | 5 - pallets/propagation/src/lib.rs | 62 +---- pallets/propagation/src/tests.rs | 48 +--- pallets/registry/src/benchmarking.rs | 28 +- pallets/registry/src/lib.rs | 256 +++++------------- pallets/registry/src/tests.rs | 96 ++----- pallets/registry/src/weights.rs | 65 +---- runtime/src/weights/pallet_registry.rs | 29 +- 22 files changed, 178 insertions(+), 829 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c587380e9..57ed829bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ At the moment this project **does not** adhere to fields, `pallet_staking_extension::initial_signers`, `pallet_parameters::total_signers`, and `pallet_parameters::threshold`, which are used to set up the initial threshold signing configuration for the network. +- In [#1030](https://github.com/entropyxyz/entropy-core/pull/1030), the registration flow got + cleaned up. A lot of storage entries, events, and extrinsics were removed from the `Registry` + pallet. The genesis build config was also removed. Additionally, the `new/user/` HTTP endpoint in + the TSS was removed since it was no longer necessary. ### Added - Jumpstart network ([#918](https://github.com/entropyxyz/entropy-core/pull/918)) @@ -34,8 +38,9 @@ At the moment this project **does not** adhere to - Fix TSS `AccountId` keys in chainspec ([#993](https://github.com/entropyxyz/entropy-core/pull/993)) ### Removed -- Remove `prune_registration` extrinsic ([#1022](https://github.com/entropyxyz/entropy-core/pull/1022)) -- Remove `confirm_registered` extrinsic ([#1025](https://github.com/entropyxyz/entropy-core/pull/1025)) +- Remove `prune_registration` extrinsic ([#1022](https://github.com/entropyxyz/entropy-core/pull/1022)) +- Remove `confirm_registered` extrinsic ([#1025](https://github.com/entropyxyz/entropy-core/pull/1025)) +- Remove old registration flow ([#1030](https://github.com/entropyxyz/entropy-core/pull/1030)) ## [0.2.0](https://github.com/entropyxyz/entropy-core/compare/release/v0.1.0...release/v0.2.0) - 2024-07-11 diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index b64bce46745bbf632b61dadc91ce674f652dec30..dfc2ecfa21135ae47c209279c98a5d52de690450 100644 GIT binary patch delta 6212 zcma)Ae_T|@wVyNh^5eoH%a7IN_YXxy5Eta9f)#-zBp(b2SWyXX-i>JOALTRL znK|>FGv}O{IcLtUxnTO`A56Z@s>dGO*W-Ih0v|Rg#emWqD2MsMaTXV7gFaRsm@A(G z*v=EqKp3}0Q-AhKU@Y{r6MfpUVb`4L+C4 zjAWdJ1?&IV7;Pzqv*G zfp=9v6x+Hb4nAW?x3t1VHs!_V;4=HK7oUMa_DEL)(r;}Y!#?OLh5=U57v<-!bF?gR zo@eP>$Cw9z5SmHOi|PW93p&)fbpx`wwRJI^XZCGr!2&7#Bv z))uF$+1`{zL9QZ@>TFeAc8|l^a)&go+1^rb_c&ec28Lv@=U<%zdF-oKV>0q6~42{ z+tds+U!f3?eX@HhlrVWuK9sV;J(JJ^O?#s8yME6CC}S7*6ih9nvZ=_qLYAwbf)bhF zhv*+#O$ioMy@r}e4YnkI9P%f%z16Q}O4Lr-w$L$b$LkTGvcs=00tbuPH&<_uWqe;X z_6P6u8}?`DoON9v%&!3ugN$q5wpvgHRBvXebA$ojd1kFnx6qM(^A zdgJHvCJKl2!TjEM+Nlp7?v0bXDICX+5-4VupCs329@)C+Fj7D}R4JZKr&B4TQ0G-X8}Y?9cCy zM^OU%O7ts~N!RJ4QEW$F9IWS0FMyc14HRsJO-;@fYQ0#@P4EV+*LNT_m29B=n3;yL zkPqyziMc=644o|K!zS3w`cFr&p{oje@x%6edLgXzqh6%k`pAwF)t!1Iri((ne#0_s zwYb%!OKfVmzNy`my))X%zC1O_%OmErh2?%c)9Cq{>s7bMSV|0>__*K>tQ=IPCSH!ghV9l`lKFqv4H*^_}$z85Zz7rZ3j%?SC)b=A4rBkHs#Ye-1bL5%|ipN z|Fj7Wbp6xixHXMu>^j}X{(hzq)y@BXB7W?Wd%lve=7}toy&iR|&o8 z03}%TIp@Wk^VpH_UrZLppcg5@q601qz-0jt(jY)sh+d@xiw+nTfMEf^vn2pG1>hDX zSabkL2mqWd`H)pQLKu@O62YP)f&?N+AbbrL!ot)d5iB|&LI5HJz|Q~(v(p5LV9^0d z0+1vC!dsybVk_umiD1zI=>m{0;rfzdAjJOAJc(e@5d{KKfE_8o0EjK4B@)4+14;#; zQ~&}EfY?S_ArUM(pjrT`1;Atg#MaVUiD1zIcBv8(c7X^o5UM~pB!WdpGz%8Z0x-$| zh%-RjB!Wc;vT&-_AZyrP&L)Me6U^32 z1Ph)evOyvnSp0w+FWK`0Nwia98z;v04h(bx-e~9N-=hC$I9G+=!{_EeS8wq7lR&$r z-m_oi8GxAHDSx~uE1Lz^&6ISpn2Tvjx9FM4E}a>xy%fY&UfdqmC4tF2+v#jYp-q*l zM;03wtbWFc)bJgZn6U#tc31qnlYd^nvbXtZc#6C%s;1 z=&A=rSFcs$jJK}^062P2{H=gu$a(sDI!@2No{it;>k)S-hprbR<%jEU;#d22CeFL@ z?IfJ{uTNk%zFopPzWWpzlz;!cSGwNY-{Wh0N~{k0 z!7|}_{Q<4OSNOx5ut~Ei@VL+_X5U!;+W^SYn|cryzPj*sBzFhG1f6q)uX(Y(sYyib zB<;&U_!Bi2)NsLI)QSH!81`fJq8a8#;Bm^Gqc%9)9+%x!it%zK0aJ^*LWBq_pAdp8 zS$J&-?27(pNUL!u^?1T_LScnhwUePR#gK)Y!(iG-a};-ld55h0tuScNha!1?ICxO! zo^V(wRL7vBO|-x#64daUR(Kh@wCxd)Ps1_r`QxE$QSs23E1k_Mo;qzP3ceD@7vtxT zstYqk#-i|sXh-6}e`NE5`IcBXV3;_PFO7q|dtkceU&O(5ooeM7@pqcPiihQ1syPAP zHCR4sNZhFpMEZG6B9y4)DPzS#+8@Wluy^;gn@K2(cPEo3z%_4k`b7A^XllvH@HK$K zN2kCmUdjHD0yblmeffF_&rgLITot2r1%EA7sHtH)1&ttRIlMRxEYw2TqAM2Ong#*5 zhfk(~_t5hFQy_}}ISq2W%=Lcx6u_G)Y$(QSji#^JrP`Ye>WB#ZH#YdYSL`>_VXxjC zr~O+7^y3xh3$vh1ES}IY#>?EaE5!$>$sE`b#_Fge0`a-yN>0$U4)gop?P5`v?ig{2BSG5=B;SGTf-d_Wkv3h$x zIE`L1uUY^Z-bW>D&~_~VUm0)2f7ZdRuo@aY%id&1!N5xV!QW9|qa9uZSNxRql3a)_5(3Z#9oqu2I{d;mIE1foZA&-!MBza9VZgsl+Rh{JaVXZVPaq-?HgWV> zG1~6WVTPGs`x>^uW&Z3SOv38WASA&cFFXqA-24srkG|>;R*^MUiF6UeJv|6y*q`-m zj7)X8oG#312;}F^c2+fFLe=POs((anM-PxW+vzc!m+DgSiO*pTgk@5aTiRM!Uyo3C ziI`3#9a-4sX>__AKSQKUY_px^E@y+w-i)D(`0)HIU`_EQ^U56VW|4uZ^>bC1V@bQC zr9rIaCnk92+M67D{zv@zwkvpI6@K^%L_`LVs)yT}TPyMEdk|LSY;J8*J-ALFzj6g{ zs)+{;VHLzvh9HiPB7A!-B*IPZ9D)J3rQ5+`Gd4}Z9OOq!AbXMRYp+~|qdd<3C1WTcIkC-FLz&R z(J}aHlA{xpBqdpytYj&9N`XQW_{JL$$rEp&H%T(ujW_VAUv>k1#-1CH$bT(bU)_N5 z0qF{uN?dkA(zP)+p~nna#)Lm9R3zsRUo~wS`s&5#tn;`zfKH#frE~#bTr`YZF8wGD2EHE9Z*%7Wa5b_L!BvduHf4OY4Y4^MYb9_yPh^H z_XU><#AEVSgSa`WyRFINkLmSlxddtv>;LF7nXE>x1kK=*Nv#2{<*R~dBA;oZFIk z!z!KpT{HbDPPT^7R`Wk*$ozKQi3!}}_-uuY=!{`L`n`gslMoP~; zf*ibGkb{OG2mgV~(&>lpnR(>?nMaJ7qN2wXf(17;v(J|t=fmMN1!}bf3w_oXhWY*| zia#0h3sE#z_EGNoeK850#N22c#84Sc)5SGkWE!LON7KK^5Tw18Kp&$JqD73OjQ~FU zsU$ibV))S{ni+<_)Kpn*vznZXml;1;xhHd<@$@|?;Co zQXtXQ9!HbI(@yYUA5NwxaI=0niFQDT1}PMO!&;{mO{S=5vj#cTBx5WXHJx4q=+vNq zzUc$q+FP^e`2fWKM;U#_7j|n2RrHxy5rbZH(Jmj9xO62w;{#nh<`?uf&Wl+^hl0g; z@UQ83GdQ&DZFDx~1EOvK?du(MCq5FjjW5$#R*Y3=j?!-@;A`>2OLTEUxY;J$vRk<2 z;$;m0GBpqIdH#~h(-bKLE^4)k^dMNui^Q`;bv@v8jYJdvZh%yZK`J0nnh(R88Ytnb z?Kp2SNfXd>zhIK~(Odrf!5}Hqrw<73&FUa&1|;)$f~3bHPa(xe$tI~7V_NME`mcy+ zGau;ZIisX=$RIpen&vZ%FojnJONr(nB}Bnw8yr4x3#k^qE?DZsYZqmfUN!?J+Y!n% zB5%gu*#_kp-fxz|RwXFr+@eNx@v^Fw7^~d)`$>{ww&|1y)fUz5a3?D^15xI5c`$Z) z+9xZvi8?~}%UR0WRJ~{Hf1a{-QBk8vGpk(o7Poz|_)VtV-rkJ?UGf>qSAV0gBBYll~X)u&KT*U44u3>QJRj&a#NyIByU#a-|@(?(pdgo zq7;eG!qH=;RiPc!W;dP!p7uINeLL;qIQao>aIAEeT9(uNWomn!b482ls%v#QolCln zRidaiJW(1Y!7fcsm5T9~M#a>mwwQPG@-!*p$KGDPJWW~(m$h?gQar#Rert;K3ttB< IKdzAf2fgTEnE(I) delta 7168 zcmbtZe|%KMwVyNh?q+YYAsdpA4dllL444>T^9%BW0D%Mv5(JVU7D8CFH_0~pBkXQ~ zw8ofEv4!@b!iZC>QKBC}{D>0r=xUW#K7u^di0HHasPJm7ZK*}TLi^|gd1vn3OG4>? z{d{KcoSFH~Idf*t%$Yg+gO99zpIgm7HS9_l9yI?!0)?HEG9Z2^PELw}6D%vv3Bzn@ zT&a8tU>6^3h9vfM++_HWy%|>mSJ>4!Ck4u;S)E`SYPNQ5g8ZQmAB}2Repy{uW+=xLXMEuMRhyE0mpr+BN*+gx~Dl- zX>H-Ipr~HgrG~;r&!(lz)fSJsUUkn8_**?epDXMM_}SOnZj)sBJ(jZFk+f<cym{1E^?~VFZQ!jMi-yrW z=DYkYsuvmG$}C2Haul=l76a)Bx2v9DRBfF*TeN)*SlGVpyKrF^u@z&Aqe`mo>VRKm z14T)!FOZT*jsZAUs4cU)iVA0wSoxCBJ44s|K5w9#CABx zjy$m$F0w^GYs311{#172XVq|;Ej>OVCe-Zlw+23E&L>lCrvdpFlg~x=C6F&QsQ<~W zY*)XXCG{_c&)M4kX$S+S7{8`N>Fn9tpBm3z?q7rg1ncon6{KGVC>E`&x=Je?MTJeX z3Arqo%H#_0!Sj&Le(=<3xW-&hFQC`7n(^$7r{(QH z=z(X`=-6DrVJanQljkT*q-IHDk+rS-6@QE);+4Htvx)O075r5hXY}k_g}T535C=k z?ubg3y*CLeS;^iUsA8-3=B3wAXjrNSt39DkSGc8pzSk8B=|)3paHuY*WzuuAppMOb zE)VCSffk^F4q4>89c;3 zdc${v?mfB|wy@tG-GnvjF=tsfwbZ(yhY}k&0n9FC}1T4TgV#-_ffFbV~!^v zo=>50bGzz7OgDS?SXz8H1^Ys+Oh`AobnH85gLl9A9q3`tzgY@>?29)iqx{(8HQL@P zrdue_S_u=_!0{~D%8ne*$lON3S>^QxHmGhPB}8ynY}J+`Kc8%4SC4nW4i@~cov@Qx zPk5o94=5;O+FRq<3n$t~io?*)uD<2MrZsQhl`%lUq3yInXA`b784xR)tgYy7%6>J@ z&d$7@XLLUBP70g(&Ri@mON0qB9?nrA2i{pe8dES_VHAHjTx*bLhj$s|v+t^~k0~dg zq5E*xu7*^0^rVySr#!nIrm{)z<-s6Z@?IHE%jWmII4zgoTZc>0cFLvIJK2>}Rj6yl z`xEiI=KcGz_BZd(OgJQVpCc6Tc8N`Nh-H6}k4CWMgQDJ7P%E&n4}{fF;jt0A=#vq; z_{$Nx@SRg_HdYBSSxb@QL5lS!-b2kv74?RH%CL&wY=&)!U)^JDYDM~QK5c7wWd`MZw z7vtHk|4!}|dehUCVAA@0F8X|q1qpwmvzQEemJ&=F;G6)Q696$91c(`;mnp%d0j>zZ z6#+ma5`b$0020BZ0ZbACOjwXiq*p_T$)xcT!K5J)1tL)(&{YW*Vuop&L@;T93<1ax zfEXPhrk&g&Pm!Lcx{ykCNw3oO~SJy>!T3vB=%7b-6^q0Z_gb1_@fx zKE<=5_0tCYzWV8M7#JG!o8v%_!O%yal_6*-@eDO7{eo*hB?Bz|i)n1x7sE*d5?F0Z z0)b8(p|xHO%Vc1v;P*WMX+tl4c}9Ysp@+}z#u6+3sub&dU%f=Jkn!~btua*Ed$rI|>91EyqNK^{a=W!N)y|r)Jz!w}c5Pae9IE{GHqiYPJ9&K}QoTp3 z53PoSaumR)m|za1@ir6GWA>^EI*gV%W~juf9y7cgyF&uEe%D}YJCh^zF;Ih=Yp$BV zW`U9sXFQfqQDCOQV}k;@I-Lo=2zqvlWHh1RME;ExzC{@q9&9P+1=;YQ&>#6jHsIY^A8O?KBpjYG?mKee8-uQz3~w2r=~Lh@ z0P#FM7xo&mPUnI%+Ro0rkjUrZ^)TA@_#IHn2lH;s!O!o2vC%0?!?sMmC?8UA8Q126 z1GBz-C^opB%m?F!z>%K7S4;(Cqw5u?roucOJ>C>lUEb(4j~Wi&H4Xk@sPW6`u-{PQ z5huKlX9vHp2k1c^RY^g4Rg_vzE zfu)$8DS^)nzy~wn!6>(gZzi0gaGK94hdQHucR3VA+xa);h7M_bNrhn__DG-tx{ZnA zv=UAl9lolBR)g-C4Y3CO$m|=P`GMIu_ULT%Q=;L|!z*)?aS3*E9^c0vTV8cJK1ejm_1-v^J^S6f~E$-%%G9@B?0+qJ3g?a=40}cMRYK*CP zydiul5t6J;ZG3Uds0al=SpccpZ1aQnLoUB?7VK>BQ9GY|7Jdfj_-AJ!53`s*Lk?Wz zeZPbPzUI$h8F$$NcJU5w5D#Fy&(K3au2`_=_}*YJ5ESxY<=iSSZWecUeb;?HPdKc) z@dztj5@=}0mtlLr>%L3v#%BW7gh!2-P*Oaq20KZ_V00twfa_3N+gasyBd1V}kVs5q z;gUdow?DZ>b+2%FJ=z;KM9gGuooge1{Tz(zjS;d|7y%d|-J;2=>I%09f}Zc8EQKsy z8Vs}rUB2po8p3hZY{bYBi6zb@foKEX7DyavXb1%AT>kF5fZNmRaoy)tkza^dNoSoW zgwdatb{xhiQHxdo^tNz2i6<-nHGqr}GQu~8P7?UyuOPkHMpiEB@^#ju;|XJXtzR1k zs!b9}!|0az0beKX5>#R=fAT9l7nAreM3&4y{tB`ng((qsZ^>IlEW)6!FYc1uW$#SbqSB% zluIxLri$8GXlGjEW0zoZY>5KanxKo2lE~3ZFld7^z41{K6#)`(U`P)=WDB~;N`BZx z%cUyC!V6BJ>Pr)73{NrBLa5;j%`_8gd7GK$)pXO_YBq*(hlD&WP^aKlw@2T@BQIgN zT{R$~WeSvvDZ0@K?Y@rx+DsQX`>Aa{o=@PTw`tKv5gvqdazh494|I)$H$p_4)7$rd_sf=7Xcjs6aQV1vIQ=<136;O|CikN+^9KAJml6W86ha^09|QGJiIePQrs=B<^nEU#3!w&U495 zow(hL?X*B__qEp4$OCrzCm9kWZ5eb8g_OvfS+pJSu$i7sDgpo%2LM zSPvzJ3i!cn`UX_-^%Lm<+@ez^(PpTN{Ad#W01{jjNP1(~J0h~z|P2vrSxSJ?2fd}rl(^OUsFprn_++C z-Mi`LOyQST`e{Et4kCXG(GN^8z~Abo*RkJQ-=h~2MEh$$rju>>ocQ7ix&$9_B5#Ap z&Zp?JcvX*7K11i(@ko5|7xeE_@Xo&RceEutFFk*>LjxH@78Z;=IK^=t5(jy8M;lTr2qdA!Z_@W{~AILjLbUl&}wT_gWN7l=}K!L=;*>IYlv6K zQeNg+A*I@ot-9~_&lh(pf}#$=*+{D_-3jSl?a8SI7X*U0Ao+!Bw1ofNBqgJ_x0ZcfNp-;Ds6>E-VrZV!@c}Ky!1P?C;r$N>4a3L zH1V|w5<}Fb1gS#uDa&}FP5N$ZtzyHAi_53<@I5x^Tymdc32&@ZZrA@l!M~^4oexF! zCQ5TDbVvS_B$ZHUtFkTP7$+^IX!Yyt(w8#q1E|Tc=pPs^7L>A=UVYSg^afRaKka`mNJq?b;ZQ0tR^~SNblTi=4@o T8~}&;^*reTvyU!4p^*OvmhoZ; diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 172ac1f8b..5e1285e82 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -37,7 +37,7 @@ use crate::{ EntropyConfig, }, client::entropy::staking_extension::events::{EndpointChanged, ThresholdAccountChanged}, - substrate::{get_registered_details, query_chain, submit_transaction_with_pair}, + substrate::{get_registered_details, submit_transaction_with_pair}, user::{get_signers_from_chain, UserSignatureRequest}, Hasher, }; @@ -49,7 +49,6 @@ use futures::{future, stream::StreamExt}; use sp_core::{sr25519, Pair}; use subxt::{ backend::legacy::LegacyRpcMethods, - events::EventsClient, utils::{AccountId32 as SubxtAccountId32, H256}, Config, OnlineClient, }; @@ -76,27 +75,15 @@ pub async fn register( signature_request_keypair: sr25519::Pair, program_account: SubxtAccountId32, programs_data: BoundedVec, - on_chain: bool, ) -> Result<([u8; VERIFYING_KEY_LENGTH], RegisteredInfo), ClientError> { - let registration_event = if on_chain { - put_register_request_on_chain( - api, - rpc, - signature_request_keypair.clone(), - program_account, - programs_data, - ) - .await? - } else { - put_old_register_request_on_chain( - api, - rpc, - signature_request_keypair.clone(), - program_account, - programs_data, - ) - .await? - }; + let registration_event = put_register_request_on_chain( + api, + rpc, + signature_request_keypair.clone(), + program_account, + programs_data, + ) + .await?; let verifying_key = registration_event.1 .0; let registered_info = get_registered_details(api, rpc, verifying_key.clone()).await?; @@ -126,10 +113,7 @@ pub async fn sign( ) -> Result { let message_hash = Hasher::keccak(&message); - let registered_info = - get_registered_details(api, rpc, signature_verifying_key.to_vec()).await?; - let with_parent_key = registered_info.derivation_path.is_some(); - let validators_info = get_signers_from_chain(api, rpc, with_parent_key).await?; + let validators_info = get_signers_from_chain(api, rpc).await?; tracing::debug!("Validators info {:?}", validators_info); let block_number = rpc.chain_get_header(None).await?.ok_or(ClientError::BlockNumber)?.number; @@ -332,7 +316,7 @@ pub async fn put_register_request_on_chain( ) -> Result { tracing::debug!("Registering an account using on-chain flow."); - let registering_tx = entropy::tx().registry().register_on_chain(deployer, program_instances); + let registering_tx = entropy::tx().registry().register(deployer, program_instances); let registered_events = submit_transaction_with_pair(api, rpc, &signature_request_keypair, ®istering_tx, None) .await?; @@ -348,63 +332,6 @@ pub async fn put_register_request_on_chain( registered_event } -/// Submits a transaction registering an account on-chain using the old off-chain flow. -#[tracing::instrument( - skip_all, - fields( - user_account = ?signature_request_keypair.public(), - ) -)] -pub async fn put_old_register_request_on_chain( - api: &OnlineClient, - rpc: &LegacyRpcMethods, - signature_request_keypair: sr25519::Pair, - deployer: SubxtAccountId32, - program_instances: BoundedVec, -) -> Result { - tracing::debug!("Registering an account using old off-chain flow."); - - let registering_tx = entropy::tx().registry().register(deployer, program_instances); - submit_transaction_with_pair(api, rpc, &signature_request_keypair, ®istering_tx, None) - .await?; - - let account_id: SubxtAccountId32 = signature_request_keypair.public().into(); - - for _ in 0..50 { - let block_hash = rpc.chain_get_block_hash(None).await?; - let events = - EventsClient::new(api.clone()).at(block_hash.ok_or(ClientError::BlockHash)?).await?; - let registered_event = events.find::(); - for event in registered_event.flatten() { - // check if the event belongs to this user - if event.0 == account_id { - return Ok(event); - } - } - std::thread::sleep(std::time::Duration::from_millis(1000)); - } - - Err(ClientError::RegistrationTimeout) -} - -/// Check that the verfiying key from a new signature matches that in the from the -/// on-chain registration info for a given account -pub async fn check_verifying_key( - api: &OnlineClient, - rpc: &LegacyRpcMethods, - verifying_key: VerifyingKey, -) -> Result<(), ClientError> { - let verifying_key_serialized = verifying_key.to_encoded_point(true).as_bytes().to_vec(); - - // Get the verifying key associated with this account, if it exist return ok - let registered_query = - entropy::storage().registry().registered(BoundedVec(verifying_key_serialized)); - let query_registered_status = query_chain(api, rpc, registered_query, None).await; - query_registered_status?.ok_or(ClientError::NotRegistered)?; - - Ok(()) -} - /// Changes the endpoint of a validator pub async fn change_endpoint( api: &OnlineClient, diff --git a/crates/client/src/substrate.rs b/crates/client/src/substrate.rs index 519316f84..c8dfe4b41 100644 --- a/crates/client/src/substrate.rs +++ b/crates/client/src/substrate.rs @@ -119,25 +119,11 @@ pub async fn get_registered_details( ) -> Result { tracing::info!("Querying chain for registration info."); - let registered_info_query = - entropy::storage().registry().registered(BoundedVec(verifying_key.clone())); - let registered_result = query_chain(api, rpc, registered_info_query, None).await?; + let registered_info_query = entropy::storage().registry().registered(BoundedVec(verifying_key)); - let registration_info = if let Some(old_registration_info) = registered_result { - tracing::debug!("Found user in old `Registered` struct."); - - old_registration_info - } else { - // We failed with the old registration path, let's try the new one - tracing::warn!("Didn't find user in old `Registered` struct, trying new one."); - - let registered_info_query = - entropy::storage().registry().registered_on_chain(BoundedVec(verifying_key)); - - query_chain(api, rpc, registered_info_query, None) - .await? - .ok_or_else(|| SubstrateError::NotRegistered)? - }; + let registration_info = query_chain(api, rpc, registered_info_query, None) + .await? + .ok_or_else(|| SubstrateError::NotRegistered)?; Ok(registration_info) } diff --git a/crates/client/src/tests.rs b/crates/client/src/tests.rs index 68667b841..f04df75c1 100644 --- a/crates/client/src/tests.rs +++ b/crates/client/src/tests.rs @@ -147,14 +147,12 @@ async fn test_remove_program_reference_counter() { .unwrap(); // Register, using that program - let register_on_chain = true; let (verifying_key, _registered_info) = register( &api, &rpc, program_owner.clone(), AccountId32(program_owner.public().0), BoundedVec(vec![ProgramInstance { program_pointer, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index 679bfc2b3..99dc1a67a 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -43,22 +43,11 @@ pub struct UserSignatureRequest { pub async fn get_signers_from_chain( api: &OnlineClient, rpc: &LegacyRpcMethods, - with_parent_key: bool, ) -> Result, SubgroupGetError> { - let mut validators = if with_parent_key { - let signer_query = entropy::storage().staking_extension().signers(); - query_chain(api, rpc, signer_query, None) - .await? - .ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))? - } else { - let all_validators_query = entropy::storage().session().validators(); - let mut validators = query_chain(api, rpc, all_validators_query, None) - .await? - .ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))?; - - validators.sort(); - validators - }; + let signer_query = entropy::storage().staking_extension().signers(); + let mut validators = query_chain(api, rpc, signer_query, None) + .await? + .ok_or_else(|| SubgroupGetError::ChainFetch("Get all validators error"))?; // TODO #898 For now we use a fix proportion of the number of validators as the threshold let threshold = (validators.len() as f32 * 0.75) as usize; diff --git a/crates/test-cli/src/lib.rs b/crates/test-cli/src/lib.rs index 78984891f..f29819fb5 100644 --- a/crates/test-cli/src/lib.rs +++ b/crates/test-cli/src/lib.rs @@ -80,9 +80,6 @@ enum CliCommand { /// If giving a mnemonic it must be enclosed in quotes, eg: "--mnemonic-option "alarm mutual concert..."" #[arg(short, long)] mnemonic_option: Option, - /// Indicates that a user wants to register using the fully on-chain registration flow. - #[arg(long)] - on_chain: bool, }, /// Ask the network to sign a given message Sign { @@ -186,7 +183,7 @@ pub async fn run_command( let rpc = get_rpc(&endpoint_addr).await?; match cli.command { - CliCommand::Register { mnemonic_option, programs, on_chain } => { + CliCommand::Register { mnemonic_option, programs } => { let mnemonic = if let Some(mnemonic_option) = mnemonic_option { mnemonic_option } else { @@ -211,7 +208,6 @@ pub async fn run_command( program_keypair.clone(), program_account, BoundedVec(programs_info), - on_chain, ) .await?; diff --git a/crates/threshold-signature-server/src/lib.rs b/crates/threshold-signature-server/src/lib.rs index ff145a3b3..b7e1212e1 100644 --- a/crates/threshold-signature-server/src/lib.rs +++ b/crates/threshold-signature-server/src/lib.rs @@ -72,14 +72,6 @@ //! //! ### For the blockchain node //! -//! #### `/user/new` - POST -//! -//! [crate::user::api::new_user()] -//! -//! Called by the off-chain worker (propagation pallet) during user registration. -//! This takes a parity scale encoded [entropy_shared::types::OcwMessageDkg] which tells us which -//! validators are in the registration group and will perform a DKG. -//! //! ### For other instances of the threshold server //! //! Takes a [UserRegistrationInfo] containing the users account ID and associated keyshare, wrapped @@ -176,7 +168,6 @@ impl AppState { pub fn app(app_state: AppState) -> Router { let mut routes = Router::new() .route("/generate_network_key", post(generate_network_key)) - .route("/user/new", post(new_user)) .route("/user/sign_tx", post(sign_tx)) .route("/signer/proactive_refresh", post(proactive_refresh)) .route("/validator/reshare", post(new_reshare)) diff --git a/crates/threshold-signature-server/src/user/api.rs b/crates/threshold-signature-server/src/user/api.rs index b410205f1..199259c55 100644 --- a/crates/threshold-signature-server/src/user/api.rs +++ b/crates/threshold-signature-server/src/user/api.rs @@ -61,10 +61,7 @@ use zeroize::Zeroize; use super::{ParsedUserInputPartyInfo, ProgramError, UserErr, UserInputPartyInfo}; use crate::{ - chain_api::{ - entropy::{self, runtime_types::pallet_registry::pallet::RegisteringDetails}, - get_api, get_rpc, EntropyConfig, - }, + chain_api::{entropy, get_api, get_rpc, EntropyConfig}, helpers::{ launch::LATEST_BLOCK_NUMBER_NEW_USER, signing::{do_signing, Hasher}, @@ -83,12 +80,6 @@ use crate::{ pub use entropy_client::user::{get_signers_from_chain, UserSignatureRequest}; pub const REQUEST_KEY_HEADER: &str = "REQUESTS"; -/// Used to differentiate different flows which perform distributed key generation. -enum DkgFlow { - Jumpstart, - Registration, -} - /// Type for validators to send user key's back and forth #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Debug, Clone, PartialEq)] @@ -197,8 +188,7 @@ pub async fn sign_tx( )?; } - let with_parent_key = user_details.derivation_path.is_some(); - let signers = get_signers_from_chain(&api, &rpc, with_parent_key).await?; + let signers = get_signers_from_chain(&api, &rpc).await?; // Use the validator info from chain as we can be sure it is in the correct order and the // details are correct @@ -220,12 +210,6 @@ pub async fn sign_tx( request_author, }; - // In the new registration flow we don't store the verifying key in the KVDB, so we only do this - // check if we're using the old registration flow - if user_details.derivation_path.is_none() { - let _has_key = check_for_key(&string_verifying_key, &app_state.kv_store).await?; - } - let derivation_path = if let Some(path) = user_details.derivation_path { let decoded_path = String::decode(&mut path.as_ref())?; let path = bip32::DerivationPath::from_str(&decoded_path)?; @@ -281,38 +265,10 @@ pub async fn generate_network_key( let data = OcwMessageDkg::decode(&mut encoded_data.as_ref())?; tracing::Span::current().record("block_number", data.block_number); - distributed_key_generation(app_state, data, DkgFlow::Jumpstart).await -} - -/// HTTP POST endpoint called by the off-chain worker (Propagation pallet) during user registration. -/// -/// The HTTP request takes a Parity SCALE encoded [OcwMessageDkg] which indicates which validators -/// are in the validator group. -/// -/// This will trigger the Distributed Key Generation (DKG) process. -#[tracing::instrument(skip_all, fields(block_number))] -pub async fn new_user( - State(app_state): State, - encoded_data: Bytes, -) -> Result { - let data = OcwMessageDkg::decode(&mut encoded_data.as_ref())?; - tracing::Span::current().record("block_number", data.block_number); - - distributed_key_generation(app_state, data, DkgFlow::Registration).await -} - -/// An internal helper which kicks off the distributed key generation (DKG) process. -/// -/// Since the jumpstart and registration flows are both doing DKG at the moment, we've split this -/// out. In the future though, only the jumpstart flow will require this. -async fn distributed_key_generation( - app_state: AppState, - data: OcwMessageDkg, - flow: DkgFlow, -) -> Result { if data.sig_request_accounts.is_empty() { return Ok(StatusCode::NO_CONTENT); } + let api = get_api(&app_state.configuration.endpoint).await?; let rpc = get_rpc(&app_state.configuration.endpoint).await?; let (signer, x25519_secret_key) = get_signer_and_x25519_secret(&app_state.kv_store).await?; @@ -330,7 +286,7 @@ async fn distributed_key_generation( return Ok(StatusCode::MISDIRECTED_REQUEST); } - validate_new_user(&data, &api, &rpc, &app_state.kv_store, flow).await?; + validate_jump_start(&data, &api, &rpc, &app_state.kv_store).await?; // Do the DKG protocol in another task, so we can already respond tokio::spawn(async move { @@ -346,7 +302,7 @@ async fn distributed_key_generation( /// Setup and execute DKG. /// -/// Called internally by the [new_user] function. +/// Called internally by the [generate_network_key] function. #[tracing::instrument( skip_all, fields(data), @@ -407,36 +363,6 @@ async fn setup_dkg( Ok(()) } -/// Returns details of a given registering user including key key visibility and X25519 public key. -#[tracing::instrument( - skip_all, - fields(who), - level = tracing::Level::DEBUG -)] -pub async fn get_registering_user_details( - api: &OnlineClient, - who: &::AccountId, - rpc: &LegacyRpcMethods, -) -> Result { - let registering_info_query = entropy::storage().registry().registering(who); - let register_info = query_chain(api, rpc, registering_info_query, None) - .await? - .ok_or_else(|| UserErr::ChainFetch("Register Onchain first"))?; - Ok(register_info) -} - -/// Returns `true` if the given account is in a "registering" state. -pub async fn is_registering( - api: &OnlineClient, - rpc: &LegacyRpcMethods, - who: &::AccountId, -) -> Result { - let registering_info_query = entropy::storage().registry().registering(who); - let register_info = query_chain(api, rpc, registering_info_query, None).await?; - - Ok(register_info.is_some()) -} - /// Confirms that the network wide distributed key generation process has taken place. pub async fn confirm_jump_start( api: &OnlineClient, @@ -463,14 +389,14 @@ pub async fn confirm_jump_start( Ok(()) } -/// Validates new user endpoint +/// Validates network jump start endpoint. +/// /// Checks the chain for validity of data and block number of data matches current block -async fn validate_new_user( +async fn validate_jump_start( chain_data: &OcwMessageDkg, api: &OnlineClient, rpc: &LegacyRpcMethods, kv_manager: &KvManager, - flow: DkgFlow, ) -> Result<(), UserErr> { let last_block_number_recorded = kv_manager.kv().get(LATEST_BLOCK_NUMBER_NEW_USER).await?; if u32::from_be_bytes( @@ -498,10 +424,7 @@ async fn validate_new_user( let chain_data_hash = hasher_chain_data.finalize(); let mut hasher_verifying_data = Blake2s256::new(); - let verifying_data_query = match flow { - DkgFlow::Jumpstart => entropy::storage().registry().jumpstart_dkg(chain_data.block_number), - DkgFlow::Registration => entropy::storage().registry().dkg(chain_data.block_number), - }; + let verifying_data_query = entropy::storage().registry().jumpstart_dkg(chain_data.block_number); let verifying_data = query_chain(api, rpc, verifying_data_query, None).await?; hasher_verifying_data.update(verifying_data.encode()); @@ -510,9 +433,11 @@ async fn validate_new_user( if verifying_data_hash != chain_data_hash { return Err(UserErr::InvalidData); } + kv_manager.kv().delete(LATEST_BLOCK_NUMBER_NEW_USER).await?; let reservation = kv_manager.kv().reserve_key(LATEST_BLOCK_NUMBER_NEW_USER.to_string()).await?; kv_manager.kv().put(reservation, chain_data.block_number.to_be_bytes().to_vec()).await?; + Ok(()) } diff --git a/crates/threshold-signature-server/src/user/tests.rs b/crates/threshold-signature-server/src/user/tests.rs index d1a580013..59d41f3d8 100644 --- a/crates/threshold-signature-server/src/user/tests.rs +++ b/crates/threshold-signature-server/src/user/tests.rs @@ -127,7 +127,6 @@ use crate::{ user::compute_hash, validator::get_signer_and_x25519_secret_from_mnemonic, }, - new_user, r#unsafe::api::UnsafeQuery, signing_client::ListenerState, user::{ @@ -196,38 +195,19 @@ async fn test_signature_requests_fail_on_different_conditions() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); - // Test: We check that an account without a program fails to submit a signature request - - let with_parent_key = true; - let (validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; - - // This verifying key doesn't have a program registered with it - signature_request.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number; - - // test points to no program - let test_no_program = - submit_transaction_requests(validator_ips_and_keys.clone(), signature_request.clone(), one) - .await; - - for res in test_no_program { - assert_eq!(res.unwrap().text().await.unwrap(), "No program pointer defined for account"); - } - // Test: We check that an account with a program succeeds in submiting a signature request + let (validators_info, mut signature_request, validator_ips_and_keys) = + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; // The account we registered does have a program pointer, so this should succeed signature_request.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number; @@ -372,22 +352,18 @@ async fn signature_request_with_derived_account_works() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, charlie.clone().into(), // This is our program modification account subxtAccountId32(alice.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); - let with_parent_key = true; let (validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; // We'll use the actual verifying key we registered for the signature request signature_request.signature_verifying_key = verifying_key.to_vec(); @@ -419,16 +395,21 @@ async fn test_signing_fails_if_wrong_participants_are_used() { let one = AccountKeyring::Dave; - let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await; - let substrate_context = test_context_stationary().await; - let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap(); - let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap(); + let add_parent_key = true; + let (_validator_ips, _validator_ids) = spawn_testing_validators(add_parent_key).await; + + let force_authoring = true; + let substrate_context = test_node_process_testing_state(force_authoring).await; + + let entropy_api = get_api(&substrate_context.ws_url).await.unwrap(); + let rpc = get_rpc(&substrate_context.ws_url).await.unwrap(); + + jump_start_network(&entropy_api, &rpc).await; + let mock_client = reqwest::Client::new(); - let with_parent_key = false; let (_validators_info, signature_request, _validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; // fails verification tests // wrong key for wrong validator @@ -511,14 +492,12 @@ async fn test_request_limit_are_updated_during_signing() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); @@ -526,10 +505,8 @@ async fn test_request_limit_are_updated_during_signing() { // Test: We check that the rate limiter changes as expected when signature requests are sent // First we need to get a signature request to populate the KVDB for our verifying key - let with_parent_key = true; let (validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; signature_request.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number; signature_request.signature_verifying_key = verifying_key.to_vec(); @@ -637,22 +614,18 @@ async fn test_fails_to_sign_if_non_signing_group_participants_are_used() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); - let with_parent_key = true; let (_validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; let message_hash = Hasher::keccak(PREIMAGE_SHOULD_SUCCEED); let signature_request_account = subxtAccountId32(one.pair().public().0); @@ -752,14 +725,12 @@ async fn test_program_with_config() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); @@ -790,9 +761,8 @@ async fn test_program_with_config() { .unwrap(); // Now we'll send off a signature request using the new program - let with_parent_key = true; let (validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(message), with_parent_key).await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(message)).await; // We'll use the actual verifying key we registered for the signature request signature_request.signature_verifying_key = verifying_key.to_vec(); @@ -903,23 +873,8 @@ async fn test_jumpstart_network() { clean_tests(); } -pub async fn put_register_request_on_chain( - api: &OnlineClient, - rpc: &LegacyRpcMethods, - sig_req_keyring: &Sr25519Keyring, - program_modification_account: subxtAccountId32, - program_instances: BoundedVec, -) { - let sig_req_account = - PairSigner::::new(sig_req_keyring.pair()); - - let registering_tx = - entropy::tx().registry().register(program_modification_account, program_instances); - submit_transaction(api, rpc, &sig_req_account, ®istering_tx, None).await.unwrap(); -} - /// Registers an account on-chain using the new registration flow. -pub async fn put_new_register_request_on_chain( +pub async fn put_register_request_on_chain( api: &OnlineClient, rpc: &LegacyRpcMethods, signature_request_account: &Sr25519Keyring, @@ -931,7 +886,7 @@ pub async fn put_new_register_request_on_chain( PairSigner::::new(signature_request_account.pair()); let registering_tx = - entropy::tx().registry().register_on_chain(program_modification_account, program_instances); + entropy::tx().registry().register(program_modification_account, program_instances); let events = submit_transaction(api, rpc, &signature_request_account, ®istering_tx, None).await?; @@ -1069,22 +1024,19 @@ async fn test_fail_infinite_program() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); // Now we'll send off a signature request using the new program - let with_parent_key = true; let (_validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key).await; + get_sign_tx_data(&api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; // We'll use the actual verifying key we registered for the signature request signature_request.signature_verifying_key = verifying_key.to_vec(); @@ -1159,14 +1111,12 @@ async fn test_device_key_proxy() { .await .unwrap(); - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &entropy_api, &rpc, one.clone().into(), // This is our program modification account subxtAccountId32(two.public().0), // This is our signature request account BoundedVec(vec![ProgramInstance { program_pointer: program_hash, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); @@ -1226,10 +1176,8 @@ async fn test_device_key_proxy() { ))]); // Now we'll send off a signature request using the new program with auxilary data - let with_parent_key = true; let (validators_info, mut signature_request, validator_ips_and_keys) = - get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key) - .await; + get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED)).await; // We'll use the actual verifying key we registered for the signature request signature_request.signature_verifying_key = verifying_key.to_vec(); @@ -1479,7 +1427,7 @@ async fn test_new_registration_flow() { .await .unwrap(); - let registration_request = put_new_register_request_on_chain( + let registration_request = put_register_request_on_chain( &entropy_api, &rpc, &alice, // This is our signature request account @@ -1635,9 +1583,8 @@ pub async fn get_sign_tx_data( api: &OnlineClient, rpc: &LegacyRpcMethods, message: String, - with_parent_key: bool, ) -> (Vec, UserSignatureRequest, Vec<(String, [u8; 32])>) { - let validators_info = get_signers_from_chain(api, rpc, with_parent_key).await.unwrap(); + let validators_info = get_signers_from_chain(api, rpc).await.unwrap(); let signature_request = UserSignatureRequest { message, diff --git a/crates/threshold-signature-server/tests/register_and_sign.rs b/crates/threshold-signature-server/tests/register_and_sign.rs index 454ff701e..839bd41d9 100644 --- a/crates/threshold-signature-server/tests/register_and_sign.rs +++ b/crates/threshold-signature-server/tests/register_and_sign.rs @@ -68,14 +68,12 @@ async fn integration_test_register_and_sign() { .unwrap(); // Register, using that program - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &api, &rpc, account_owner.clone(), AccountId32(account_owner.public().0), BoundedVec(vec![ProgramInstance { program_pointer, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); diff --git a/crates/threshold-signature-server/tests/sign_eth_tx.rs b/crates/threshold-signature-server/tests/sign_eth_tx.rs index cd586edbf..00279dad4 100644 --- a/crates/threshold-signature-server/tests/sign_eth_tx.rs +++ b/crates/threshold-signature-server/tests/sign_eth_tx.rs @@ -78,14 +78,12 @@ async fn integration_test_sign_eth_tx() { .unwrap(); // Register, using that program - let register_on_chain = true; let (verifying_key, _registered_info) = test_client::register( &api, &rpc, account_owner.clone(), AccountId32(account_owner.public().0), BoundedVec(vec![ProgramInstance { program_pointer, program_config: vec![] }]), - register_on_chain, ) .await .unwrap(); diff --git a/node/cli/src/chain_spec/dev.rs b/node/cli/src/chain_spec/dev.rs index 44e29e38b..6a8fa3cd6 100644 --- a/node/cli/src/chain_spec/dev.rs +++ b/node/cli/src/chain_spec/dev.rs @@ -19,14 +19,14 @@ use crate::endowed_accounts::endowed_accounts_dev; use entropy_runtime::{ constants::currency::*, wasm_binary_unwrap, AttestationConfig, AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, ElectionsConfig, GrandpaConfig, ImOnlineConfig, IndicesConfig, - MaxNominations, ParametersConfig, ProgramsConfig, RegistryConfig, SessionConfig, StakerStatus, - StakingConfig, StakingExtensionConfig, SudoConfig, TechnicalCommitteeConfig, + MaxNominations, ParametersConfig, ProgramsConfig, SessionConfig, StakerStatus, StakingConfig, + StakingExtensionConfig, SudoConfig, TechnicalCommitteeConfig, }; use entropy_runtime::{AccountId, Balance}; use entropy_shared::{ - X25519PublicKey as TssX25519PublicKey, DAVE_VERIFYING_KEY, DEVICE_KEY_AUX_DATA_TYPE, - DEVICE_KEY_CONFIG_TYPE, DEVICE_KEY_HASH, DEVICE_KEY_PROXY, EVE_VERIFYING_KEY, - FERDIE_VERIFYING_KEY, INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, SIGNER_THRESHOLD, TOTAL_SIGNERS, + X25519PublicKey as TssX25519PublicKey, DEVICE_KEY_AUX_DATA_TYPE, DEVICE_KEY_CONFIG_TYPE, + DEVICE_KEY_HASH, DEVICE_KEY_PROXY, INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, SIGNER_THRESHOLD, + TOTAL_SIGNERS, }; use grandpa_primitives::AuthorityId as GrandpaId; use itertools::Itertools; @@ -35,7 +35,7 @@ use sc_service::ChainType; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; use sp_consensus_babe::AuthorityId as BabeId; use sp_core::{sr25519, ByteArray}; -use sp_runtime::{BoundedVec, Perbill}; +use sp_runtime::Perbill; pub fn devnet_three_node_initial_tss_servers( ) -> Vec<(sp_runtime::AccountId32, TssX25519PublicKey, String)> { @@ -303,22 +303,6 @@ pub fn development_genesis_config( "imOnline": ImOnlineConfig { keys: vec![] }, "authorityDiscovery": AuthorityDiscoveryConfig { keys: vec![], ..Default::default() }, "grandpa": GrandpaConfig { authorities: vec![], ..Default::default() }, - "registry": RegistryConfig { - registered_accounts: vec![ - ( - get_account_id_from_seed::("Dave"), - BoundedVec::try_from(DAVE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ( - get_account_id_from_seed::("Eve"), - BoundedVec::try_from(EVE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ( - get_account_id_from_seed::("Ferdie"), - BoundedVec::try_from(FERDIE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ], - }, "parameters": ParametersConfig { request_limit: 20, max_instructions_per_programs: INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, diff --git a/node/cli/src/chain_spec/integration_tests.rs b/node/cli/src/chain_spec/integration_tests.rs index a7d9b9fbe..c5dd75816 100644 --- a/node/cli/src/chain_spec/integration_tests.rs +++ b/node/cli/src/chain_spec/integration_tests.rs @@ -19,14 +19,14 @@ use crate::endowed_accounts::endowed_accounts_dev; use entropy_runtime::{ constants::currency::*, wasm_binary_unwrap, AttestationConfig, AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, ElectionsConfig, GrandpaConfig, ImOnlineConfig, IndicesConfig, - MaxNominations, ParametersConfig, ProgramsConfig, RegistryConfig, SessionConfig, StakerStatus, - StakingConfig, StakingExtensionConfig, SudoConfig, TechnicalCommitteeConfig, + MaxNominations, ParametersConfig, ProgramsConfig, SessionConfig, StakerStatus, StakingConfig, + StakingExtensionConfig, SudoConfig, TechnicalCommitteeConfig, }; use entropy_runtime::{AccountId, Balance}; use entropy_shared::{ DAVE_VERIFYING_KEY, DEVICE_KEY_AUX_DATA_TYPE, DEVICE_KEY_CONFIG_TYPE, DEVICE_KEY_HASH, - DEVICE_KEY_PROXY, EVE_VERIFYING_KEY, FERDIE_VERIFYING_KEY, - INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, SIGNER_THRESHOLD, TOTAL_SIGNERS, + DEVICE_KEY_PROXY, EVE_VERIFYING_KEY, INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, SIGNER_THRESHOLD, + TOTAL_SIGNERS, }; use grandpa_primitives::AuthorityId as GrandpaId; use itertools::Itertools; @@ -35,7 +35,7 @@ use sc_service::ChainType; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; use sp_consensus_babe::AuthorityId as BabeId; use sp_core::{sr25519, ByteArray}; -use sp_runtime::{BoundedVec, Perbill}; +use sp_runtime::Perbill; /// The configuration used for the Threshold Signature Scheme server integration tests. /// @@ -242,22 +242,6 @@ pub fn integration_tests_genesis_config( "imOnline": ImOnlineConfig { keys: vec![] }, "authorityDiscovery": AuthorityDiscoveryConfig { keys: vec![], ..Default::default() }, "grandpa": GrandpaConfig { authorities: vec![], ..Default::default() }, - "registry": RegistryConfig { - registered_accounts: vec![ - ( - get_account_id_from_seed::("Dave"), - BoundedVec::try_from(DAVE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ( - get_account_id_from_seed::("Eve"), - BoundedVec::try_from(EVE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ( - get_account_id_from_seed::("Ferdie"), - BoundedVec::try_from(FERDIE_VERIFYING_KEY.to_vec()).unwrap(), - ), - ], - }, "parameters": ParametersConfig { request_limit: 20, max_instructions_per_programs: INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 72e7fac5b..873e6b6d0 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -357,11 +357,6 @@ pub fn new_full_base( b"propagation", &format!("{}/generate_network_key", endpoint).into_bytes(), ); - offchain_db.local_storage_set( - sp_core::offchain::StorageKind::PERSISTENT, - b"registration", - &format!("{}/user/new", endpoint).into_bytes(), - ); offchain_db.local_storage_set( sp_core::offchain::StorageKind::PERSISTENT, b"refresh", diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index 31424c5bf..06b299b25 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -69,13 +69,11 @@ pub mod pallet { fn offchain_worker(block_number: BlockNumberFor) { let _ = Self::post_dkg(block_number); let _ = Self::post_reshare(block_number); - let _ = Self::post_user_registration(block_number); let _ = Self::post_proactive_refresh(block_number); let _ = Self::post_attestation_request(block_number); } - fn on_initialize(block_number: BlockNumberFor) -> Weight { - pallet_registry::Dkg::::remove(block_number.saturating_sub(2u32.into())); + fn on_initialize(_block_number: BlockNumberFor) -> Weight { pallet_staking_extension::ProactiveRefresh::::take(); ::WeightInfo::on_initialize() } @@ -164,64 +162,6 @@ pub mod pallet { Ok(()) } - /// Submits a distributed key generation request to register a set of users to the threshold - /// servers. - pub fn post_user_registration(block_number: BlockNumberFor) -> Result<(), http::Error> { - let messages = - pallet_registry::Pallet::::dkg(block_number.saturating_sub(1u32.into())); - - let deadline = sp_io::offchain::timestamp().add(Duration::from_millis(2_000)); - let kind = sp_core::offchain::StorageKind::PERSISTENT; - let from_local = sp_io::offchain::local_storage_get(kind, b"registration") - .unwrap_or_else(|| b"http://localhost:3001/user/new".to_vec()); - let url = str::from_utf8(&from_local).unwrap_or("http://localhost:3001/user/new"); - - log::warn!("propagation::post::messages: {:?}", &messages); - let converted_block_number: u32 = - BlockNumberFor::::try_into(block_number).unwrap_or_default(); - let servers_info = - pallet_registry::Pallet::::get_validators_info().unwrap_or_default(); - let validators_info = servers_info - .iter() - .map(|server_info| ValidatorInfo { - x25519_public_key: server_info.x25519_public_key, - ip_address: server_info.endpoint.clone(), - tss_account: server_info.tss_account.encode(), - }) - .collect::>(); - // the data is serialized / encoded to Vec by parity-scale-codec::encode() - let req_body = OcwMessageDkg { - // subtract 1 from blocknumber since the request is from the last block - block_number: converted_block_number.saturating_sub(1), - sig_request_accounts: messages, - validators_info, - }; - - log::warn!("propagation::post::req_body: {:?}", &[req_body.encode()]); - // We construct the request - // important: the header->Content-Type must be added and match that of the receiving - // party!! - let pending = http::Request::post(url, vec![req_body.encode()]) - .deadline(deadline) - .send() - .map_err(|_| http::Error::IoError)?; - - // We await response, same as in fn get() - let response = - pending.try_wait(deadline).map_err(|_| http::Error::DeadlineReached)??; - - // check response code - if response.code != 200 { - log::warn!("Unexpected status code: {}", response.code); - return Err(http::Error::Unknown); - } - let _res_body = response.body().collect::>(); - - Self::deposit_event(Event::DkgMessagePassed(req_body)); - - Ok(()) - } - /// Submits a request to do a key refresh on the signers parent key. pub fn post_reshare(block_number: BlockNumberFor) -> Result<(), http::Error> { let reshare_data = pallet_staking_extension::Pallet::::reshare_data(); diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs index b3e411e11..e139c3cd4 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -17,9 +17,7 @@ use std::sync::Arc; use codec::Encode; use entropy_shared::ValidatorInfo; -use frame_support::{assert_ok, traits::OnInitialize, BoundedVec}; -use pallet_programs::ProgramInfo; -use pallet_registry::ProgramInstance; +use frame_support::traits::OnInitialize; use pallet_staking_extension::{RefreshInfo, ReshareInfo}; use sp_core::offchain::{testing, OffchainDbExt, OffchainWorkerExt, TransactionPoolExt}; use sp_io::TestExternalities; @@ -45,21 +43,6 @@ fn knows_how_to_mock_several_http_calls() { ..Default::default() }); - state.expect_request(testing::PendingRequest { - method: "POST".into(), - uri: "http://localhost:3001/generate_network_key".into(), - sent: true, - response: Some([].to_vec()), - body: [ - 3, 0, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 10, 32, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, - 11, 32, 4, 0, 0, 0, 0, 0, 0, 0, - ] - .to_vec(), - ..Default::default() - }); - state.expect_request(testing::PendingRequest { method: "POST".into(), uri: "http://localhost:3001/signer/proactive_refresh".into(), @@ -86,35 +69,6 @@ fn knows_how_to_mock_several_http_calls() { t.execute_with(|| { Propagation::post_dkg(1).unwrap(); - System::set_block_number(3); - pallet_programs::Programs::::insert( - ::Hash::default(), - ProgramInfo { - bytecode: vec![], - configuration_schema: vec![], - auxiliary_data_schema: vec![], - oracle_data_pointer: vec![], - deployer: 1, - ref_counter: 0, - }, - ); - - let programs_info = BoundedVec::try_from(vec![ProgramInstance { - program_pointer: ::Hash::default(), - program_config: vec![], - }]) - .unwrap(); - assert_ok!(Registry::register(RuntimeOrigin::signed(1), 2, programs_info.clone(),)); - assert_ok!(Registry::register(RuntimeOrigin::signed(2), 3, programs_info,)); - - // full send - Propagation::post_dkg(4).unwrap(); - - // test pruning - assert_eq!(Registry::dkg(3).len(), 2); - Propagation::on_initialize(5); - assert_eq!(Registry::dkg(3).len(), 0); - Propagation::post_proactive_refresh(6).unwrap(); let ocw_message = RefreshInfo { validators_info: vec![ValidatorInfo { diff --git a/pallets/registry/src/benchmarking.rs b/pallets/registry/src/benchmarking.rs index c520e8172..492009951 100644 --- a/pallets/registry/src/benchmarking.rs +++ b/pallets/registry/src/benchmarking.rs @@ -142,30 +142,6 @@ benchmarks! { register { let p in 1 .. T::MaxProgramHashes::get(); - let program = vec![0u8]; - let configuration_schema = vec![1u8]; - let auxiliary_data_schema = vec![2u8]; - let oracle_data_pointer = vec![3u8]; - let program_hash = T::Hashing::hash(&program); - let programs_info = BoundedVec::try_from(vec![ProgramInstance { - program_pointer: program_hash, - program_config: vec![], - }; p as usize]) - .unwrap(); - - let program_modification_account: T::AccountId = whitelisted_caller(); - Programs::::insert(program_hash, ProgramInfo {bytecode: program, configuration_schema, auxiliary_data_schema, oracle_data_pointer, deployer: program_modification_account.clone(), ref_counter: 0}); - let sig_req_account: T::AccountId = whitelisted_caller(); - let balance = ::Currency::minimum_balance() * 100u32.into(); - let _ = ::Currency::make_free_balance_be(&sig_req_account, balance); - }: _(RawOrigin::Signed(sig_req_account.clone()), program_modification_account, programs_info) - verify { - assert_last_event::(Event::SignalRegister(sig_req_account.clone()).into()); - assert!(Registering::::contains_key(sig_req_account)); - } - - register_on_chain { - let p in 1 .. T::MaxProgramHashes::get(); let program_modification_account: T::AccountId = whitelisted_caller(); let signature_request_account: T::AccountId = whitelisted_caller(); @@ -225,7 +201,7 @@ benchmarks! { // We substract one from the count since this gets incremented after a succesful registration, // and we're interested in the account we just registered. - let count = >::count() - 1; + let count = >::count() - 1; let derivation_path = bip32::DerivationPath::from_str(&scale_info::prelude::format!("m/0/{}", count)).unwrap(); @@ -247,7 +223,7 @@ benchmarks! { ).into(), ); - assert!(RegisteredOnChain::::contains_key(expected_verifying_key)); + assert!(Registered::::contains_key(expected_verifying_key)); } change_program_instance { diff --git a/pallets/registry/src/lib.rs b/pallets/registry/src/lib.rs index 99371e7d1..c22f42231 100644 --- a/pallets/registry/src/lib.rs +++ b/pallets/registry/src/lib.rs @@ -52,7 +52,7 @@ pub mod weights; #[frame_support::pallet] pub mod pallet { - use entropy_shared::{MAX_SIGNERS, NETWORK_PARENT_KEY, VERIFICATION_KEY_LENGTH}; + use entropy_shared::{MAX_SIGNERS, NETWORK_PARENT_KEY}; use frame_support::{ dispatch::DispatchResultWithPostInfo, pallet_prelude::*, traits::ConstU32, }; @@ -120,41 +120,10 @@ pub mod pallet { pub version_number: u8, } - #[pallet::genesis_config] - #[derive(frame_support::DefaultNoBound)] - pub struct GenesisConfig { - #[allow(clippy::type_complexity)] - pub registered_accounts: Vec<(T::AccountId, VerifyingKey)>, - } - - #[pallet::genesis_build] - impl BuildGenesisConfig for GenesisConfig { - fn build(&self) { - for (account_id, verifying_key) in &self.registered_accounts { - assert!(verifying_key.len() as u32 == VERIFICATION_KEY_LENGTH); - - Registered::::insert( - verifying_key.clone(), - RegisteredInfo { - programs_data: BoundedVec::default(), - program_modification_account: account_id.clone(), - derivation_path: None, - version_number: T::KeyVersionNumber::get(), - }, - ); - } - } - } - #[pallet::pallet] #[pallet::without_storage_info] pub struct Pallet(_); - #[pallet::storage] - #[pallet::getter(fn registering)] - pub type Registering = - StorageMap<_, Blake2_128Concat, T::AccountId, RegisteringDetails, OptionQuery>; - /// Used for triggering a network wide distributed key generation request via an offchain /// worker. #[pallet::storage] @@ -162,26 +131,14 @@ pub mod pallet { pub type JumpstartDkg = StorageMap<_, Blake2_128Concat, BlockNumberFor, Vec>, ValueQuery>; - /// Used to store requests and trigger distributed key generation for users via an offchain - /// worker. - #[pallet::storage] - #[pallet::getter(fn dkg)] - pub type Dkg = - StorageMap<_, Blake2_128Concat, BlockNumberFor, Vec>, ValueQuery>; - - #[pallet::storage] - #[pallet::getter(fn registered)] - pub type Registered = - StorageMap<_, Blake2_128Concat, VerifyingKey, RegisteredInfo, OptionQuery>; - /// An item tracking all the users registered on the Entropy network. /// /// Notice that the registration state does not depend on any Substrate account being /// registered, but rather a _verifying key_, which represents the user beyond the scope of the /// Entropy network itself (e.g it can be an account on Bitcoin or Ethereum). #[pallet::storage] - #[pallet::getter(fn registered_on_chain)] - pub type RegisteredOnChain = + #[pallet::getter(fn registered)] + pub type Registered = CountedStorageMap<_, Blake2_128Concat, VerifyingKey, RegisteredInfo, OptionQuery>; /// Mapping of program_modification accounts to verifying keys they can control @@ -206,41 +163,24 @@ pub mod pallet { FinishedNetworkJumpStart(), /// The network has had a jump start confirmation. [who, confirmation_count] JumpStartConfirmation(T::ValidatorId, u8), - /// An account has signaled to be registered. [signature request account] - SignalRegister(T::AccountId), - /// An account has been registered. [who, verifying_key] - RecievedConfirmation(T::AccountId, VerifyingKey), /// An account has been registered. \[who, verifying_key] AccountRegistered(T::AccountId, VerifyingKey), - /// An account registration has failed - FailedRegistration(T::AccountId), - /// An account cancelled their registration - RegistrationCancelled(T::AccountId), /// An account hash changed their program info [who, new_program_instance] ProgramInfoChanged(T::AccountId, BoundedVec, T::MaxProgramHashes>), /// An account has changed their program modification account [old, new, verifying_key] ProgramModificationAccountChanged(T::AccountId, T::AccountId, VerifyingKey), - /// An account has been registered. [who, block_number, failures] - ConfirmedDone(T::AccountId, BlockNumberFor, Vec), } // Errors inform users that something went wrong. #[pallet::error] pub enum Error { - AlreadySubmitted, NoThresholdKey, - NotRegistering, NotRegistered, AlreadyConfirmed, IpAddressError, - NoSyncedValidators, - MaxProgramLengthExceeded, - NoVerifyingKey, NotAuthorized, - ProgramDoesNotExist, NoProgramSet, TooManyModifiableKeys, - MismatchedVerifyingKeyLength, MismatchedVerifyingKey, NotValidator, JumpStartProgressNotReady, @@ -383,9 +323,8 @@ pub mod pallet { /// /// The caller provides an initial program pointer. /// - /// Note that a user needs to be confirmed by validators through the - /// [`Self::confirm_register`] extrinsic before they can be considered as registered on the - /// network. + /// Note: Substrate origins are allowed to register as many accounts as they wish. Each + /// registration request will produce a different verifying key. #[pallet::call_index(2)] #[pallet::weight({ ::WeightInfo::register(::MaxProgramHashes::get()) @@ -395,18 +334,19 @@ pub mod pallet { program_modification_account: T::AccountId, programs_data: BoundedVec, T::MaxProgramHashes>, ) -> DispatchResultWithPostInfo { - let sig_req_account = ensure_signed(origin)?; + use core::str::FromStr; + use synedrion::{ecdsa::VerifyingKey as SynedrionVerifyingKey, DeriveChildKey}; + + let signature_request_account = ensure_signed(origin)?; ensure!( - sig_req_account.encode() != NETWORK_PARENT_KEY.encode(), + signature_request_account.encode() != NETWORK_PARENT_KEY.encode(), Error::::NoRegisteringFromParentKey ); - ensure!( - !Registering::::contains_key(&sig_req_account), - Error::::AlreadySubmitted - ); - ensure!(!programs_data.is_empty(), Error::::NoProgramSet); - let block_number = >::block_number(); + + let num_programs = programs_data.len(); + ensure!(num_programs != 0, Error::::NoProgramSet); + // Change program ref counter for program_instance in &programs_data { pallet_programs::Programs::::try_mutate( @@ -422,25 +362,61 @@ pub mod pallet { )?; } - Dkg::::try_mutate(block_number, |messages| -> Result<_, DispatchError> { - messages.push(sig_req_account.encode()); - Ok(()) - })?; + let network_verifying_key = + if let Some(key) = >::get().verifying_key { + SynedrionVerifyingKey::try_from(key.as_slice()) + .expect("The network verifying key must be valid.") + } else { + return Err(Error::::JumpStartNotCompleted.into()); + }; - // Put account into a registering state - Registering::::insert( - &sig_req_account, - RegisteringDetails:: { - program_modification_account, - confirmations: vec![], - programs_data: programs_data.clone(), - verifying_key: None, + // TODO (#984): For a `CountedStorageMap` there is the possibility that the counter + // can decrease as storage entries are removed from the map. In our case we don't ever + // remove entries from the `Registered` map so the counter should never + // decrease. If it does we will end up with the same verifying key for different + // accounts, which would be bad. + // + // For a V1 of this flow it's fine, but we'll need to think about a better solution + // down the line. + let count = Registered::::count(); + let inner_path = scale_info::prelude::format!("m/0/{}", count); + let path = bip32::DerivationPath::from_str(&inner_path) + .map_err(|_| Error::::InvalidBip32DerivationPath)?; + let child_verifying_key = network_verifying_key + .derive_verifying_key_bip32(&path) + .map_err(|_| Error::::Bip32AccountDerivationFailed)?; + + let child_verifying_key = BoundedVec::try_from( + child_verifying_key.to_encoded_point(true).as_bytes().to_vec(), + ) + .expect("Synedrion must have returned a valid verifying key."); + + Registered::::insert( + child_verifying_key.clone(), + RegisteredInfo { + programs_data, + program_modification_account: program_modification_account.clone(), + derivation_path: Some(inner_path.encode()), version_number: T::KeyVersionNumber::get(), }, ); - Self::deposit_event(Event::SignalRegister(sig_req_account)); - Ok(Some(::WeightInfo::register(programs_data.len() as u32)).into()) + ModifiableKeys::::try_mutate( + program_modification_account, + |verifying_keys| -> Result<(), DispatchError> { + verifying_keys + .try_push(child_verifying_key.clone()) + .map_err(|_| Error::::TooManyModifiableKeys)?; + Ok(()) + }, + )?; + + Self::deposit_event(Event::AccountRegistered( + signature_request_account, + child_verifying_key, + )); + + Ok(Some(::WeightInfo::register(num_programs as u32)).into()) } /// Allows a user's program modification account to change their program pointer @@ -475,7 +451,7 @@ pub mod pallet { let mut old_programs_length = 0; let programs_data = - RegisteredOnChain::::try_mutate(&verifying_key, |maybe_registered_details| { + Registered::::try_mutate(&verifying_key, |maybe_registered_details| { if let Some(registered_details) = maybe_registered_details { ensure!( who == registered_details.program_modification_account, @@ -522,7 +498,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - RegisteredOnChain::::try_mutate(&verifying_key, |maybe_registered_details| { + Registered::::try_mutate(&verifying_key, |maybe_registered_details| { if let Some(registered_details) = maybe_registered_details { ensure!( who == registered_details.program_modification_account, @@ -568,106 +544,6 @@ pub mod pallet { )) .into()) } - - /// Allows a user to signal that they want to register an account with the Entropy network. - /// - /// The caller provides an initial program pointer. - /// - /// Note: Substrate origins are allowed to register as many accounts as they wish. Each - /// registration request will produce a different verifying key. - #[pallet::call_index(5)] - #[pallet::weight({ - ::WeightInfo::register_on_chain(::MaxProgramHashes::get()) - })] - pub fn register_on_chain( - origin: OriginFor, - program_modification_account: T::AccountId, - programs_data: BoundedVec, T::MaxProgramHashes>, - ) -> DispatchResultWithPostInfo { - use core::str::FromStr; - use synedrion::{ecdsa::VerifyingKey as SynedrionVerifyingKey, DeriveChildKey}; - - let signature_request_account = ensure_signed(origin)?; - - ensure!( - signature_request_account.encode() != NETWORK_PARENT_KEY.encode(), - Error::::NoRegisteringFromParentKey - ); - - let num_programs = programs_data.len(); - ensure!(num_programs != 0, Error::::NoProgramSet); - - // Change program ref counter - for program_instance in &programs_data { - pallet_programs::Programs::::try_mutate( - program_instance.program_pointer, - |maybe_program_info| { - if let Some(program_info) = maybe_program_info { - program_info.ref_counter = program_info.ref_counter.saturating_add(1); - Ok(()) - } else { - Err(Error::::NoProgramSet) - } - }, - )?; - } - - let network_verifying_key = - if let Some(key) = >::get().verifying_key { - SynedrionVerifyingKey::try_from(key.as_slice()) - .expect("The network verifying key must be valid.") - } else { - return Err(Error::::JumpStartNotCompleted.into()); - }; - - // TODO (#984): For a `CountedStorageMap` there is the possibility that the counter - // can decrease as storage entries are removed from the map. In our case we don't ever - // remove entries from the `RegisteredOnChain` map so the counter should never - // decrease. If it does we will end up with the same verifying key for different - // accounts, which would be bad. - // - // For a V1 of this flow it's fine, but we'll need to think about a better solution - // down the line. - let count = RegisteredOnChain::::count(); - let inner_path = scale_info::prelude::format!("m/0/{}", count); - let path = bip32::DerivationPath::from_str(&inner_path) - .map_err(|_| Error::::InvalidBip32DerivationPath)?; - let child_verifying_key = network_verifying_key - .derive_verifying_key_bip32(&path) - .map_err(|_| Error::::Bip32AccountDerivationFailed)?; - - let child_verifying_key = BoundedVec::try_from( - child_verifying_key.to_encoded_point(true).as_bytes().to_vec(), - ) - .expect("Synedrion must have returned a valid verifying key."); - - RegisteredOnChain::::insert( - child_verifying_key.clone(), - RegisteredInfo { - programs_data, - program_modification_account: program_modification_account.clone(), - derivation_path: Some(inner_path.encode()), - version_number: T::KeyVersionNumber::get(), - }, - ); - - ModifiableKeys::::try_mutate( - program_modification_account, - |verifying_keys| -> Result<(), DispatchError> { - verifying_keys - .try_push(child_verifying_key.clone()) - .map_err(|_| Error::::TooManyModifiableKeys)?; - Ok(()) - }, - )?; - - Self::deposit_event(Event::AccountRegistered( - signature_request_account, - child_verifying_key, - )); - - Ok(Some(::WeightInfo::register(num_programs as u32)).into()) - } } impl Pallet { diff --git a/pallets/registry/src/tests.rs b/pallets/registry/src/tests.rs index 951475348..77a6517bb 100644 --- a/pallets/registry/src/tests.rs +++ b/pallets/registry/src/tests.rs @@ -24,7 +24,7 @@ use pallet_staking_extension::{JumpStartDetails, JumpStartProgress, JumpStartSta use sp_runtime::traits::Hash; use crate as pallet_registry; -use crate::{mock::*, Error, ModifiableKeys, ProgramInstance, RegisteredInfo, RegisteredOnChain}; +use crate::{mock::*, Error, ModifiableKeys, ProgramInstance, Registered, RegisteredInfo}; const NULL_ARR: [u8; 32] = [0; 32]; @@ -69,7 +69,7 @@ fn it_tests_get_validators_info() { } #[test] -fn it_registers_a_user_on_chain() { +fn it_registers_a_user() { new_test_ext().execute_with(|| { use synedrion::{ecdsa::VerifyingKey as SynedrionVerifyingKey, DeriveChildKey}; @@ -87,11 +87,7 @@ fn it_registers_a_user_on_chain() { }); // Test: Run through registration - assert_ok!(Registry::register_on_chain( - RuntimeOrigin::signed(alice), - bob, - programs_info.clone(), - )); + assert_ok!(Registry::register(RuntimeOrigin::signed(alice), bob, programs_info.clone(),)); // Validate: Our expected verifying key is registered correctly let network_verifying_key = @@ -104,7 +100,7 @@ fn it_registers_a_user_on_chain() { BoundedVec::try_from(expected_verifying_key.to_encoded_point(true).as_bytes().to_vec()) .unwrap(); - let registered_info = Registry::registered_on_chain(expected_verifying_key.clone()); + let registered_info = Registry::registered(expected_verifying_key.clone()); assert!(registered_info.is_some()); assert_eq!(registered_info.unwrap().program_modification_account, bob); }); @@ -129,11 +125,7 @@ fn it_increases_program_reference_count_on_register() { }); // Test: Run through registration - assert_ok!(Registry::register_on_chain( - RuntimeOrigin::signed(alice), - bob, - programs_info.clone(), - )); + assert_ok!(Registry::register(RuntimeOrigin::signed(alice), bob, programs_info.clone(),)); // Validate: We expect that the program reference count has gone up assert_eq!( @@ -164,17 +156,9 @@ fn it_registers_different_users_with_the_same_sig_req_account() { // Test: Run through registration twice using the same signature request account. We should // get different verifying keys. - assert_ok!(Registry::register_on_chain( - RuntimeOrigin::signed(alice), - bob, - programs_info.clone(), - )); + assert_ok!(Registry::register(RuntimeOrigin::signed(alice), bob, programs_info.clone(),)); - assert_ok!(Registry::register_on_chain( - RuntimeOrigin::signed(alice), - bob, - programs_info.clone(), - )); + assert_ok!(Registry::register(RuntimeOrigin::signed(alice), bob, programs_info.clone(),)); // Validate: We expect two different verifying keys to be registered let network_verifying_key = @@ -199,8 +183,8 @@ fn it_registers_different_users_with_the_same_sig_req_account() { // Knowing that the two keys are indeed different, we still expect both registration // requests to have succeeded. assert!(first_expected_verifying_key != second_expected_verifying_key); - assert!(Registry::registered_on_chain(first_expected_verifying_key).is_some()); - assert!(Registry::registered_on_chain(second_expected_verifying_key).is_some()); + assert!(Registry::registered(first_expected_verifying_key).is_some()); + assert!(Registry::registered(second_expected_verifying_key).is_some()); }); } @@ -214,7 +198,7 @@ fn it_fails_registration_if_no_program_is_set() { // Test: Run through registration, this should fail assert_noop!( - Registry::register_on_chain(RuntimeOrigin::signed(alice), bob, programs_info,), + Registry::register(RuntimeOrigin::signed(alice), bob, programs_info,), Error::::NoProgramSet ); }) @@ -236,7 +220,7 @@ fn it_fails_registration_if_an_empty_program_is_set() { // Test: Run through registration, this should fail assert_noop!( - Registry::register_on_chain(RuntimeOrigin::signed(alice), bob, programs_info,), + Registry::register(RuntimeOrigin::signed(alice), bob, programs_info,), Error::::NoProgramSet ); }) @@ -260,7 +244,7 @@ fn it_fails_registration_if_no_jump_start_has_happened() { // Test: Run through registration, this should fail assert_noop!( - Registry::register_on_chain(RuntimeOrigin::signed(alice), bob, programs_info,), + Registry::register(RuntimeOrigin::signed(alice), bob, programs_info,), Error::::JumpStartNotCompleted ); }) @@ -294,7 +278,7 @@ fn it_fails_registration_with_too_many_modifiable_keys() { // Test: Run through registration, this should fail assert_noop!( - Registry::register_on_chain(RuntimeOrigin::signed(alice), bob, programs_info,), + Registry::register(RuntimeOrigin::signed(alice), bob, programs_info,), Error::::TooManyModifiableKeys ); }) @@ -481,11 +465,8 @@ fn it_changes_a_program_instance() { version_number: 1, }; - RegisteredOnChain::::insert(expected_verifying_key.clone(), ®istered_info); - assert_eq!( - Registry::registered_on_chain(expected_verifying_key.clone()).unwrap(), - registered_info - ); + Registered::::insert(expected_verifying_key.clone(), ®istered_info); + assert_eq!(Registry::registered(expected_verifying_key.clone()).unwrap(), registered_info); assert_ok!(Registry::change_program_instance( RuntimeOrigin::signed(2), @@ -494,10 +475,7 @@ fn it_changes_a_program_instance() { )); registered_info.programs_data = new_programs_info; - assert_eq!( - Registry::registered_on_chain(expected_verifying_key.clone()).unwrap(), - registered_info - ); + assert_eq!(Registry::registered(expected_verifying_key.clone()).unwrap(), registered_info); assert_eq!( pallet_programs::Programs::::get(program_hash).unwrap().ref_counter, 0, @@ -552,11 +530,8 @@ fn it_changes_a_program_mod_account() { version_number: 1, }; - RegisteredOnChain::::insert(expected_verifying_key.clone(), ®istered_info); - assert_eq!( - Registry::registered_on_chain(expected_verifying_key.clone()).unwrap(), - registered_info - ); + Registered::::insert(expected_verifying_key.clone(), ®istered_info); + assert_eq!(Registry::registered(expected_verifying_key.clone()).unwrap(), registered_info); // Idk why this state could happen but still test to make sure it fails with a noop if ModifiableKeys not set assert_noop!( @@ -588,7 +563,7 @@ fn it_changes_a_program_mod_account() { registered_info.program_modification_account = 3; assert_eq!( - Registry::registered_on_chain(expected_verifying_key.clone()).unwrap(), + Registry::registered(expected_verifying_key.clone()).unwrap(), registered_info, "account 3 now in registered info" ); @@ -605,36 +580,3 @@ fn it_changes_a_program_mod_account() { ); }) } - -#[test] -fn it_fails_on_non_matching_verifying_keys() { - new_test_ext().execute_with(|| { - let empty_program = vec![]; - let program_hash = ::Hashing::hash(&empty_program); - let programs_info = BoundedVec::try_from(vec![ProgramInstance { - program_pointer: program_hash, - program_config: vec![], - }]) - .unwrap(); - - pallet_programs::Programs::::insert( - program_hash, - ProgramInfo { - bytecode: empty_program.clone(), - configuration_schema: empty_program.clone(), - auxiliary_data_schema: empty_program.clone(), - oracle_data_pointer: empty_program.clone(), - deployer: 1, - ref_counter: 0, - }, - ); - - assert_ok!(Registry::register(RuntimeOrigin::signed(1), 2, programs_info.clone(),)); - - // error if they try to submit another request, even with a different program key - assert_noop!( - Registry::register(RuntimeOrigin::signed(1), 2, programs_info), - Error::::AlreadySubmitted - ); - }); -} diff --git a/pallets/registry/src/weights.rs b/pallets/registry/src/weights.rs index 6584f4075..a9862b35e 100644 --- a/pallets/registry/src/weights.rs +++ b/pallets/registry/src/weights.rs @@ -51,8 +51,7 @@ use core::marker::PhantomData; /// Weight functions needed for pallet_registry. pub trait WeightInfo { - fn register(p: u32) -> Weight; - fn register_on_chain(_p: u32) -> Weight; + fn register(_p: u32) -> Weight; fn jump_start_network() -> Weight; fn confirm_jump_start_done(c: u32, ) -> Weight; fn confirm_jump_start_confirm(c: u32, ) -> Weight; @@ -111,39 +110,18 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: `Registry::Registered` (r:1 w:0) - /// Proof: `Registry::Registered` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Registry::Registering` (r:1 w:1) - /// Proof: `Registry::Registering` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Programs::Programs` (r:1 w:1) - /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Registry::Dkg` (r:1 w:1) - /// Proof: `Registry::Dkg` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// The range of component `p` is `[1, 5]`. - fn register(p: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `401` - // Estimated: `3866` - // Minimum execution time: 21_000_000 picoseconds. - Weight::from_parts(19_100_000, 0) - .saturating_add(Weight::from_parts(0, 3866)) - // Standard Error: 134_629 - .saturating_add(Weight::from_parts(2_000_000, 0).saturating_mul(p.into())) - .saturating_add(T::DbWeight::get().reads(4)) - .saturating_add(T::DbWeight::get().writes(3)) - } /// Storage: `Programs::Programs` (r:1 w:1) /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::JumpStartProgress` (r:1 w:0) /// Proof: `Registry::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `Registry::CounterForRegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::CounterForRegisteredOnChain` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) - /// Storage: `Registry::RegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::RegisteredOnChain` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Registry::CounterForRegistered` (r:1 w:1) + /// Proof: `Registry::CounterForRegistered` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Registry::Registered` (r:1 w:1) + /// Proof: `Registry::Registered` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::ModifiableKeys` (r:1 w:1) /// Proof: `Registry::ModifiableKeys` (`max_values`: None, `max_size`: None, mode: `Measured`) /// The range of component `p` is `[1, 5]`. - fn register_on_chain(_p: u32, ) -> Weight { + fn register(_p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `563` // Estimated: `4028` @@ -239,39 +217,18 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(3)) .saturating_add(RocksDbWeight::get().writes(1)) } - /// Storage: `Registry::Registered` (r:1 w:0) - /// Proof: `Registry::Registered` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Registry::Registering` (r:1 w:1) - /// Proof: `Registry::Registering` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Programs::Programs` (r:1 w:1) - /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Registry::Dkg` (r:1 w:1) - /// Proof: `Registry::Dkg` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// The range of component `p` is `[1, 5]`. - fn register(p: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `401` - // Estimated: `3866` - // Minimum execution time: 21_000_000 picoseconds. - Weight::from_parts(19_100_000, 0) - .saturating_add(Weight::from_parts(0, 3866)) - // Standard Error: 134_629 - .saturating_add(Weight::from_parts(2_000_000, 0).saturating_mul(p.into())) - .saturating_add(RocksDbWeight::get().reads(4)) - .saturating_add(RocksDbWeight::get().writes(3)) - } /// Storage: `Programs::Programs` (r:1 w:1) /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::JumpStartProgress` (r:1 w:0) /// Proof: `Registry::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `Registry::CounterForRegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::CounterForRegisteredOnChain` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) - /// Storage: `Registry::RegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::RegisteredOnChain` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Registry::CounterForRegistered` (r:1 w:1) + /// Proof: `Registry::CounterForRegistered` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Registry::Registered` (r:1 w:1) + /// Proof: `Registry::Registered` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::ModifiableKeys` (r:1 w:1) /// Proof: `Registry::ModifiableKeys` (`max_values`: None, `max_size`: None, mode: `Measured`) /// The range of component `p` is `[1, 5]`. - fn register_on_chain(_p: u32, ) -> Weight { + fn register(_p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `563` // Estimated: `4028` diff --git a/runtime/src/weights/pallet_registry.rs b/runtime/src/weights/pallet_registry.rs index 720a5c5a9..bae0793a0 100644 --- a/runtime/src/weights/pallet_registry.rs +++ b/runtime/src/weights/pallet_registry.rs @@ -97,37 +97,18 @@ impl pallet_registry::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: `Registry::Registering` (r:1 w:1) - /// Proof: `Registry::Registering` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Programs::Programs` (r:1 w:1) - /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `Registry::Dkg` (r:1 w:1) - /// Proof: `Registry::Dkg` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// The range of component `p` is `[1, 5]`. - fn register(p: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `393` - // Estimated: `3858` - // Minimum execution time: 20_000_000 picoseconds. - Weight::from_parts(18_500_000, 0) - .saturating_add(Weight::from_parts(0, 3858)) - // Standard Error: 362_284 - .saturating_add(Weight::from_parts(2_500_000, 0).saturating_mul(p.into())) - .saturating_add(T::DbWeight::get().reads(3)) - .saturating_add(T::DbWeight::get().writes(3)) - } /// Storage: `Programs::Programs` (r:1 w:1) /// Proof: `Programs::Programs` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::JumpStartProgress` (r:1 w:0) /// Proof: `Registry::JumpStartProgress` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `Registry::CounterForRegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::CounterForRegisteredOnChain` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) - /// Storage: `Registry::RegisteredOnChain` (r:1 w:1) - /// Proof: `Registry::RegisteredOnChain` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Registry::CounterForRegistered` (r:1 w:1) + /// Proof: `Registry::CounterForRegistered` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Registry::Registered` (r:1 w:1) + /// Proof: `Registry::Registered` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Registry::ModifiableKeys` (r:1 w:1) /// Proof: `Registry::ModifiableKeys` (`max_values`: None, `max_size`: None, mode: `Measured`) /// The range of component `p` is `[1, 5]`. - fn register_on_chain(_p: u32, ) -> Weight { + fn register(_p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `563` // Estimated: `4028` From b5cd90a9b0defd5ad2a608f0dd779d9263108b1c Mon Sep 17 00:00:00 2001 From: JesseAbram <33698952+JesseAbram@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:31:03 -0400 Subject: [PATCH 2/4] Rotate parent network key takes 2 steps (#1018) * Rotate parent netowrk key takes 2 steps * add rotate keyshare trigger to chain * make sure is signer * add validate rotate keyshare * test for stale check * docs * test fix * Apply suggestions from code review Co-authored-by: peg * change when old network key is deleted * Apply suggestions from code review Co-authored-by: Hernando Castano --------- Co-authored-by: peg Co-authored-by: Hernando Castano --- crates/client/entropy_metadata.scale | Bin 207747 -> 207926 bytes crates/shared/src/constants.rs | 3 + crates/threshold-signature-server/src/lib.rs | 3 +- .../src/validator/api.rs | 97 +++++++++++++++--- .../src/validator/tests.rs | 84 ++++++++++++++- node/cli/src/service.rs | 5 + pallets/propagation/src/lib.rs | 48 +++++++++ pallets/propagation/src/tests.rs | 11 ++ pallets/staking/src/lib.rs | 8 ++ 9 files changed, 244 insertions(+), 15 deletions(-) diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index dfc2ecfa21135ae47c209279c98a5d52de690450..2c9faa9d0e84c708ce08592872a63be6aa7fd84a 100644 GIT binary patch delta 174 zcmZp^&a>?X&xRE<8AUd)ocWwlD#kmtGAO?!u_U$FH?_DpF+DXPvA8%jg@s{qW4 Router { .route("/user/sign_tx", post(sign_tx)) .route("/signer/proactive_refresh", post(proactive_refresh)) .route("/validator/reshare", post(new_reshare)) + .route("/validator/rotate_network_key", post(rotate_network_key)) .route("/attest", post(attest)) .route("/healthz", get(healthz)) .route("/version", get(get_version)) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index 810996711..fa7a8f880 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -35,7 +35,7 @@ pub use entropy_protocol::{ execute_protocol::{execute_protocol_generic, execute_reshare, Channels, PairWrapper}, KeyParams, KeyShareWithAuxInfo, Listener, PartyId, SessionId, ValidatorInfo, }; -use entropy_shared::{OcwMessageReshare, NETWORK_PARENT_KEY}; +use entropy_shared::{OcwMessageReshare, NETWORK_PARENT_KEY, NEXT_NETWORK_PARENT_KEY}; use parity_scale_codec::{Decode, Encode}; use sp_core::Pair; use std::{collections::BTreeSet, str::FromStr}; @@ -93,12 +93,9 @@ pub async fn new_reshare( ) .map_err(|e| ValidatorErr::VerifyingKeyError(e.to_string()))?; - let is_proper_signer = is_signer_or_delete_parent_key( - signer.account_id(), - validators_info.clone(), - &app_state.kv_store, - ) - .await?; + let is_proper_signer = validators_info + .iter() + .any(|validator_info| validator_info.tss_account == *signer.account_id()); if !is_proper_signer { return Ok(StatusCode::MISDIRECTED_REQUEST); @@ -176,13 +173,13 @@ pub async fn new_reshare( let serialized_key_share = key_serialize(&(new_key_share, aux_info)) .map_err(|_| ProtocolErr::KvSerialize("Kv Serialize Error".to_string()))?; - let network_parent_key = hex::encode(NETWORK_PARENT_KEY); - // TODO: should this be a two step process? see # https://github.com/entropyxyz/entropy-core/issues/968 - if app_state.kv_store.kv().exists(&network_parent_key).await? { - app_state.kv_store.kv().delete(&network_parent_key).await? + let next_network_parent_key = hex::encode(NEXT_NETWORK_PARENT_KEY); + + if app_state.kv_store.kv().exists(&next_network_parent_key).await? { + app_state.kv_store.kv().delete(&next_network_parent_key).await? }; - let reservation = app_state.kv_store.kv().reserve_key(network_parent_key).await?; + let reservation = app_state.kv_store.kv().reserve_key(next_network_parent_key).await?; app_state.kv_store.kv().put(reservation, serialized_key_share.clone()).await?; // TODO: Error handling really complex needs to be thought about. @@ -190,6 +187,58 @@ pub async fn new_reshare( Ok(StatusCode::OK) } +/// HTTP POST endpoint called by the off-chain worker (propagation pallet) after a network key reshare. +/// +/// This rotates network key, deleting the previous network parent key. +#[tracing::instrument(skip_all)] +pub async fn rotate_network_key( + State(app_state): State, +) -> Result { + // validate from chain + let api = get_api(&app_state.configuration.endpoint).await?; + let rpc = get_rpc(&app_state.configuration.endpoint).await?; + validate_rotate_network_key(&api, &rpc).await?; + + let (signer, _) = get_signer_and_x25519_secret(&app_state.kv_store) + .await + .map_err(|e| ValidatorErr::UserError(e.to_string()))?; + + let signers_query = entropy::storage().staking_extension().signers(); + let signers = query_chain(&api, &rpc, signers_query, None) + .await? + .ok_or_else(|| ValidatorErr::ChainFetch("Error getting signers"))?; + + let validators_info = get_validators_info(&api, &rpc, signers) + .await + .map_err(|e| ValidatorErr::UserError(e.to_string()))?; + + let is_proper_signer = is_signer_or_delete_parent_key( + signer.account_id(), + validators_info.clone(), + &app_state.kv_store, + ) + .await?; + + if !is_proper_signer { + return Ok(StatusCode::MISDIRECTED_REQUEST); + } + + let network_parent_key_heading = hex::encode(NETWORK_PARENT_KEY); + let next_network_parent_key_heading = hex::encode(NEXT_NETWORK_PARENT_KEY); + + let new_parent_key = app_state.kv_store.kv().get(&next_network_parent_key_heading).await?; + + if app_state.kv_store.kv().exists(&network_parent_key_heading).await? { + app_state.kv_store.kv().delete(&network_parent_key_heading).await?; + }; + + app_state.kv_store.kv().delete(&next_network_parent_key_heading).await?; + + let reservation = app_state.kv_store.kv().reserve_key(network_parent_key_heading).await?; + app_state.kv_store.kv().put(reservation, new_parent_key).await?; + Ok(StatusCode::OK) +} + // Validates new reshare endpoint /// Checks the chain for validity of data and block number of data matches current block pub async fn validate_new_reshare( @@ -236,6 +285,30 @@ pub async fn validate_new_reshare( Ok(()) } +/// Checks the chain that the reshare was completed recently. +/// +/// We only care that this happens after a reshare so we just check that message isn't stale +pub async fn validate_rotate_network_key( + api: &OnlineClient, + rpc: &LegacyRpcMethods, +) -> Result<(), ValidatorErr> { + let latest_block_number = rpc + .chain_get_header(None) + .await? + .ok_or_else(|| ValidatorErr::OptionUnwrapError("Failed to get block number".to_string()))? + .number; + + let rotate_keyshares_info_query = entropy::storage().staking_extension().rotate_keyshares(); + let rotate_keyshare_block = query_chain(api, rpc, rotate_keyshares_info_query, None) + .await? + .ok_or_else(|| ValidatorErr::ChainFetch("Rotate Keyshare not in progress"))?; + + if latest_block_number > rotate_keyshare_block { + return Err(ValidatorErr::StaleData); + } + + Ok(()) +} /// Confirms that a validator has succefully reshared. pub async fn confirm_key_reshare( api: &OnlineClient, diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index 187c46206..07854ac49 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -101,15 +101,95 @@ async fn test_reshare() { let (key_share_after, aux_info_after): KeyShareWithAuxInfo = deserialize(&key_share_and_aux_data_after).unwrap(); + // Check key share has not yet changed + assert_eq!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); + // Check aux info has not yet changed + assert_eq!(serialize(&aux_info_before).unwrap(), serialize(&aux_info_after).unwrap()); + + let _ = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + let key_share_and_aux_data_after_rotate = + unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), validator_ports[i]).await; + let (key_share_after_rotate, aux_info_after_rotate): KeyShareWithAuxInfo = + deserialize(&key_share_and_aux_data_after_rotate).unwrap(); + // Check key share has changed - assert_ne!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); + assert_ne!( + serialize(&key_share_before).unwrap(), + serialize(&key_share_after_rotate).unwrap() + ); // Check aux info has changed - assert_ne!(serialize(&aux_info_before).unwrap(), serialize(&aux_info_after).unwrap()); + assert_ne!( + serialize(&aux_info_before).unwrap(), + serialize(&aux_info_after_rotate).unwrap() + ); + + // calling twice doesn't do anything + let response = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + assert_eq!(response.text().await.unwrap(), "Kv error: Recv Error: channel closed"); + let key_share_and_aux_data_after_rotate_twice = + unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), validator_ports[i]).await; + let (key_share_after_rotate_twice, aux_info_after_rotate_twice): KeyShareWithAuxInfo = + deserialize(&key_share_and_aux_data_after_rotate_twice).unwrap(); + + // Check key share has not changed + assert_eq!( + serialize(&key_share_after_rotate_twice).unwrap(), + serialize(&key_share_after_rotate).unwrap() + ); + // Check aux info has not changed + assert_eq!( + serialize(&aux_info_after_rotate_twice).unwrap(), + serialize(&aux_info_after_rotate).unwrap() + ); } + + run_to_block(&rpc, block_number + 7).await; + + let response_stale = + client.post("http://127.0.0.1:3001/validator/rotate_network_key").send().await.unwrap(); + + assert_eq!(response_stale.text().await.unwrap(), "Data is stale"); + // TODO #981 - test signing a message with the new keyshare set clean_tests(); } +#[tokio::test] +#[serial] +async fn test_reshare_none_called() { + initialize_test_logger().await; + clean_tests(); + + let _cxt = test_node_process_testing_state(true).await; + + let add_parent_key_to_kvdb = true; + let (_validator_ips, _validator_ids) = spawn_testing_validators(add_parent_key_to_kvdb).await; + + let validator_ports = vec![3001, 3002, 3003]; + + let client = reqwest::Client::new(); + + for i in 0..validator_ports.len() { + let response = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + assert_eq!(response.text().await.unwrap(), "Chain Fetch: Rotate Keyshare not in progress"); + } +} + #[tokio::test] #[serial] async fn test_reshare_validation_fail() { diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 873e6b6d0..ac481472a 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -367,6 +367,11 @@ pub fn new_full_base( b"reshare_validators", &format!("{}/validator/reshare", endpoint).into_bytes(), ); + offchain_db.local_storage_set( + sp_core::offchain::StorageKind::PERSISTENT, + b"rotate_keyshares", + &format!("{}/validator/rotate_keyshares", endpoint).into_bytes(), + ); offchain_db.local_storage_set( sp_core::offchain::StorageKind::PERSISTENT, b"attest", diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index 06b299b25..630aa4396 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -71,6 +71,7 @@ pub mod pallet { let _ = Self::post_reshare(block_number); let _ = Self::post_proactive_refresh(block_number); let _ = Self::post_attestation_request(block_number); + let _ = Self::post_rotate_keyshare(block_number); } fn on_initialize(_block_number: BlockNumberFor) -> Weight { @@ -96,6 +97,10 @@ pub mod pallet { /// Attestations request message passed AttestationRequestMessagePassed(OcwMessageAttestationRequest), + + /// Key Rotate Message passed to validators + /// parameters. [BlockNumberFor] + KeyRotatesMessagePassed(BlockNumberFor), } #[pallet::call] @@ -259,6 +264,49 @@ pub mod pallet { Ok(()) } + /// Submits a request to rotate parent network key the threshold servers. + pub fn post_rotate_keyshare(block_number: BlockNumberFor) -> Result<(), http::Error> { + let rotate_keyshares = pallet_staking_extension::Pallet::::rotate_keyshares(); + if rotate_keyshares != block_number { + return Ok(()); + } + + let deadline = sp_io::offchain::timestamp().add(Duration::from_millis(2_000)); + let kind = sp_core::offchain::StorageKind::PERSISTENT; + let from_local = sp_io::offchain::local_storage_get(kind, b"rotate_keyshares") + .unwrap_or_else(|| b"http://localhost:3001/validator/rotate_keyshares".to_vec()); + let url = str::from_utf8(&from_local) + .unwrap_or("http://localhost:3001/validator/rotate_keyshares"); + + log::warn!("propagation::post rotate keyshare"); + + let converted_block_number: u32 = + BlockNumberFor::::try_into(block_number).unwrap_or_default(); + + // We construct the request + // important: the header->Content-Type must be added and match that of the receiving + // party!! + let pending = http::Request::post(url, vec![converted_block_number.encode()]) + .deadline(deadline) + .send() + .map_err(|_| http::Error::IoError)?; + + // We await response, same as in fn get() + let response = + pending.try_wait(deadline).map_err(|_| http::Error::DeadlineReached)??; + + // check response code + if response.code != 200 { + log::warn!("Unexpected status code: {}", response.code); + return Err(http::Error::Unknown); + } + let _res_body = response.body().collect::>(); + + Self::deposit_event(Event::KeyRotatesMessagePassed(block_number)); + + Ok(()) + } + /// Submits a request for a TDX attestation. pub fn post_attestation_request( block_number: BlockNumberFor, diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs index e139c3cd4..05172236c 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -64,6 +64,14 @@ fn knows_how_to_mock_several_http_calls() { body: [32, 1, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0].to_vec(), ..Default::default() }); + state.expect_request(testing::PendingRequest { + method: "POST".into(), + uri: "http://localhost:3001/validator/rotate_keyshares".into(), + sent: true, + response: Some([].to_vec()), + body: [10, 0, 0, 0].to_vec(), + ..Default::default() + }); }); t.execute_with(|| { @@ -91,6 +99,9 @@ fn knows_how_to_mock_several_http_calls() { }); // now triggers Propagation::post_reshare(7).unwrap(); + + pallet_staking_extension::RotateKeyshares::::put(10); + Propagation::post_rotate_keyshare(10).unwrap(); }) } diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index 76af74d3b..fd4c79f50 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -230,6 +230,11 @@ pub mod pallet { #[pallet::getter(fn jump_start_progress)] pub type JumpStartProgress = StorageValue<_, JumpStartDetails, ValueQuery>; + /// Tell Signers to rotate keyshare + #[pallet::storage] + #[pallet::getter(fn rotate_keyshares)] + pub type RotateKeyshares = StorageValue<_, BlockNumberFor, ValueQuery>; + /// A type used to simplify the genesis configuration definition. pub type ThresholdServersConfig = ( ::ValidatorId, @@ -505,6 +510,9 @@ pub mod pallet { let current_signer_length = signers_info.next_signers.len(); if signers_info.confirmations.len() == (current_signer_length - 1) { Signers::::put(signers_info.next_signers.clone()); + RotateKeyshares::::put( + >::block_number() + sp_runtime::traits::One::one(), + ); Self::deposit_event(Event::SignersRotation(signers_info.next_signers)); Ok(Pays::No.into()) } else { From 519440e6e7c08c7281bb37bb058f6fc005270d9b Mon Sep 17 00:00:00 2001 From: peg Date: Fri, 30 Aug 2024 10:30:53 +0200 Subject: [PATCH 3/4] Check build-time measurement (MRTD) values when verifying TDX quotes (#1024) * Check MRTD value * Comment * Add accepted MRTD values parameter to parameters pallet * Use parameter from parameters pallet * Rm constant from entropy-shared * Add accepted MRTD values parameters to chainspec * Clippy * Use vec from sp_std * Rm unused dependency * Import vec, update staking pallet mock * Update mock for registry pallet * Update mock for attestation pallet * Add accepted MRTD values parameters to testnet chainspec * Import BoundedVec * Update weights and rename benchmark to match extrinsic * Suggestion from review * Update weights * Update weights --- node/cli/src/chain_spec/dev.rs | 4 + node/cli/src/chain_spec/integration_tests.rs | 4 + node/cli/src/chain_spec/testnet.rs | 6 +- pallets/attestation/Cargo.toml | 3 +- pallets/attestation/src/lib.rs | 9 +- pallets/attestation/src/mock.rs | 13 +- pallets/parameters/src/benchmarking.rs | 12 ++ pallets/parameters/src/lib.rs | 31 ++- pallets/parameters/src/mock.rs | 1 + pallets/parameters/src/weights.rs | 200 ++++++++++--------- pallets/registry/src/mock.rs | 4 +- pallets/staking/src/mock.rs | 4 +- runtime/src/weights/pallet_parameters.rs | 74 ++++--- 13 files changed, 238 insertions(+), 127 deletions(-) diff --git a/node/cli/src/chain_spec/dev.rs b/node/cli/src/chain_spec/dev.rs index 6a8fa3cd6..4d8b2fd68 100644 --- a/node/cli/src/chain_spec/dev.rs +++ b/node/cli/src/chain_spec/dev.rs @@ -308,6 +308,10 @@ pub fn development_genesis_config( max_instructions_per_programs: INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, total_signers: TOTAL_SIGNERS, threshold: SIGNER_THRESHOLD, + accepted_mrtd_values: vec![ + BoundedVec::try_from([0; 48].to_vec()).unwrap(), + BoundedVec::try_from([1; 48].to_vec()).unwrap(), + ], ..Default::default() }, "programs": ProgramsConfig { diff --git a/node/cli/src/chain_spec/integration_tests.rs b/node/cli/src/chain_spec/integration_tests.rs index c5dd75816..1c8c833bf 100644 --- a/node/cli/src/chain_spec/integration_tests.rs +++ b/node/cli/src/chain_spec/integration_tests.rs @@ -247,6 +247,10 @@ pub fn integration_tests_genesis_config( max_instructions_per_programs: INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, total_signers: TOTAL_SIGNERS, threshold: SIGNER_THRESHOLD, + accepted_mrtd_values: vec![ + BoundedVec::try_from([0; 48].to_vec()).unwrap(), + BoundedVec::try_from([1; 48].to_vec()).unwrap(), + ], ..Default::default() }, "programs": ProgramsConfig { diff --git a/node/cli/src/chain_spec/testnet.rs b/node/cli/src/chain_spec/testnet.rs index a3ec871ac..fb5848420 100644 --- a/node/cli/src/chain_spec/testnet.rs +++ b/node/cli/src/chain_spec/testnet.rs @@ -37,7 +37,7 @@ use sc_telemetry::TelemetryEndpoints; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; use sp_consensus_babe::AuthorityId as BabeId; use sp_core::{crypto::UncheckedInto, sr25519}; -use sp_runtime::Perbill; +use sp_runtime::{BoundedVec, Perbill}; /// The AccountID of a Threshold Signature server. This is to meant to be registered on-chain. type TssAccountId = sp_runtime::AccountId32; @@ -446,6 +446,10 @@ pub fn testnet_genesis_config( max_instructions_per_programs: INITIAL_MAX_INSTRUCTIONS_PER_PROGRAM, total_signers: TOTAL_SIGNERS, threshold: SIGNER_THRESHOLD, + accepted_mrtd_values: vec![ + BoundedVec::try_from([0; 48].to_vec()).unwrap(), + BoundedVec::try_from([1; 48].to_vec()).unwrap(), + ], ..Default::default() }, "programs": ProgramsConfig { diff --git a/pallets/attestation/Cargo.toml b/pallets/attestation/Cargo.toml index 0b3b25065..6149b527c 100644 --- a/pallets/attestation/Cargo.toml +++ b/pallets/attestation/Cargo.toml @@ -22,6 +22,7 @@ frame-benchmarking={ version="29.0.0", default-features=false, optional=true } sp-std ={ version="14.0.0", default-features=false } pallet-session ={ version="29.0.0", default-features=false, optional=true } +pallet-parameters={ version="0.2.0", path="../parameters", default-features=false } entropy-shared={ version="0.2.0", path="../../crates/shared", features=[ "wasm-no-std", ], default-features=false } @@ -37,7 +38,6 @@ pallet-timestamp ={ version="28.0.0", default-features=false } sp-npos-elections ={ version="27.0.0", default-features=false } frame-election-provider-support={ version="29.0.0", default-features=false } pallet-staking-reward-curve ={ version="11.0.0" } -pallet-parameters ={ version="0.2.0", path="../parameters", default-features=false } tdx-quote ={ git="https://github.com/entropyxyz/tdx-quote", features=["mock"] } rand_core ="0.6.4" @@ -51,6 +51,7 @@ std=[ 'log/std', 'pallet-staking-extension/std', 'pallet-balances/std', + 'pallet-parameters/std', 'sp-io/std', "sp-runtime/std", ] diff --git a/pallets/attestation/src/lib.rs b/pallets/attestation/src/lib.rs index e98d315ff..ebab0a19e 100644 --- a/pallets/attestation/src/lib.rs +++ b/pallets/attestation/src/lib.rs @@ -121,6 +121,8 @@ pub mod pallet { NoStashAccount, /// Cannot lookup associated TS server info NoServerInfo, + /// Unacceptable VM image running + BadMrtdValue, } #[pallet::call] @@ -167,8 +169,11 @@ pub mod pallet { Error::::IncorrectInputData ); - // TODO #982 Check measurements match current release of entropy-tss - let _mrtd = quote.mrtd(); + // Check build-time measurement matches a current-supported release of entropy-tss + let mrtd_value = BoundedVec::try_from(quote.mrtd().to_vec()) + .map_err(|_| Error::::BadMrtdValue)?; + let accepted_mrtd_values = pallet_parameters::Pallet::::accepted_mrtd_values(); + ensure!(accepted_mrtd_values.contains(&mrtd_value), Error::::BadMrtdValue); // TODO #982 Check that the attestation public key matches that from PCK certificate let _attestation_key = quote.attestation_key; diff --git a/pallets/attestation/src/mock.rs b/pallets/attestation/src/mock.rs index fea3719bc..086153d1f 100644 --- a/pallets/attestation/src/mock.rs +++ b/pallets/attestation/src/mock.rs @@ -29,7 +29,7 @@ use sp_runtime::{ curve::PiecewiseLinear, testing::{TestXt, UintAuthorityId}, traits::{BlakeTwo256, ConvertInto, IdentityLookup}, - BuildStorage, Perbill, + BoundedVec, BuildStorage, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; use std::cell::RefCell; @@ -352,5 +352,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities { }; pallet_staking_extension.assimilate_storage(&mut t).unwrap(); + let pallet_parameters = pallet_parameters::GenesisConfig:: { + request_limit: 5u32, + max_instructions_per_programs: 5u64, + total_signers: 3u8, + threshold: 2u8, + accepted_mrtd_values: vec![BoundedVec::try_from([0; 48].to_vec()).unwrap()], + _config: Default::default(), + }; + + pallet_parameters.assimilate_storage(&mut t).unwrap(); + t.into() } diff --git a/pallets/parameters/src/benchmarking.rs b/pallets/parameters/src/benchmarking.rs index 0d24fa885..a0ff82bc1 100644 --- a/pallets/parameters/src/benchmarking.rs +++ b/pallets/parameters/src/benchmarking.rs @@ -16,6 +16,7 @@ use frame_benchmarking::benchmarks; use frame_support::assert_ok; use frame_system::EventRecord; +use sp_std::vec; use super::*; #[allow(unused)] @@ -65,6 +66,17 @@ benchmarks! { assert_last_event::(Event::SignerInfoChanged{ signer_info }.into()); } + change_accepted_mrtd_values { + let origin = T::UpdateOrigin::try_successful_origin().unwrap(); + let accepted_mrtd_values = vec![BoundedVec::try_from([0; 48].to_vec()).unwrap()]; + }: { + assert_ok!( + >::change_accepted_mrtd_values(origin, accepted_mrtd_values.clone()) + ); + } + verify { + assert_last_event::(Event::AcceptedMrtdValuesChanged{ accepted_mrtd_values }.into()); + } impl_benchmark_test_suite!(Parameters, crate::mock::new_test_ext(), crate::mock::Runtime); } diff --git a/pallets/parameters/src/lib.rs b/pallets/parameters/src/lib.rs index cad018b12..4be3c7614 100644 --- a/pallets/parameters/src/lib.rs +++ b/pallets/parameters/src/lib.rs @@ -13,7 +13,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//! # Programs Parameters +//! # Parameters Pallet //! //! ## Overview //! @@ -37,6 +37,7 @@ use entropy_shared::MAX_SIGNERS; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use sp_runtime::DispatchResult; +use sp_std::vec::Vec; #[cfg(test)] mod mock; @@ -67,6 +68,8 @@ pub mod module { type WeightInfo: WeightInfo; } + pub type MrtdValues = Vec>>; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig { @@ -74,6 +77,7 @@ pub mod module { pub max_instructions_per_programs: u64, pub threshold: u8, pub total_signers: u8, + pub accepted_mrtd_values: MrtdValues, #[serde(skip)] pub _config: sp_std::marker::PhantomData, } @@ -83,6 +87,10 @@ pub mod module { fn build(&self) { assert!(self.threshold > 0, "Threhsold too low"); assert!(self.total_signers >= self.threshold, "Threshold is larger then signer"); + assert!( + !self.accepted_mrtd_values.is_empty(), + "At least one accepted MRTD value is required" + ); RequestLimit::::put(self.request_limit); MaxInstructionsPerPrograms::::put(self.max_instructions_per_programs); let signer_info = SignersSize { @@ -91,6 +99,7 @@ pub mod module { last_session_change: 0, }; SignersInfo::::put(signer_info); + AcceptedMrtdValues::::put(self.accepted_mrtd_values.clone()); } } @@ -128,6 +137,8 @@ pub mod module { MaxInstructionsPerProgramsChanged { max_instructions_per_programs: u64 }, /// Signer Info changed SignerInfoChanged { signer_info: SignersSize }, + /// Accepted MRTD values changed + AcceptedMrtdValuesChanged { accepted_mrtd_values: MrtdValues }, } /// The request limit a user can ask to a specific set of TSS in a block @@ -145,6 +156,12 @@ pub mod module { #[pallet::getter(fn signers_info)] pub type SignersInfo = StorageValue<_, SignersSize, ValueQuery>; + /// Accepted values of the TDX build-time measurement register - from the currently-supported + /// releases of entropy-tss + #[pallet::storage] + #[pallet::getter(fn accepted_mrtd_values)] + pub type AcceptedMrtdValues = StorageValue<_, MrtdValues, ValueQuery>; + #[pallet::pallet] #[pallet::without_storage_info] pub struct Pallet(_); @@ -205,5 +222,17 @@ pub mod module { Self::deposit_event(Event::SignerInfoChanged { signer_info }); Ok(()) } + + #[pallet::call_index(3)] + #[pallet::weight( ::WeightInfo::change_accepted_mrtd_values())] + pub fn change_accepted_mrtd_values( + origin: OriginFor, + accepted_mrtd_values: MrtdValues, + ) -> DispatchResult { + T::UpdateOrigin::ensure_origin(origin)?; + AcceptedMrtdValues::::put(&accepted_mrtd_values); + Self::deposit_event(Event::AcceptedMrtdValuesChanged { accepted_mrtd_values }); + Ok(()) + } } } diff --git a/pallets/parameters/src/mock.rs b/pallets/parameters/src/mock.rs index 726bbe025..812da2b27 100644 --- a/pallets/parameters/src/mock.rs +++ b/pallets/parameters/src/mock.rs @@ -138,6 +138,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { max_instructions_per_programs: 5u64, total_signers: 5u8, threshold: 3u8, + accepted_mrtd_values: vec![BoundedVec::try_from([0; 48].to_vec()).unwrap()], _config: Default::default(), }; pallet_parameters.assimilate_storage(&mut t).unwrap(); diff --git a/pallets/parameters/src/weights.rs b/pallets/parameters/src/weights.rs index 1cef038d8..b78620c33 100644 --- a/pallets/parameters/src/weights.rs +++ b/pallets/parameters/src/weights.rs @@ -13,13 +13,13 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//! Autogenerated weights for pallet_transaction_pause +//! Autogenerated weights for `pallet_parameters` //! -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-11-02, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 33.0.0 +//! DATE: 2024-08-30, STEPS: `5`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `hcastano`, CPU: `` -//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! HOSTNAME: `turnip`, CPU: `Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: 1024 // Executed Command: // ./target/release/entropy @@ -27,106 +27,128 @@ // pallet // --chain // dev -// --wasm-execution=compiled -// --pallet -// pallet_transaction_pause -// --extrinsic -// * -// --steps -// 50 -// --repeat -// 20 -// --template -// .maintain/frame-weight-template.hbs -// --output -// pallets/transaction-pause/src/weights.rs +// --pallet=pallet_parameters +// --extrinsic=change_mrtd_values +// --steps=5 +// --repeat=2 +// --header=.maintain/AGPL-3.0-header.txt +// --output=./runtime/src/weights/ #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] #![allow(unused_imports)] #![allow(missing_docs)] -use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use frame_support::{traits::Get, weights::{constants::RocksDbWeight, Weight}}; use core::marker::PhantomData; /// Weight functions needed for pallet_transaction_pause. pub trait WeightInfo { - fn change_request_limit() -> Weight; - fn max_instructions_per_programs() -> Weight; - fn change_signers_info() -> Weight; + fn change_request_limit() -> Weight; + fn max_instructions_per_programs() -> Weight; + fn change_signers_info() -> Weight; + fn change_accepted_mrtd_values() -> Weight; } -/// Weights for pallet_transaction_pause using the Substrate node and recommended hardware. +/// Weight functions for `pallet_parameters`. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - /// Storage: `Parameters::RequestLimit` (r:0 w:1) - /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_request_limit() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 5_000_000 picoseconds. - Weight::from_parts(6_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) - /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn max_instructions_per_programs() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 3_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Parameters::SignersInfo` (r:0 w:1) - /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_signers_info() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } + /// Storage: `Parameters::RequestLimit` (r:0 w:1) + /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_request_limit() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 5_000_000 picoseconds. + Weight::from_parts(6_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) + /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn max_instructions_per_programs() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 3_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::SignersInfo` (r:0 w:1) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_signers_info() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::AcceptedMrtdValues` (r:0 w:1) + /// Proof: `Parameters::AcceptedMrtdValues` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_accepted_mrtd_values() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 14_134_000 picoseconds. + Weight::from_parts(22_719_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } } // For backwards compatibility and tests impl WeightInfo for () { - /// Storage: `Parameters::RequestLimit` (r:0 w:1) - /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_request_limit() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 5_000_000 picoseconds. - Weight::from_parts(6_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(RocksDbWeight::get().writes(1)) - } - /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) - /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn max_instructions_per_programs() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 3_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(RocksDbWeight::get().writes(1)) - } - /// Storage: `Parameters::SignersInfo` (r:0 w:1) - /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_signers_info() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(RocksDbWeight::get().writes(1)) - } + /// Storage: `Parameters::RequestLimit` (r:0 w:1) + /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_request_limit() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 5_000_000 picoseconds. + Weight::from_parts(6_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(RocksDbWeight::get().writes(1)) + } + + /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) + /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn max_instructions_per_programs() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 3_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(RocksDbWeight::get().writes(1)) + } + + /// Storage: `Parameters::SignersInfo` (r:0 w:1) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_signers_info() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(RocksDbWeight::get().writes(1)) + } + + /// Storage: `Parameters::AcceptedMrtdValues` (r:0 w:1) + /// Proof: `Parameters::AcceptedMrtdValues` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_accepted_mrtd_values() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 14_134_000 picoseconds. + Weight::from_parts(22_719_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(RocksDbWeight::get().writes(1)) + } } diff --git a/pallets/registry/src/mock.rs b/pallets/registry/src/mock.rs index dc5b75608..3d2c10f19 100644 --- a/pallets/registry/src/mock.rs +++ b/pallets/registry/src/mock.rs @@ -29,9 +29,10 @@ use sp_runtime::{ curve::PiecewiseLinear, testing::{TestXt, UintAuthorityId}, traits::{BlakeTwo256, ConvertInto, IdentityLookup}, - BuildStorage, Perbill, + BoundedVec, BuildStorage, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; +use sp_std::vec; use std::cell::RefCell; use crate as pallet_registry; @@ -387,6 +388,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { max_instructions_per_programs: 5u64, total_signers: 3u8, threshold: 2u8, + accepted_mrtd_values: vec![BoundedVec::try_from([0; 48].to_vec()).unwrap()], _config: Default::default(), } .assimilate_storage(&mut t) diff --git a/pallets/staking/src/mock.rs b/pallets/staking/src/mock.rs index 372866da4..0d1fe7159 100644 --- a/pallets/staking/src/mock.rs +++ b/pallets/staking/src/mock.rs @@ -31,9 +31,10 @@ use sp_runtime::{ curve::PiecewiseLinear, testing::{TestXt, UintAuthorityId}, traits::{BlakeTwo256, ConvertInto, IdentityLookup, OpaqueKeys, Zero}, - BuildStorage, KeyTypeId, Perbill, + BoundedVec, BuildStorage, KeyTypeId, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; +use sp_std::vec; use crate as pallet_staking_extension; @@ -411,6 +412,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { max_instructions_per_programs: 5u64, total_signers: 3u8, threshold: 2u8, + accepted_mrtd_values: vec![BoundedVec::try_from([0; 48].to_vec()).unwrap()], _config: Default::default(), } .assimilate_storage(&mut t) diff --git a/runtime/src/weights/pallet_parameters.rs b/runtime/src/weights/pallet_parameters.rs index 20c750acb..24ae04b49 100644 --- a/runtime/src/weights/pallet_parameters.rs +++ b/runtime/src/weights/pallet_parameters.rs @@ -16,9 +16,9 @@ //! Autogenerated weights for `pallet_parameters` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 33.0.0 -//! DATE: 2024-07-16, STEPS: `5`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-08-30, STEPS: `5`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `Jesses-MacBook-Pro.local`, CPU: `` +//! HOSTNAME: `turnip`, CPU: `Intel(R) Core(TM) i7-4710MQ CPU @ 2.50GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: 1024 // Executed Command: @@ -28,7 +28,7 @@ // --chain // dev // --pallet=pallet_parameters -// --extrinsic=* +// --extrinsic=change_accepted_mrtd_values // --steps=5 // --repeat=2 // --header=.maintain/AGPL-3.0-header.txt @@ -45,36 +45,50 @@ use core::marker::PhantomData; /// Weight functions for `pallet_parameters`. pub struct WeightInfo(PhantomData); impl pallet_parameters::WeightInfo for WeightInfo { - /// Storage: `Parameters::RequestLimit` (r:0 w:1) - /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_request_limit() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) - /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn max_instructions_per_programs() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 3_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) - .saturating_add(Weight::from_parts(0, 0)) - .saturating_add(T::DbWeight::get().writes(1)) - } - /// Storage: `Parameters::SignersInfo` (r:0 w:1) - /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn change_signers_info() -> Weight { + /// Storage: `Parameters::RequestLimit` (r:0 w:1) + /// Proof: `Parameters::RequestLimit` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_request_limit() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 5_000_000 picoseconds. + Weight::from_parts(6_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::MaxInstructionsPerPrograms` (r:0 w:1) + /// Proof: `Parameters::MaxInstructionsPerPrograms` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn max_instructions_per_programs() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 3_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::SignersInfo` (r:0 w:1) + /// Proof: `Parameters::SignersInfo` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_signers_info() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 4_000_000 picoseconds. + Weight::from_parts(4_000_000, 0) + .saturating_add(Weight::from_parts(0, 0)) + .saturating_add(T::DbWeight::get().writes(1)) + } + + /// Storage: `Parameters::AcceptedMrtdValues` (r:0 w:1) + /// Proof: `Parameters::AcceptedMrtdValues` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + fn change_accepted_mrtd_values() -> Weight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 4_000_000 picoseconds. - Weight::from_parts(4_000_000, 0) + // Minimum execution time: 14_134_000 picoseconds. + Weight::from_parts(22_719_000, 0) .saturating_add(Weight::from_parts(0, 0)) .saturating_add(T::DbWeight::get().writes(1)) } From 624b37d3bad1acdee74f724c7a2cf2a3e2ab0159 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 30 Aug 2024 11:10:35 -0400 Subject: [PATCH 4/4] Fix `change_signers_info` benchmark (#1035) In https://github.com/entropyxyz/entropy-core/pull/978/ a couple of extra checks were added to ensure that parameters related to signing could only change so quickly. This ended up breaking one of the benchmarks since the benchmark setup used a difference that was larger than the allowed amount. I've fixed that by reading the current values from storage (set at Genesis) and updating those by the allowed amount. --- pallets/parameters/src/benchmarking.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pallets/parameters/src/benchmarking.rs b/pallets/parameters/src/benchmarking.rs index a0ff82bc1..6359bc1ce 100644 --- a/pallets/parameters/src/benchmarking.rs +++ b/pallets/parameters/src/benchmarking.rs @@ -56,7 +56,18 @@ benchmarks! { change_signers_info { let origin = T::UpdateOrigin::try_successful_origin().unwrap(); pallet_session::CurrentIndex::::put(1); - let signer_info = SignersSize { total_signers: 5, threshold: 3, last_session_change: 1 }; + + let SignersSize { + threshold: old_threshold, + total_signers: old_total_signers, + last_session_change: old_last_session_change, + } = SignersInfo::::get(); + + let signer_info = SignersSize { + total_signers: old_total_signers + 1, + threshold: old_threshold + 1, + last_session_change: old_last_session_change + 1, + }; }: { assert_ok!( >::change_signers_info(origin, signer_info.total_signers, signer_info.threshold)