From ba572ae892d4e4fae89ca053d8a137117b0f3a17 Mon Sep 17 00:00:00 2001 From: PG Herveou <pgherveou@gmail.com> Date: Mon, 13 Jan 2025 15:49:37 +0100 Subject: [PATCH] [pallet-revive] Update gas encoding (#6689) Update the current approach to attach the `ref_time`, `pov` and `deposit` parameters to an Ethereum transaction. Previously we will pass these 3 parameters along with the signed payload, and check that the fees resulting from `gas x gas_price` match the actual fees paid by the user for the extrinsic. This approach unfortunately can be attacked. A malicious actor could force such a transaction to fail by injecting low values for some of these extra parameters as they are not part of the signed payload. The new approach encodes these 3 extra parameters in the lower digits of the transaction gas, approximating the the log2 of the actual values to encode each components on 2 digits --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: command-bot <> --- .../assets/asset-hub-westend/src/lib.rs | 1 + prdoc/pr_6689.prdoc | 19 ++ substrate/bin/node/runtime/src/lib.rs | 1 + .../frame/revive/rpc/examples/js/bun.lockb | Bin 40649 -> 40649 bytes .../frame/revive/rpc/examples/js/package.json | 4 +- .../rpc/examples/js/src/geth-diff.test.ts | 22 -- .../revive/rpc/examples/js/src/piggy-bank.ts | 15 +- .../frame/revive/rpc/revive_chain.metadata | Bin 659977 -> 661594 bytes substrate/frame/revive/rpc/src/client.rs | 4 +- substrate/frame/revive/rpc/src/lib.rs | 23 +- substrate/frame/revive/src/evm.rs | 2 + substrate/frame/revive/src/evm/api/byte.rs | 5 +- substrate/frame/revive/src/evm/gas_encoder.rs | 174 +++++++++++++ substrate/frame/revive/src/evm/runtime.rs | 228 +++++++++--------- substrate/frame/revive/src/lib.rs | 31 ++- 15 files changed, 340 insertions(+), 189 deletions(-) create mode 100644 prdoc/pr_6689.prdoc create mode 100644 substrate/frame/revive/src/evm/gas_encoder.rs diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 5fb495e4e8c..71cfdc58cce 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -981,6 +981,7 @@ impl pallet_revive::Config for Runtime { type Xcm = pallet_xcm::Pallet<Self>; type ChainId = ConstU64<420_420_421>; type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12. + type EthGasEncoder = (); } impl TryFrom<RuntimeCall> for pallet_revive::Call<Runtime> { diff --git a/prdoc/pr_6689.prdoc b/prdoc/pr_6689.prdoc new file mode 100644 index 00000000000..2cbb49cd7dd --- /dev/null +++ b/prdoc/pr_6689.prdoc @@ -0,0 +1,19 @@ +title: '[pallet-revive] Update gas encoding' +doc: +- audience: Runtime Dev + description: |- + Update the current approach to attach the `ref_time`, `pov` and `deposit` parameters to an Ethereum transaction. +Previously, these three parameters were passed along with the signed payload, and the fees resulting from gas × gas_price were checked to ensure they matched the actual fees paid by the user for the extrinsic + + This approach unfortunately can be attacked. A malicious actor could force such a transaction to fail by injecting low values for some of these extra parameters as they are not part of the signed payload. + + The new approach encodes these 3 extra parameters in the lower digits of the transaction gas, using the log2 of the actual values to encode each components on 2 digits +crates: +- name: pallet-revive-eth-rpc + bump: minor +- name: pallet-revive + bump: minor +- name: asset-hub-westend-runtime + bump: minor +- name: pallet-revive-mock-network + bump: minor diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 93b134e8165..7de04b27ff8 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1468,6 +1468,7 @@ impl pallet_revive::Config for Runtime { type Xcm = (); type ChainId = ConstU64<420_420_420>; type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12. + type EthGasEncoder = (); } impl pallet_sudo::Config for Runtime { diff --git a/substrate/frame/revive/rpc/examples/js/bun.lockb b/substrate/frame/revive/rpc/examples/js/bun.lockb index 46994bb147547bfb9b960b7630d1a6d274ee75dd..67df5841e43fba141c7a146a1e4a8958b4c7a84c 100755 GIT binary patch delta 279 zcmV+y0qFk8y#mR-0+22soiH!oX)-nTSX|in0HhK|x0gz@V`@A~BLYiBe~%4#u}+#2 zlVl7ilMoXalcWwPlaLz;vsn|wBtT0(J!NEn1~X2p?UXB7Jvi1>R<9n);G^G0U~|Q5 zMB7D`Fj;o&?!D+#cbOwVUcVM=HzK{nVywxpEIqXw>`RkfI2b@*8u|njLIC6<#`-5a zC$;wp^T;@qwKuAj(~g=w!jb0XqITSH4R$<SCAs_2cDiF<8mE~?o{3IGI1jtCmts+~ zwKyj*0X(zaXb%AeI{*LxJhSm>p8^RE0000m0000WvzcsD2a_Ol8k6jEG_yK%I~4&l dlW~tAlTe%nlg@(=1Tio!GLvzDAG6Mh`xGRLa$o=e delta 274 zcmV+t0qy?Dy#mR-0+22sm$qsC-QFrj08A7+@VAjr|M5l`Sq&6l^96BcM<AV~u}+#2 zlV}StlNb{alPC%>vse?wBtSbdFKlv(4w$;pImPzg)KO3c)E*{sr!?C>Pa^u})5WrH z?2dCjw;3D6xm-<8UmEmHQmoo8gdP^y(@S_h*pic7I2b^vs4P<l$7NlX31YyVQ}rgP z=@X(R>K)kA=h%S6+OX6+SMP!aeojwTtkMl1`0TE2v1*0>msHX$nBE8?5g-|}wKyj* z0XwtZXb%AeIsgCwJG1d=p8^Rm0000W0000EvzcsD2a_Cg5|bEoB9rWM8nZfdI~9|J YfgqDloClN6gAV~QlVOG;v(Ab86w_&Re*gdg diff --git a/substrate/frame/revive/rpc/examples/js/package.json b/substrate/frame/revive/rpc/examples/js/package.json index 6d8d00fd421..0119f4f34a1 100644 --- a/substrate/frame/revive/rpc/examples/js/package.json +++ b/substrate/frame/revive/rpc/examples/js/package.json @@ -9,10 +9,10 @@ "preview": "vite preview" }, "dependencies": { + "@parity/revive": "^0.0.5", "ethers": "^6.13.4", "solc": "^0.8.28", - "viem": "^2.21.47", - "@parity/revive": "^0.0.5" + "viem": "^2.21.47" }, "devDependencies": { "prettier": "^3.3.3", diff --git a/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts b/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts index b9ee877927b..871adeccbc9 100644 --- a/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts +++ b/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts @@ -289,27 +289,5 @@ for (const env of envs) { ], }) }) - - test.only('eth_estimate (no gas specified) child_call', async () => { - let balance = await env.serverWallet.getBalance(env.accountWallet.account) - expect(balance).toBe(0n) - - const data = encodeFunctionData({ - abi: FlipperCallerAbi, - functionName: 'callFlip', - }) - - await env.accountWallet.request({ - method: 'eth_estimateGas', - params: [ - { - data, - from: env.accountWallet.account.address, - to: flipperCallerAddr, - gas: `0x${Number(1000000).toString(16)}`, - }, - ], - }) - }) }) } diff --git a/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts b/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts index 0040b0c78dc..8289ac8b76e 100644 --- a/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts +++ b/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts @@ -1,9 +1,9 @@ import { assert, getByteCode, walletClient } from './lib.ts' -import { abi } from '../abi/piggyBank.ts' +import { PiggyBankAbi } from '../abi/piggyBank.ts' import { parseEther } from 'viem' const hash = await walletClient.deployContract({ - abi, + abi: PiggyBankAbi, bytecode: getByteCode('piggyBank'), }) const deployReceipt = await walletClient.waitForTransactionReceipt({ hash }) @@ -16,7 +16,7 @@ assert(contractAddress, 'Contract address should be set') const result = await walletClient.estimateContractGas({ account: walletClient.account, address: contractAddress, - abi, + abi: PiggyBankAbi, functionName: 'deposit', value: parseEther('10'), }) @@ -26,7 +26,7 @@ assert(contractAddress, 'Contract address should be set') const { request } = await walletClient.simulateContract({ account: walletClient.account, address: contractAddress, - abi, + abi: PiggyBankAbi, functionName: 'deposit', value: parseEther('10'), }) @@ -36,9 +36,6 @@ assert(contractAddress, 'Contract address should be set') const receipt = await walletClient.waitForTransactionReceipt({ hash }) console.log(`Deposit receipt: ${receipt.status}`) - if (process.env.STOP) { - process.exit(0) - } } // Withdraw 5 WST @@ -46,7 +43,7 @@ assert(contractAddress, 'Contract address should be set') const { request } = await walletClient.simulateContract({ account: walletClient.account, address: contractAddress, - abi, + abi: PiggyBankAbi, functionName: 'withdraw', args: [parseEther('5')], }) @@ -58,7 +55,7 @@ assert(contractAddress, 'Contract address should be set') // Check remaining balance const balance = await walletClient.readContract({ address: contractAddress, - abi, + abi: PiggyBankAbi, functionName: 'getDeposit', }) diff --git a/substrate/frame/revive/rpc/revive_chain.metadata b/substrate/frame/revive/rpc/revive_chain.metadata index 64b1f2014dd06815fcea6a87bc96306eb00eda8b..402e8c2d22b21471929e9c61acd2cc968af614cf 100644 GIT binary patch delta 4640 zcmaJ_dr%eE8Q*j7Ue4X?W7i0Zh{+N~A%Pn(V6jRvK5&AG&q$0l3@-N`VCCN3>+W72 z8bzj|;2(C9rsmW%w$a8`nsH(#V^=%csYXprCp9=4<6AUpe1O4;&DcEJ@7zVX*PE$> z?Cv?=<M%zzcfOBx@8q1?k&_Z8mLh)An$nhncBZ)A*pd;M9?}E#bEDyEV$3r1@O(pW zP=mynSJVFP7MK0*7I)Lhlme+b8jifaIeUct^vnDyAIhZ}?fkQkaK&g*9gz)1tCzz{ zvus*wC?r=Cc_lG)zf7;J;qy~uMGKZ8`j>V-8_D!+JO4PcsL;Vz4=qx|5#6w=!h~sO z;@yLC9MLtL@_$9oP&B3|poBssC|5Uwl4e~avaYc=IiSOOz)~BC><E|hWmOA=qCr)w z&9eZrNi?vMu5IUUxDcY7C%V(<-nIN7I=h|srS&seI#Z%*!DzrDL3?=z-*g)V>7|vP z{V0?Eag|4IDUZQy)Q$R(t_0=Aeh1WoYJege#3V+8VyU{8Cs#);*=p2fQwb3{LJU<0 z=z3HODuzl-IUF@DIii^6ud7LoZV;K_46ql~l$C%L1!KWb(^P9U67+L>^5tc8REsCi z{`V@+%}lWfm;$8AQAxbC>!Fkpbl6*-H227YZS+s8JYxpSYB)@Ss$!8)Go8EEGZ9Ur z->mXHGYUed7#29EM2@HNSG@6tC#%KL877(~n{|3L1hB!eN|4A=4MH9R8fU3&DfPr$ zfMl7})Ib>ga=FE92$33afbQ7EB4@BKqJo`<tOSFw0aO}kbgb45a353MQtL!jXM{~M zh&Ss^M-b4HCKd8dtBo=Fxk|K%N$K|jkbx8LhX+!TEocza<pAt~T~+G`w-~}12|<d1 zM}fYD0H`KQK@SAa^vf|_dIK>U4b_4evgErDXuxe|za#(_3#bBqU`fcFF6%Xm*auS) zpBnx=vr&mug@|I`_>CuH&~P4QMwDijW+f=|1rnGVEorZK#{>UU?232r1UG%}&6HWW zpdm<rO_R{z=#vvUG_#u2DGjO~HL3up390Dp*k~I*mztV$cj)&+k?0WxahRAUjuNxP zT)S@%-ZPxKj^On1C5=R_t+ODn0wF~W7cGu`E_A+leh~-%@<p087p2m_e~KSPh4d0U zMih&vgrO{JvJ6!-)xc96SJ+SYL>GQhC>A-+RVZ?k>8o9MHY%m3yKwG;X(FFKFTxHC z$VR0i@=9~zWP=jvc|A;ct`yi1$CUxyLSjZkmKW$=;mc4t(0b?9^V};8(%>?~;JI>< zqo<GJ3H0Tocs!az|8NvPI(mVKo~h94HN8<=1hvbAvjysy&^h*nqnJl%0fpyS>OF?% zq6)hB7|upZ=;~wmvBD)HFHc`&5Hd%Jc==~hE6=}xN;saY1X!_OE=A<vhvU7id<jSB z=g08E;T0k(DAyo;;S9}GEQRA#rs7drbsSGB2q)OGLX0<N1(Q2q4MVxYVZC}B??6$S zaRO)2-JjvY{N@BPtC^VAdp=nmC#DtN5!FxNadgTFJPEZrGG0G{OHmu`I)QW1I{NJi zoIQShLNMBrgV7cfm30!2q?MoJLudmXcM|_AcVhzIrewZNjF00s(b1>yRJ56{IgKBt zA6>-5(N=o;B7O<IPv5(MOU11s;umr#oj^IY%$>CREFR<EnqX%qv$L*0^qp~L*2PqG zGT9|)JGD>aEO?weji-S0n8GvNxV&Iz0@mHhSa&;E*~2xT+v~7?d<Mkqi^F{|0r$Zl z!+kIT_u+Wlhm&w0c4W^!hyAFNHlM@!{!bG4yW;t~lK8tE{+ZYC2<knLXQm!c;J%gu zUOP#boyV!(la7HW9ga88<LA*?+It?4&gd4AyfiwgXo_4^$#LC4=jI_B3%JX4UpLOA zPhY?fp=*xieX)q4JGAEl9*3Mz!5+|!rf;3b8K}nzK@aWdhJf_3ouL0tBC2?st|i%Y z*O8`Oghc&*0D%ih^vp%P6lAibad{-#djXH_$G~<AyKx2uSB|7NF5y9d>YGb=tluNW zmk=by<q?twPMwoS0zG#Hr_*I$;6gNn{^|=nJ!6!Fa#)3%-VCMxDGtejIz`p0)L<6v z{Q{53$d=Glr<6^d77W6T#E1shDrOda_%a?nDoa8I{reRYizmTms;WhzRxVw58Bai2 zbme78*)00jW&GIeA0gIsEkKgjy_Wu?n$wN3R&#{pQ|SsGIU|XW8v^basJz3Zq{=%C zoC1fIJzQxLmv-MHL+PtmaG5ZQZH$q)F>K?>+^ht7XOj=sStswZ=})iX2@7%)$j2nB z2gyu*ega)#GF_oVSIAI|p+~M_sjwJ$1}=-qEYhXG7`rVd1NprITk5b*j%7?H&HEC+ zm<hlo79I{sL8#pK?_RjhO6l8Q;-ybaOORd`pGReJ!ks)S>(53wn2XZN6F6gMa*jlm z^LV&0$AJQ$vE+qQIfCB3hR4$#*P(9DxQ@ecGrfNuF3u(NyX$yj?vez*R3`hS(veun zxW>|zH*j#NlE6cfd5FWq9=ta_ol_GS!pRI_haoJ{?nCZWI_D-{>{`+P@z<NU04|st zH}P}uDCxo3{%C>&iXbk{_u^7C5SC^~Xfyq)7pKzS_uy?Us}mRt;u#B)7+VvF+gO0^ zt7!w-_rl+=hb{5mb<%4Rw+>!i>m_cz<eh0os={HTDq^T%mHxLEd+0xVq0+s^P&GZi zAwfq+yo!zoG|*eOa2nlq3;$&JCMW^VLt%jaV}6}tLYuRR-oAzPlFbR?wlc`u?|Eix z9OUir4u^s*xjH4TlWzP9GGjZ%eR%Pd_d&?OiQbtUt(^`xd$@K>9Q=UcC7lA?avS?_ z9$avT`tXE72ibnMmpd$ZDca&mwFlqE-wr{~?euz|7Fw3w@9+{@LlQ(>j+f&y--a$3 zdhl4=2z`01i-e)g>8%O0%l-YKpUXl{L#=AC8IDa&vDv3VQA0{K3{3;$28?geII}Lg zWk3SFkwBC+bAAvthmB4VP^g+P?f@xt-w#MONEn*;AS}zvG~FUqN+c3eNwDfZsAYa` z1(KiBYt?`p)Y+I}LKiAWbePqGsu@wN*@3zN<JhDK>YZ6G8i9hr#yC)HTDqas!iO#^ z4Z0eXgHg~Ffc-r1Nk3crEWvBf5(M=Fly2|XBUF0m!~2D7ltX9k7dnR}w%)m72kqG} z%tQIK^h2QpO{NVW3Liil{n!CvCYomJ2ZZg&T_~2(M-B-`+!bOm?K>na_vMSRI#wYP zN<I>LQdQAwH5H0E)$9)eRgi>rMPa+<AHocm7!}9Ch++{o`5nZ~_O6eGKfBy*;yU}Q zZeg^Gu00|YqxJR=-wAIbdw!RYF&Ium{aK-f-yre_Xu&yQEFE@E$bhym@0{=|+DL(P zmhC<-Tz9!QIbyc+(Ek1IiqI+Ub%2#fFdthb`)K10VK#mHsxS?GLN8twevXdYHD3x* z1kK5rYeF}=9AlI2#<R6TUjTed-GWq1kKYhh!GOEqrtk_Xw?DfntU=yFaWET=7SNY^ zg;DpuD(uz0LWc_}G^J0t4Bh|DK4B%QuxoD%6)vRGy<ZEzh?za>j<DR7zCxN>PnxS> zd?2hG=z0yfM3hTu`5ht8{`wo?x2}}fN@Z+?x(>L<(SnN*+n)ajlFRYfa5ukR^p;Wd zy^y*g*Y`^l-LT6y39a66%eNdl@}D8!p+W1U)R_3!qz(4tE?);qSoxdVw+n$!8$G`1 zJk*O_6s7pa&{H1YSa9hN9^YcrX}^%-YvZBcJ>&CjMtkiwgM57m?Xx38d{u6A(Ed~c Jo$kXD_dmxEok9Qr delta 3431 zcmZu!3s6+o8NTQ4UH&~QdM^cZ@sWUvLPyAA0AU0J=yoJFD#l0E(#@*Zu*$Ny4<bIh zQ>oG!GI28I7aePHWI8l)M#r&Noj55;X(3azI->-OiA1S@g5s-MqtkP_Y}wn}nT2!r zod5gIcfNDa|DU~nGNtx-N|Z+qxWu+lR815)6UBdW>|;w^?B4Uoc;);tQ=pT~Qr_4h zJh75eJhD$%G%0a;nVjfzRcuc5xT<sf0e5+M;yO8To9y%A;#%RZviM@`6_-Kdu zOJ3TFfB&U99=|wjwxGD%JX;CvGT)o5{BXoPZ48OV#3pkbZduCD`b06u?9FU4v%e@M zvk8(-#D;Z8qa1kjGl)r_<*IZ$t7M<w?X9RMBs!Rp&5;G`<T6*4+Z*)#&ydpk8GIWz ze){uqFUJwlEK)H>j29EcB$0bQ4)=cv2}(yZ9F4`Pi%BfDUWKVbn#ftO<RQeqMpERh z0iV0V?_TfE5J}ETd3~NE+%pePUIiP@xduO@86wB=43~umuff|S7jIpI)Rp<7U|Uid zU^)e1Ims1?MOy5wkU5T%-t~H9fy-qu!f}NxZkg;4mIo{>Zjay~4i;-!vRUBvuwX9Q z;qwWcLko7`z&9`z_jJGvQjDK=z*O=MUhIGbbalWqw0;BMvkn%sg_NMP6Ozdatm%Xw zv*u(c*lepA4X6I&OGS=-IKdJUayS>>?1W{pB_gpGSCq*<xgxN@6>xE!tj+iSb;!26 zbwWL2q*9N{tC#A*_UmwkY{B3C1D4r?I&ifL-WgtIbp&`PduFCotEeQL-v!y2)WueO zxC`=02s^tVmDHf=1|+A}>I@DU4Gw8sby#u(uH(`h5RV1l!uiw#I$*sKSg!%=RbU*> z`3_RZ5q$4E$RG_^@ICyToWO!!$QK(#A}r&GO|D{w`A^~8n=mE4LATOIb){t^``H+= z(6ZrmHmbC*;qjX=ku>2KHzA2N-8W$_n}UtJj(Od%(0)oM_>58T8BMU-@JqRK8tIj8 zMsq>a-K^8y{Qq<}>vXq7>TWUUZqcUb>V*mDz6BFVBZh84T6&v~q&<?P-9XZ=k^FQJ z#-nr_7T7QA(0x(Nx~t)rq%Ugp4ZEr>_o{}!tK!ezhWt65q9w1==k~Y*?kd?CsIHX# z(u&zx+3YpvZipmtZ7@4?ZenIB$G&l#Nnnh2u3N;*gD?><-GNxrqxm#2yubXAh`W1X z8qs!^?-_|4-mN}uxBBp-e?ksFFfuf^8;ai;(k-JW(u5v^2~S3lRd*nte;%pjWuz8D za91x(ACZEm@4#ea?oVZknelP?l54{TUojAAVMex{y;@RaQh6CE;iW->8JG4!F`;;< z4~j@U-soeQVv0oUTKdXUEu6<FH~PGuBpllhwk+f5^!HtUCo_M&Y#fVoS5yWA$+)H; zW==B>=XqRPof~D@St<LRjAjZR?`J7%3SR1m3FD3NmVh@2f9QuvYg0!>7F7piCNi$d zRUR}-;3zX>G#ihV4B2?BG<+b{hL4fB7}b**c<?SbXuR5&AaMz5pABE#g-NMXbbOu0 zBiYGpW{E5bi|)b9l4M<Yf^m9AuZE}U;?j(9X<A&GCgsXKh_`1j%xD40lGv%qWx(MI zlEuQqc{x{uW?^$LOv2d%u!eME-2lrxxmwp(15l8juVX7z*~00oFoH}=Uxk=42wuBG zhZ#P%#bE`J=eAf=P^?YI&TSqmR?Y{ka4otI9<^er&t3OnCaYJ%1DKt<Lg&Y7qaUj^ z%GD}#@0wCwxXT#s(!$k7u-I&ztOK}>0JjEkW5E!V<6A>4)$DFH+4+!&r&>*mSV<~) z2+8SNbPi-P(}H0GGDmkHsF4Nn<A?D1?rI&z9*MCVFgta@A$42+J41-khBtX+3bjkH z)JSy_SHm9pT8XQbEDQXVPLIc@9#fASFFs<mv<{y=g01QMb@clp+4mX958!VfLvd_9 zJACiTe)T4{w9MsaH=BC=_hazp9MO?As9KxCZZt$_ZDP;tomac~Mu}_0h9~STXhQKR zteSg*k&GVLQ^uV+rJ>b^KO=GM!&aQrL|OT3cnY&v<-7P4X3}PLJlVpvD6gJE%s8@8 zk<6C2=-z{(ML8(aveRU;LcXG<=9rYiO83g}&C<eFjlXE6$z(e2X{Aj#uZ=DtsrX?V z%^_L%*EV{Z9h>c6(*-17`P0|*1Tm$Fg;@DFddXBGW?)e}-DFJ_r48&FRU*3ZkL`3I zP8KbJt!ZM)x^nOO%}(Z7S*BZ2F4FnDxJ8`q54ZxdhutJvAgIvG^bfo#B-SW}y>t?f z2`1~Crg&1TEHPOR5v8GnPK;)o_xUZlTi7oOf5y;lIt|y|rW472+<BXRL=GSe{fV-? zhxYTPdX4CWz%DZP@6jgloF*+t^1I3dITx^Xh`x<O1GJE|;hTf>SLCvy4ALNBDI)Cw z?IzvBFzHDoEJU8GxV0v;lz}rI((Qb{gqeeM2fqG@0&yraAJJXJk|su%%N5c)c;Ycl z2!AV4E<UFFc=lsr)xT&DO9T1O=r&TKoP9=D@I=OOFX*p_mya*#Cf??e@;1xWPVWa5 ztVC4$yxxt>8Zj{+551tX6vr!in2#FnDb;#@M67wd=2omps1+>^jOn3qxGl>1A-j@N zux68LECA~!vJ*cA>tDvyNO8lwb<%!ihsC;&=z2t}^(<kQwvMqDF$>2;Tj#T8Nwjqp qX;gk6ZLJa5g)J?{+Ca`Jmtw3#gj`UXW35gTX;zXZu*pm<689g-;Mqz5 diff --git a/substrate/frame/revive/rpc/src/client.rs b/substrate/frame/revive/rpc/src/client.rs index 901c15e9756..de97844eccb 100644 --- a/substrate/frame/revive/rpc/src/client.rs +++ b/substrate/frame/revive/rpc/src/client.rs @@ -17,7 +17,7 @@ //! The client connects to the source substrate chain //! and is used by the rpc server to query and send transactions to the substrate chain. use crate::{ - runtime::GAS_PRICE, + runtime::gas_from_fee, subxt_client::{ revive::{calls::types::EthTransact, events::ContractEmitted}, runtime_types::pallet_revive::storage::ContractInfo, @@ -771,7 +771,7 @@ impl Client { pub async fn evm_block(&self, block: Arc<SubstrateBlock>) -> Result<Block, ClientError> { let runtime_api = self.inner.api.runtime_api().at(block.hash()); let max_fee = Self::weight_to_fee(&runtime_api, self.max_block_weight()).await?; - let gas_limit = U256::from(max_fee / GAS_PRICE as u128); + let gas_limit = gas_from_fee(max_fee); let header = block.header(); let timestamp = extract_block_timestamp(&block).await.unwrap_or_default(); diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs index ccd8bb043e9..230f2f8b7ef 100644 --- a/substrate/frame/revive/rpc/src/lib.rs +++ b/substrate/frame/revive/rpc/src/lib.rs @@ -148,31 +148,12 @@ impl EthRpcServer for EthRpcServerImpl { async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult<H256> { let hash = H256(keccak_256(&transaction.0)); - - let tx = TransactionSigned::decode(&transaction.0).map_err(|err| { - log::debug!(target: LOG_TARGET, "Failed to decode transaction: {err:?}"); - EthRpcError::from(err) - })?; - - let eth_addr = tx.recover_eth_address().map_err(|err| { - log::debug!(target: LOG_TARGET, "Failed to recover eth address: {err:?}"); - EthRpcError::InvalidSignature - })?; - - let tx = GenericTransaction::from_signed(tx, Some(eth_addr)); - - // Dry run the transaction to get the weight limit and storage deposit limit - let dry_run = self.client.dry_run(tx, BlockTag::Latest.into()).await?; - - let call = subxt_client::tx().revive().eth_transact( - transaction.0, - dry_run.gas_required.into(), - dry_run.storage_deposit, - ); + let call = subxt_client::tx().revive().eth_transact(transaction.0); self.client.submit(call).await.map_err(|err| { log::debug!(target: LOG_TARGET, "submit call failed: {err:?}"); err })?; + log::debug!(target: LOG_TARGET, "send_raw_transaction hash: {hash:?}"); Ok(hash) } diff --git a/substrate/frame/revive/src/evm.rs b/substrate/frame/revive/src/evm.rs index c3495fc0559..c8c967fbe09 100644 --- a/substrate/frame/revive/src/evm.rs +++ b/substrate/frame/revive/src/evm.rs @@ -19,4 +19,6 @@ mod api; pub use api::*; +mod gas_encoder; +pub use gas_encoder::*; pub mod runtime; diff --git a/substrate/frame/revive/src/evm/api/byte.rs b/substrate/frame/revive/src/evm/api/byte.rs index df4ed1740ec..c2d64f8e5e4 100644 --- a/substrate/frame/revive/src/evm/api/byte.rs +++ b/substrate/frame/revive/src/evm/api/byte.rs @@ -116,7 +116,10 @@ macro_rules! impl_hex { impl Debug for $type { fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { - write!(f, concat!(stringify!($type), "({})"), self.0.to_hex()) + let hex_str = self.0.to_hex(); + let truncated = &hex_str[..hex_str.len().min(100)]; + let ellipsis = if hex_str.len() > 100 { "..." } else { "" }; + write!(f, concat!(stringify!($type), "({}{})"), truncated,ellipsis) } } diff --git a/substrate/frame/revive/src/evm/gas_encoder.rs b/substrate/frame/revive/src/evm/gas_encoder.rs new file mode 100644 index 00000000000..ffdf8b13c04 --- /dev/null +++ b/substrate/frame/revive/src/evm/gas_encoder.rs @@ -0,0 +1,174 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//! Encodes/Decodes EVM gas values. + +use crate::Weight; +use core::ops::{Div, Rem}; +use frame_support::pallet_prelude::CheckedShl; +use sp_arithmetic::traits::{One, Zero}; +use sp_core::U256; + +// We use 3 digits to store each component. +const SCALE: u128 = 100; + +/// Rounds up the given value to the nearest multiple of the mask. +/// +/// # Panics +/// Panics if the `mask` is zero. +fn round_up<T>(value: T, mask: T) -> T +where + T: One + Zero + Copy + Rem<Output = T> + Div<Output = T>, + <T as Rem>::Output: PartialEq, +{ + let rest = if value % mask == T::zero() { T::zero() } else { T::one() }; + value / mask + rest +} + +/// Rounds up the log2 of the given value to the nearest integer. +fn log2_round_up<T>(val: T) -> u128 +where + T: Into<u128>, +{ + let val = val.into(); + val.checked_ilog2() + .map(|v| if 1u128 << v == val { v } else { v + 1 }) + .unwrap_or(0) as u128 +} + +mod private { + pub trait Sealed {} + impl Sealed for () {} +} + +/// Encodes/Decodes EVM gas values. +/// +/// # Note +/// +/// This is defined as a trait rather than standalone functions to allow +/// it to be added as an associated type to [`crate::Config`]. This way, +/// it can be invoked without requiring the implementation bounds to be +/// explicitly specified. +/// +/// This trait is sealed and cannot be implemented by downstream crates. +pub trait GasEncoder<Balance>: private::Sealed { + /// Encodes all components (deposit limit, weight reference time, and proof size) into a single + /// gas value. + fn encode(gas_limit: U256, weight: Weight, deposit: Balance) -> U256; + + /// Decodes the weight and deposit from the encoded gas value. + /// Returns `None` if the gas value is invalid + fn decode(gas: U256) -> Option<(Weight, Balance)>; +} + +impl<Balance> GasEncoder<Balance> for () +where + Balance: Zero + One + CheckedShl + Into<u128>, +{ + /// The encoding follows the pattern `g...grrppdd`, where: + /// - `dd`: log2 Deposit value, encoded in the lowest 2 digits. + /// - `pp`: log2 Proof size, encoded in the next 2 digits. + /// - `rr`: log2 Reference time, encoded in the next 2 digits. + /// - `g...g`: Gas limit, encoded in the highest digits. + /// + /// # Note + /// - The deposit value is maxed by 2^99 + fn encode(gas_limit: U256, weight: Weight, deposit: Balance) -> U256 { + let deposit: u128 = deposit.into(); + let deposit_component = log2_round_up(deposit); + + let proof_size = weight.proof_size(); + let proof_size_component = SCALE * log2_round_up(proof_size); + + let ref_time = weight.ref_time(); + let ref_time_component = SCALE.pow(2) * log2_round_up(ref_time); + + let components = U256::from(deposit_component + proof_size_component + ref_time_component); + + let raw_gas_mask = U256::from(SCALE).pow(3.into()); + let raw_gas_component = if gas_limit < raw_gas_mask.saturating_add(components) { + raw_gas_mask + } else { + round_up(gas_limit, raw_gas_mask).saturating_mul(raw_gas_mask) + }; + + components.saturating_add(raw_gas_component) + } + + fn decode(gas: U256) -> Option<(Weight, Balance)> { + let deposit = gas % SCALE; + + // Casting with as_u32 is safe since all values are maxed by `SCALE`. + let deposit = deposit.as_u32(); + let proof_time = ((gas / SCALE) % SCALE).as_u32(); + let ref_time = ((gas / SCALE.pow(2)) % SCALE).as_u32(); + + let weight = Weight::from_parts( + if ref_time == 0 { 0 } else { 1u64.checked_shl(ref_time)? }, + if proof_time == 0 { 0 } else { 1u64.checked_shl(proof_time)? }, + ); + let deposit = + if deposit == 0 { Balance::zero() } else { Balance::one().checked_shl(deposit)? }; + + Some((weight, deposit)) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_gas_encoding_decoding_works() { + let raw_gas_limit = 111_111_999_999_999u128; + let weight = Weight::from_parts(222_999_999, 333_999_999); + let deposit = 444_999_999u64; + + let encoded_gas = <() as GasEncoder<u64>>::encode(raw_gas_limit.into(), weight, deposit); + assert_eq!(encoded_gas, U256::from(111_112_000_282_929u128)); + assert!(encoded_gas > raw_gas_limit.into()); + + let (decoded_weight, decoded_deposit) = + <() as GasEncoder<u64>>::decode(encoded_gas).unwrap(); + assert!(decoded_weight.all_gte(weight)); + assert!(weight.mul(2).all_gte(weight)); + + assert!(decoded_deposit >= deposit); + assert!(deposit * 2 >= decoded_deposit); + } + + #[test] + fn test_encoding_zero_values_work() { + let encoded_gas = <() as GasEncoder<u64>>::encode( + Default::default(), + Default::default(), + Default::default(), + ); + + assert_eq!(encoded_gas, U256::from(1_00_00_00)); + + let (decoded_weight, decoded_deposit) = + <() as GasEncoder<u64>>::decode(encoded_gas).unwrap(); + assert_eq!(Weight::default(), decoded_weight); + assert_eq!(0u64, decoded_deposit); + } + + #[test] + fn test_overflow() { + assert_eq!(None, <() as GasEncoder<u64>>::decode(65_00u128.into()), "Invalid proof size"); + assert_eq!(None, <() as GasEncoder<u64>>::decode(65_00_00u128.into()), "Invalid ref_time"); + } +} diff --git a/substrate/frame/revive/src/evm/runtime.rs b/substrate/frame/revive/src/evm/runtime.rs index 24b75de8356..d4b344e20eb 100644 --- a/substrate/frame/revive/src/evm/runtime.rs +++ b/substrate/frame/revive/src/evm/runtime.rs @@ -16,9 +16,13 @@ // limitations under the License. //! Runtime types for integrating `pallet-revive` with the EVM. use crate::{ - evm::api::{GenericTransaction, TransactionSigned}, - AccountIdOf, AddressMapper, BalanceOf, MomentOf, Weight, LOG_TARGET, + evm::{ + api::{GenericTransaction, TransactionSigned}, + GasEncoder, + }, + AccountIdOf, AddressMapper, BalanceOf, Config, MomentOf, LOG_TARGET, }; +use alloc::vec::Vec; use codec::{Decode, Encode}; use frame_support::{ dispatch::{DispatchInfo, GetDispatchInfo}, @@ -26,20 +30,17 @@ use frame_support::{ }; use pallet_transaction_payment::OnChargeTransaction; use scale_info::{StaticTypeInfo, TypeInfo}; -use sp_arithmetic::Percent; use sp_core::{Get, H256, U256}; use sp_runtime::{ generic::{self, CheckedExtrinsic, ExtrinsicFormat}, traits::{ - self, Checkable, Dispatchable, ExtrinsicLike, ExtrinsicMetadata, IdentifyAccount, Member, - TransactionExtension, + self, AtLeast32BitUnsigned, Checkable, Dispatchable, ExtrinsicLike, ExtrinsicMetadata, + IdentifyAccount, Member, TransactionExtension, }, transaction_validity::{InvalidTransaction, TransactionValidityError}, OpaqueExtrinsic, RuntimeDebug, Saturating, }; -use alloc::vec::Vec; - type CallOf<T> = <T as frame_system::Config>::RuntimeCall; /// The EVM gas price. @@ -48,7 +49,28 @@ type CallOf<T> = <T as frame_system::Config>::RuntimeCall; /// We use a fixed value for the gas price. /// This let us calculate the gas estimate for a transaction with the formula: /// `estimate_gas = substrate_fee / gas_price`. -pub const GAS_PRICE: u32 = 1u32; +/// +/// The chosen constant value is: +/// - Not too high, ensuring the gas value is large enough (at least 7 digits) to encode the +/// ref_time, proof_size, and deposit into the less significant (6 lower) digits of the gas value. +/// - Not too low, enabling users to adjust the gas price to define a tip. +pub const GAS_PRICE: u32 = 1_000u32; + +/// Convert a `Balance` into a gas value, using the fixed `GAS_PRICE`. +/// The gas is calculated as `balance / GAS_PRICE`, rounded up to the nearest integer. +pub fn gas_from_fee<Balance>(fee: Balance) -> U256 +where + u32: Into<Balance>, + Balance: Into<U256> + AtLeast32BitUnsigned + Copy, +{ + let gas_price = GAS_PRICE.into(); + let remainder = fee % gas_price; + if remainder.is_zero() { + (fee / gas_price).into() + } else { + (fee.saturating_add(gas_price) / gas_price).into() + } +} /// Wraps [`generic::UncheckedExtrinsic`] to support checking unsigned /// [`crate::Call::eth_transact`] extrinsic. @@ -140,15 +162,8 @@ where fn check(self, lookup: &Lookup) -> Result<Self::Checked, TransactionValidityError> { if !self.0.is_signed() { if let Ok(call) = self.0.function.clone().try_into() { - if let crate::Call::eth_transact { payload, gas_limit, storage_deposit_limit } = - call - { - let checked = E::try_into_checked_extrinsic( - payload, - gas_limit, - storage_deposit_limit, - self.encoded_size(), - )?; + if let crate::Call::eth_transact { payload } = call { + let checked = E::try_into_checked_extrinsic(payload, self.encoded_size())?; return Ok(checked) }; } @@ -251,7 +266,7 @@ where /// EthExtra convert an unsigned [`crate::Call::eth_transact`] into a [`CheckedExtrinsic`]. pub trait EthExtra { /// The Runtime configuration. - type Config: crate::Config + pallet_transaction_payment::Config; + type Config: Config + pallet_transaction_payment::Config; /// The Runtime's transaction extension. /// It should include at least: @@ -281,8 +296,6 @@ pub trait EthExtra { /// - `encoded_len`: The encoded length of the extrinsic. fn try_into_checked_extrinsic( payload: Vec<u8>, - gas_limit: Weight, - storage_deposit_limit: BalanceOf<Self::Config>, encoded_len: usize, ) -> Result< CheckedExtrinsic<AccountIdOf<Self::Config>, CallOf<Self::Config>, Self::Extension>, @@ -307,12 +320,16 @@ pub trait EthExtra { InvalidTransaction::BadProof })?; - let signer = - <Self::Config as crate::Config>::AddressMapper::to_fallback_account_id(&signer); + let signer = <Self::Config as Config>::AddressMapper::to_fallback_account_id(&signer); let GenericTransaction { nonce, chain_id, to, value, input, gas, gas_price, .. } = GenericTransaction::from_signed(tx, None); - if chain_id.unwrap_or_default() != <Self::Config as crate::Config>::ChainId::get().into() { + let Some(gas) = gas else { + log::debug!(target: LOG_TARGET, "No gas provided"); + return Err(InvalidTransaction::Call); + }; + + if chain_id.unwrap_or_default() != <Self::Config as Config>::ChainId::get().into() { log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}"); return Err(InvalidTransaction::Call); } @@ -324,6 +341,13 @@ pub trait EthExtra { })?; let data = input.unwrap_or_default().0; + + let (gas_limit, storage_deposit_limit) = + <Self::Config as Config>::EthGasEncoder::decode(gas).ok_or_else(|| { + log::debug!(target: LOG_TARGET, "Failed to decode gas: {gas:?}"); + InvalidTransaction::Call + })?; + let call = if let Some(dest) = to { crate::Call::call::<Self::Config> { dest, @@ -359,13 +383,13 @@ pub trait EthExtra { // Fees calculated with the fixed `GAS_PRICE` // When we dry-run the transaction, we set the gas to `Fee / GAS_PRICE` let eth_fee_no_tip = U256::from(GAS_PRICE) - .saturating_mul(gas.unwrap_or_default()) + .saturating_mul(gas) .try_into() .map_err(|_| InvalidTransaction::Call)?; // Fees with the actual gas_price from the transaction. let eth_fee: BalanceOf<Self::Config> = U256::from(gas_price.unwrap_or_default()) - .saturating_mul(gas.unwrap_or_default()) + .saturating_mul(gas) .try_into() .map_err(|_| InvalidTransaction::Call)?; @@ -380,27 +404,17 @@ pub trait EthExtra { Default::default(), ) .into(); - log::trace!(target: LOG_TARGET, "try_into_checked_extrinsic: encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}"); + log::debug!(target: LOG_TARGET, "try_into_checked_extrinsic: gas_price: {gas_price:?}, encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}"); // The fees from the Ethereum transaction should be greater or equal to the actual fees paid // by the account. if eth_fee < actual_fee { - log::debug!(target: LOG_TARGET, "fees {eth_fee:?} too low for the extrinsic {actual_fee:?}"); + log::debug!(target: LOG_TARGET, "eth fees {eth_fee:?} too low, actual fees: {actual_fee:?}"); return Err(InvalidTransaction::Payment.into()) } - let min = actual_fee.min(eth_fee_no_tip); - let max = actual_fee.max(eth_fee_no_tip); - let diff = Percent::from_rational(max - min, min); - if diff > Percent::from_percent(10) { - log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?} should be no more than 10% got {diff:?}"); - return Err(InvalidTransaction::Call.into()) - } else { - log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?}: {diff:?}"); - } - let tip = eth_fee.saturating_sub(eth_fee_no_tip); - log::debug!(target: LOG_TARGET, "Created checked Ethereum transaction with nonce {nonce:?} and tip: {tip:?}"); + log::debug!(target: LOG_TARGET, "Created checked Ethereum transaction with nonce: {nonce:?} and tip: {tip:?}"); Ok(CheckedExtrinsic { format: ExtrinsicFormat::Signed(signer.into(), Self::get_eth_extension(nonce, tip)), function, @@ -415,6 +429,7 @@ mod test { evm::*, test_utils::*, tests::{ExtBuilder, RuntimeCall, RuntimeOrigin, Test}, + Weight, }; use frame_support::{error::LookupError, traits::fungible::Mutate}; use pallet_revive_fixtures::compile_module; @@ -456,8 +471,6 @@ mod test { #[derive(Clone)] struct UncheckedExtrinsicBuilder { tx: GenericTransaction, - gas_limit: Weight, - storage_deposit_limit: BalanceOf<Test>, before_validate: Option<std::sync::Arc<dyn Fn() + Send + Sync>>, } @@ -467,12 +480,10 @@ mod test { Self { tx: GenericTransaction { from: Some(Account::default().address()), - chain_id: Some(<Test as crate::Config>::ChainId::get().into()), + chain_id: Some(<Test as Config>::ChainId::get().into()), gas_price: Some(U256::from(GAS_PRICE)), ..Default::default() }, - gas_limit: Weight::zero(), - storage_deposit_limit: 0, before_validate: None, } } @@ -500,7 +511,6 @@ mod test { fn call_with(dest: H160) -> Self { let mut builder = Self::new(); builder.tx.to = Some(dest); - ExtBuilder::default().build().execute_with(|| builder.estimate_gas()); builder } @@ -508,45 +518,42 @@ mod test { fn instantiate_with(code: Vec<u8>, data: Vec<u8>) -> Self { let mut builder = Self::new(); builder.tx.input = Some(Bytes(code.into_iter().chain(data.into_iter()).collect())); - ExtBuilder::default().build().execute_with(|| builder.estimate_gas()); builder } - /// Update the transaction with the given function. - fn update(mut self, f: impl FnOnce(&mut GenericTransaction) -> ()) -> Self { - f(&mut self.tx); - self - } /// Set before_validate function. fn before_validate(mut self, f: impl Fn() + Send + Sync + 'static) -> Self { self.before_validate = Some(std::sync::Arc::new(f)); self } + fn check( + self, + ) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> { + self.mutate_estimate_and_check(Box::new(|_| ())) + } + /// Call `check` on the unchecked extrinsic, and `pre_dispatch` on the signed extension. - fn check(&self) -> Result<(RuntimeCall, SignedExtra), TransactionValidityError> { + fn mutate_estimate_and_check( + mut self, + f: Box<dyn FnOnce(&mut GenericTransaction) -> ()>, + ) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> { + ExtBuilder::default().build().execute_with(|| self.estimate_gas()); + f(&mut self.tx); ExtBuilder::default().build().execute_with(|| { - let UncheckedExtrinsicBuilder { - tx, - gas_limit, - storage_deposit_limit, - before_validate, - } = self.clone(); + let UncheckedExtrinsicBuilder { tx, before_validate, .. } = self.clone(); // Fund the account. let account = Account::default(); - let _ = <Test as crate::Config>::Currency::set_balance( + let _ = <Test as Config>::Currency::set_balance( &account.substrate_account(), 100_000_000_000_000, ); - let payload = - account.sign_transaction(tx.try_into_unsigned().unwrap()).signed_payload(); - let call = RuntimeCall::Contracts(crate::Call::eth_transact { - payload, - gas_limit, - storage_deposit_limit, - }); + let payload = account + .sign_transaction(tx.clone().try_into_unsigned().unwrap()) + .signed_payload(); + let call = RuntimeCall::Contracts(crate::Call::eth_transact { payload }); let encoded_len = call.encoded_size(); let uxt: Ex = generic::UncheckedExtrinsic::new_bare(call).into(); @@ -565,7 +572,7 @@ mod test { 0, )?; - Ok((result.function, extra)) + Ok((result.function, extra, tx)) }) } } @@ -573,14 +580,18 @@ mod test { #[test] fn check_eth_transact_call_works() { let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])); + let (call, _, tx) = builder.check().unwrap(); + let (gas_limit, storage_deposit_limit) = + <<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap(); + assert_eq!( - builder.check().unwrap().0, + call, crate::Call::call::<Test> { - dest: builder.tx.to.unwrap(), - value: builder.tx.value.unwrap_or_default().as_u64(), - gas_limit: builder.gas_limit, - storage_deposit_limit: builder.storage_deposit_limit, - data: builder.tx.input.unwrap_or_default().0 + dest: tx.to.unwrap(), + value: tx.value.unwrap_or_default().as_u64(), + data: tx.input.unwrap_or_default().0, + gas_limit, + storage_deposit_limit } .into() ); @@ -591,16 +602,19 @@ mod test { let (code, _) = compile_module("dummy").unwrap(); let data = vec![]; let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone()); + let (call, _, tx) = builder.check().unwrap(); + let (gas_limit, storage_deposit_limit) = + <<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap(); assert_eq!( - builder.check().unwrap().0, + call, crate::Call::instantiate_with_code::<Test> { - value: builder.tx.value.unwrap_or_default().as_u64(), - gas_limit: builder.gas_limit, - storage_deposit_limit: builder.storage_deposit_limit, + value: tx.value.unwrap_or_default().as_u64(), code, data, - salt: None + salt: None, + gas_limit, + storage_deposit_limit } .into() ); @@ -608,11 +622,10 @@ mod test { #[test] fn check_eth_transact_nonce_works() { - let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])) - .update(|tx| tx.nonce = Some(1u32.into())); + let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])); assert_eq!( - builder.check(), + builder.mutate_estimate_and_check(Box::new(|tx| tx.nonce = Some(1u32.into()))), Err(TransactionValidityError::Invalid(InvalidTransaction::Future)) ); @@ -629,11 +642,10 @@ mod test { #[test] fn check_eth_transact_chain_id_works() { - let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])) - .update(|tx| tx.chain_id = Some(42.into())); + let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])); assert_eq!( - builder.check(), + builder.mutate_estimate_and_check(Box::new(|tx| tx.chain_id = Some(42.into()))), Err(TransactionValidityError::Invalid(InvalidTransaction::Call)) ); } @@ -646,14 +658,14 @@ mod test { // Fail because the tx input fail to get the blob length assert_eq!( - builder.clone().update(|tx| tx.input = Some(Bytes(vec![1, 2, 3]))).check(), + builder.mutate_estimate_and_check(Box::new(|tx| tx.input = Some(Bytes(vec![1, 2, 3])))), Err(TransactionValidityError::Invalid(InvalidTransaction::Call)) ); } #[test] fn check_transaction_fees() { - let scenarios: [(_, Box<dyn FnOnce(&mut GenericTransaction)>, _); 5] = [ + let scenarios: Vec<(_, Box<dyn FnOnce(&mut GenericTransaction)>, _)> = vec![ ( "Eth fees too low", Box::new(|tx| { @@ -661,42 +673,20 @@ mod test { }), InvalidTransaction::Payment, ), - ( - "Gas fees too high", - Box::new(|tx| { - tx.gas = Some(tx.gas.unwrap() * 2); - }), - InvalidTransaction::Call, - ), ( "Gas fees too low", Box::new(|tx| { - tx.gas = Some(tx.gas.unwrap() * 2); - }), - InvalidTransaction::Call, - ), - ( - "Diff > 10%", - Box::new(|tx| { - tx.gas = Some(tx.gas.unwrap() * 111 / 100); + tx.gas = Some(tx.gas.unwrap() / 2); }), - InvalidTransaction::Call, - ), - ( - "Diff < 10%", - Box::new(|tx| { - tx.gas_price = Some(tx.gas_price.unwrap() * 2); - tx.gas = Some(tx.gas.unwrap() * 89 / 100); - }), - InvalidTransaction::Call, + InvalidTransaction::Payment, ), ]; for (msg, update_tx, err) in scenarios { - let builder = - UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])).update(update_tx); + let res = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])) + .mutate_estimate_and_check(update_tx); - assert_eq!(builder.check(), Err(TransactionValidityError::Invalid(err)), "{}", msg); + assert_eq!(res, Err(TransactionValidityError::Invalid(err)), "{}", msg); } } @@ -704,16 +694,16 @@ mod test { fn check_transaction_tip() { let (code, _) = compile_module("dummy").unwrap(); let data = vec![]; - let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone()) - .update(|tx| { - tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100); - log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price); - }); + let (_, extra, tx) = + UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone()) + .mutate_estimate_and_check(Box::new(|tx| { + tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100); + log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price); + })) + .unwrap(); - let tx = &builder.tx; let expected_tip = tx.gas_price.unwrap() * tx.gas.unwrap() - U256::from(GAS_PRICE) * tx.gas.unwrap(); - let (_, extra) = builder.check().unwrap(); assert_eq!(U256::from(extra.1.tip()), expected_tip); } } diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index b9a39e7ce4d..04bce264a18 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -41,7 +41,10 @@ pub mod test_utils; pub mod weights; use crate::{ - evm::{runtime::GAS_PRICE, GenericTransaction}, + evm::{ + runtime::{gas_from_fee, GAS_PRICE}, + GasEncoder, GenericTransaction, + }, exec::{AccountIdOf, ExecError, Executable, Ext, Key, Origin, Stack as ExecStack}, gas::GasMeter, storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager}, @@ -295,6 +298,11 @@ pub mod pallet { /// The ratio between the decimal representation of the native token and the ETH token. #[pallet::constant] type NativeToEthRatio: Get<u32>; + + /// Encode and decode Ethereum gas values. + /// Only valid value is `()`. See [`GasEncoder`]. + #[pallet::no_default_bounds] + type EthGasEncoder: GasEncoder<BalanceOf<Self>>; } /// Container for different types that implement [`DefaultConfig`]` of this pallet. @@ -368,6 +376,7 @@ pub mod pallet { type PVFMemory = ConstU32<{ 512 * 1024 * 1024 }>; type ChainId = ConstU64<0>; type NativeToEthRatio = ConstU32<1>; + type EthGasEncoder = (); } } @@ -560,6 +569,8 @@ pub mod pallet { AccountUnmapped, /// Tried to map an account that is already mapped. AccountAlreadyMapped, + /// The transaction used to dry-run a contract is invalid. + InvalidGenericTransaction, } /// A reason for the pallet contracts placing a hold on funds. @@ -761,12 +772,7 @@ pub mod pallet { #[allow(unused_variables)] #[pallet::call_index(0)] #[pallet::weight(Weight::MAX)] - pub fn eth_transact( - origin: OriginFor<T>, - payload: Vec<u8>, - gas_limit: Weight, - #[pallet::compact] storage_deposit_limit: BalanceOf<T>, - ) -> DispatchResultWithPostInfo { + pub fn eth_transact(origin: OriginFor<T>, payload: Vec<u8>) -> DispatchResultWithPostInfo { Err(frame_system::Error::CallFiltered::<T>.into()) } @@ -1406,11 +1412,8 @@ where return Err(EthTransactError::Message("Invalid transaction".into())); }; - let eth_dispatch_call = crate::Call::<T>::eth_transact { - payload: unsigned_tx.dummy_signed_payload(), - gas_limit: result.gas_required, - storage_deposit_limit: result.storage_deposit, - }; + let eth_dispatch_call = + crate::Call::<T>::eth_transact { payload: unsigned_tx.dummy_signed_payload() }; let encoded_len = utx_encoded_size(eth_dispatch_call); let fee = pallet_transaction_payment::Pallet::<T>::compute_fee( encoded_len, @@ -1418,7 +1421,9 @@ where 0u32.into(), ) .into(); - let eth_gas: U256 = (fee / GAS_PRICE.into()).into(); + let eth_gas = gas_from_fee(fee); + let eth_gas = + T::EthGasEncoder::encode(eth_gas, result.gas_required, result.storage_deposit); if eth_gas == result.eth_gas { log::trace!(target: LOG_TARGET, "bare_eth_call: encoded_len: {encoded_len:?} eth_gas: {eth_gas:?}"); -- GitLab