From c5152205eef7f6bbec14798b0464a99a358abf9d Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Thu, 11 Nov 2021 11:36:18 +0100 Subject: [PATCH 01/20] Remove subkey from evm-evm e2e tests Dockerfile --- Dockerfile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index fcc29486..3d7de192 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,10 +8,8 @@ RUN cd /src && echo $(ls -1 /src) RUN go mod download RUN go build -o /bridge ./e2e/evm-evm/example/. -# # final stage +# final stage FROM debian:stretch-slim -RUN apt-get -y update && apt-get -y upgrade && apt-get install ca-certificates wget -y - COPY --from=builder /bridge ./ RUN chmod +x ./bridge From d966b89778c3a05f6c4284859f1acd368b97392a Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Thu, 11 Nov 2021 11:36:36 +0100 Subject: [PATCH 02/20] Fix typo --- e2e/evm-evm/eve-eve_test.go | 5 +---- e2e/evm/test.go | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/e2e/evm-evm/eve-eve_test.go b/e2e/evm-evm/eve-eve_test.go index a1255486..b6ed4911 100644 --- a/e2e/evm-evm/eve-eve_test.go +++ b/e2e/evm-evm/eve-eve_test.go @@ -9,10 +9,7 @@ import ( "github.com/stretchr/testify/suite" ) -const ETHEndpoint1 = "http://localhost:8545" -const ETHEndpoint2 = "http://localhost:8547" - // Alice key is used by the relayer, Eve key is used as admin and depositter func TestRunE2ETests(t *testing.T) { - suite.Run(t, evm.SetupEVM2EVEMTestSuite(evmtransaction.NewTransaction, evmtransaction.NewTransaction, ETHEndpoint1, ETHEndpoint2, local.EveKp)) + suite.Run(t, evm.SetupEVM2EVMTestSuite(evmtransaction.NewTransaction, evmtransaction.NewTransaction, ETHEndpoint1, ETHEndpoint2, local.EveKp)) } diff --git a/e2e/evm/test.go b/e2e/evm/test.go index a39c22d8..2e822290 100644 --- a/e2e/evm/test.go +++ b/e2e/evm/test.go @@ -25,7 +25,7 @@ type TestClient interface { FetchEventLogs(ctx context.Context, contractAddress common.Address, event string, startBlock *big.Int, endBlock *big.Int) ([]types.Log, error) } -func SetupEVM2EVEMTestSuite(fabric1, fabric2 calls.TxFabric, endpoint1, endpoint2 string, adminKey *secp256k1.Keypair) *IntegrationTestSuite { +func SetupEVM2EVMTestSuite(fabric1, fabric2 calls.TxFabric, endpoint1, endpoint2 string, adminKey *secp256k1.Keypair) *IntegrationTestSuite { return &IntegrationTestSuite{ fabric1: fabric1, fabric2: fabric2, From 4359ad25c54bb0a7a04629e043a684253452786a Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Fri, 12 Nov 2021 12:53:40 +0100 Subject: [PATCH 03/20] Fix evm-evm test name --- e2e/evm-evm/{eve-eve_test.go => evm-evm_test.go} | 3 +++ 1 file changed, 3 insertions(+) rename e2e/evm-evm/{eve-eve_test.go => evm-evm_test.go} (85%) diff --git a/e2e/evm-evm/eve-eve_test.go b/e2e/evm-evm/evm-evm_test.go similarity index 85% rename from e2e/evm-evm/eve-eve_test.go rename to e2e/evm-evm/evm-evm_test.go index b6ed4911..045f64a2 100644 --- a/e2e/evm-evm/eve-eve_test.go +++ b/e2e/evm-evm/evm-evm_test.go @@ -9,6 +9,9 @@ import ( "github.com/stretchr/testify/suite" ) +const ETHEndpoint1 = "ws://localhost:8546" +const ETHEndpoint2 = "ws://localhost:8548" + // Alice key is used by the relayer, Eve key is used as admin and depositter func TestRunE2ETests(t *testing.T) { suite.Run(t, evm.SetupEVM2EVMTestSuite(evmtransaction.NewTransaction, evmtransaction.NewTransaction, ETHEndpoint1, ETHEndpoint2, local.EveKp)) From a96e150019da22580c6167a4631b71b93db4b009 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Fri, 12 Nov 2021 12:53:54 +0100 Subject: [PATCH 04/20] Switch e2e tests to run with ws --- e2e/evm-evm/example/cfg/config_evm1.json | 2 +- e2e/evm-evm/example/cfg/config_evm2.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/evm-evm/example/cfg/config_evm1.json b/e2e/evm-evm/example/cfg/config_evm1.json index f26d0314..d9563822 100644 --- a/e2e/evm-evm/example/cfg/config_evm1.json +++ b/e2e/evm-evm/example/cfg/config_evm1.json @@ -1,7 +1,7 @@ { "name": "evm1", "id": "1", - "endpoint": "http://evm1-1:8545", + "endpoint": "ws://evm1-1:8546", "from": "0xff93B45308FD417dF303D6515aB04D9e89a750Ca", "bridge": "0xd606A00c1A39dA53EA7Bb3Ab570BBE40b156EB66", "erc20Handler": "0x05C5AFACf64A6082D4933752FfB447AED63581b1", diff --git a/e2e/evm-evm/example/cfg/config_evm2.json b/e2e/evm-evm/example/cfg/config_evm2.json index 30682cb3..58693124 100644 --- a/e2e/evm-evm/example/cfg/config_evm2.json +++ b/e2e/evm-evm/example/cfg/config_evm2.json @@ -1,7 +1,7 @@ { "name": "evm2", "id": "2", - "endpoint": "http://evm2-1:8545", + "endpoint": "ws://evm2-1:8546", "from": "0xff93B45308FD417dF303D6515aB04D9e89a750Ca", "bridge": "0xd606A00c1A39dA53EA7Bb3Ab570BBE40b156EB66", "erc20Handler": "0x05C5AFACf64A6082D4933752FfB447AED63581b1", From d4844f2c8ae63ca34d3cbd23f942d98e20629c75 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Fri, 12 Nov 2021 12:54:55 +0100 Subject: [PATCH 05/20] Bump go-ethereum --- go.mod | 6 +++++- go.sum | 20 ++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 4b4fa859..47e25ead 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( github.com/centrifuge/go-substrate-rpc-client v2.0.0+incompatible - github.com/ethereum/go-ethereum v1.10.9 + github.com/ethereum/go-ethereum v1.10.12 github.com/golang/mock v1.6.0 github.com/gorilla/mux v1.8.0 github.com/pkg/errors v0.9.1 @@ -34,8 +34,11 @@ require ( github.com/golang/snappy v0.0.4 // indirect github.com/google/uuid v1.1.5 // indirect github.com/gorilla/websocket v1.4.2 // indirect + github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d // indirect github.com/hashicorp/hcl v1.0.0 // indirect + github.com/huin/goupnp v1.0.2 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect + github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458 // indirect github.com/magiconair/properties v1.8.5 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect github.com/mitchellh/mapstructure v1.4.2 // indirect @@ -53,6 +56,7 @@ require ( github.com/subosito/gotenv v1.2.0 // indirect github.com/tklauser/go-sysconf v0.3.5 // indirect github.com/tklauser/numcpus v0.2.2 // indirect + golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect golang.org/x/text v0.3.6 // indirect google.golang.org/protobuf v1.27.1 // indirect diff --git a/go.sum b/go.sum index 7d962dcd..7e2d105b 100644 --- a/go.sum +++ b/go.sum @@ -141,6 +141,7 @@ github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/dave/jennifer v1.2.0/go.mod h1:fIb+770HOpJ2fmN9EPPKOqm1vMGhB+TwXKMZhrIygKg= github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -155,9 +156,11 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm github.com/dgryski/go-bitstream v0.0.0-20180413035011-3522498ce2c8/go.mod h1:VMaSuZ+SZcx/wljOQKvp5srsbCiKDEb6K2wC4+PiBmQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/dlclark/regexp2 v1.2.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= +github.com/dlclark/regexp2 v1.4.1-0.20201116162257-a2a8dda75c91/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc= github.com/docker/docker v1.4.2-0.20180625184442-8e610b2b55bf/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/dop251/goja v0.0.0-20200219165308-d1232e640a87/go.mod h1:Mw6PkjjMXWbTj+nnj4s3QPXq1jaT0s5pC0iFD4+BOAA= -github.com/dop251/goja v0.0.0-20200721192441-a695b0cdd498/go.mod h1:Mw6PkjjMXWbTj+nnj4s3QPXq1jaT0s5pC0iFD4+BOAA= +github.com/dop251/goja v0.0.0-20211011172007-d99e4b8cbf48/go.mod h1:R9ET47fwRVRPZnOGvHxxhuZcbrMCuiqOz3Rlrh4KSnk= +github.com/dop251/goja_nodejs v0.0.0-20210225215109-d91c329300e7/go.mod h1:hn7BA7c8pLvoGndExHudxTDKZ84Pyvv+90pbBjbTz0Y= github.com/eclipse/paho.mqtt.golang v1.2.0/go.mod h1:H9keYFcgq3Qr5OUJm/JZI/i6U7joQ8SYLhZwfeOo6Ts= github.com/edsrzf/mmap-go v0.0.0-20160512033002-935e0e8a636c/go.mod h1:YO35OhQPt3KJa3ryjFM5Bs14WD66h8eGKpfaBNrHW5M= github.com/edsrzf/mmap-go v1.0.0 h1:CEBF7HpRnUCSJgGUb5h1Gm7e3VkmVDrR8lvWVLtrOFw= @@ -172,8 +175,8 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.m github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/ethereum/go-ethereum v1.9.12/go.mod h1:PvsVkQmhZFx92Y+h2ylythYlheEDt/uBgFbl61Js/jo= -github.com/ethereum/go-ethereum v1.10.9 h1:uMSWt0qDhaqqCk0PWqfDFOMUExmk4Tnbma6c6oXW+Pk= -github.com/ethereum/go-ethereum v1.10.9/go.mod h1:CaTMQrv51WaAlD2eULQ3f03KiahDRO28fleQcKjWrrg= +github.com/ethereum/go-ethereum v1.10.12 h1:el/KddB3gLEsnNgGQ3SQuZuiZjwnFTYHe5TwUet5Om4= +github.com/ethereum/go-ethereum v1.10.12/go.mod h1:W3yfrFyL9C1pHcwY5hmRHVDaorTiQxhYBkKyu5mEDHw= github.com/fatih/color v1.3.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= @@ -207,6 +210,7 @@ github.com/go-ole/go-ole v1.2.1/go.mod h1:7FAglXiTm7HKlQRDeOQ6ZNUHidzCWXuZWq/1dT github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk= github.com/go-sourcemap/sourcemap v2.1.2+incompatible/go.mod h1:F8jJfvm2KbVjc5NqelyYJmf/v5J0dwNLS2mL4sNA1Jg= +github.com/go-sourcemap/sourcemap v2.1.3+incompatible/go.mod h1:F8jJfvm2KbVjc5NqelyYJmf/v5J0dwNLS2mL4sNA1Jg= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-stack/stack v1.8.0 h1:5SgMzNM5HxrEjV0ww2lTmX6E2Izsfxas4+YHWRs3Lsk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= @@ -389,6 +393,7 @@ github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8 github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/jwilder/encoding v0.0.0-20170811194829-b4e1701a28ef/go.mod h1:Ct9fl0F6iIOGgxJ5npU/IUOhOhqlVrGjyIZc8/MagT0= github.com/karalabe/usb v0.0.0-20190919080040-51dc0efba356/go.mod h1:Od972xHfMJowv7NGVDiWVxk2zxnWgjLlJzE+F4F7AGU= +github.com/karalabe/usb v0.0.0-20211005121534-4c5740d64559/go.mod h1:Od972xHfMJowv7NGVDiWVxk2zxnWgjLlJzE+F4F7AGU= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= @@ -402,11 +407,13 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxv github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/labstack/echo/v4 v4.2.1/go.mod h1:AA49e0DZ8kk5jTOOCKNuPR6oTnBS0dYiM4FW1e6jwpg= github.com/labstack/gommon v0.3.0/go.mod h1:MULnywXg0yavhxWKc+lOruYdAhDwPK9wf0OL7NoOu+k= @@ -1069,8 +1076,9 @@ google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQ gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= From 5db7a5860febedbe5f31ab447c0961f4d1291b41 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 11:24:59 +0100 Subject: [PATCH 06/20] Refactor proposal status to include yes votes count --- chains/evm/calls/bridge.go | 19 ++++++------------- chains/evm/calls/bridge_test.go | 7 ++++--- relayer/message.go | 11 ++++++++--- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/chains/evm/calls/bridge.go b/chains/evm/calls/bridge.go index 06596baf..1c55429f 100644 --- a/chains/evm/calls/bridge.go +++ b/chains/evm/calls/bridge.go @@ -282,32 +282,25 @@ func GetThreshold(evmCaller ContractCallerClient, bridgeAddress *common.Address) func ProposalStatus(evmCaller ContractCallerClient, p *proposal.Proposal) (relayer.ProposalStatus, error) { a, err := abi.JSON(strings.NewReader(consts.BridgeABI)) if err != nil { - return relayer.ProposalStatusInactive, err + return relayer.ProposalStatus{}, err } input, err := a.Pack("getProposal", p.Source, p.DepositNonce, SliceTo32Bytes(p.Data)) if err != nil { - return relayer.ProposalStatusInactive, err + return relayer.ProposalStatus{}, err } msg := ethereum.CallMsg{From: common.Address{}, To: &p.BridgeAddress, Data: input} out, err := evmCaller.CallContract(context.TODO(), ToCallArg(msg), nil) if err != nil { - return relayer.ProposalStatusInactive, err - } - - type bridgeProposal struct { - Status uint8 - YesVotes *big.Int - YesVotesTotal uint8 - ProposedBlock *big.Int + return relayer.ProposalStatus{}, err } res, err := a.Unpack("getProposal", out) if err != nil { - return relayer.ProposalStatusInactive, err + return relayer.ProposalStatus{}, err } - out0 := *abi.ConvertType(res[0], new(bridgeProposal)).(*bridgeProposal) - return relayer.ProposalStatus(out0.Status), nil + ps := *abi.ConvertType(res[0], new(relayer.ProposalStatus)).(*relayer.ProposalStatus) + return ps, nil } func idAndNonce(srcId uint8, nonce uint64) *big.Int { diff --git a/chains/evm/calls/bridge_test.go b/chains/evm/calls/bridge_test.go index d8586bc0..e1e94115 100644 --- a/chains/evm/calls/bridge_test.go +++ b/chains/evm/calls/bridge_test.go @@ -51,7 +51,7 @@ func (s *ProposalStatusTestSuite) TestProposalStatusFailedContractCall() { status, err := calls.ProposalStatus(s.mockContractCaller, &proposal.Proposal{}) - s.Equal(relayer.ProposalStatusInactive, status) + s.Equal(relayer.ProposalStatus{}, status) s.NotNil(err) } @@ -60,7 +60,7 @@ func (s *ProposalStatusTestSuite) TestProposalStatusFailedUnpack() { status, err := calls.ProposalStatus(s.mockContractCaller, &proposal.Proposal{}) - s.Equal(relayer.ProposalStatusInactive, status) + s.Equal(relayer.ProposalStatus{}, status) s.NotNil(err) } @@ -70,8 +70,9 @@ func (s *ProposalStatusTestSuite) TestProposalStatusSuccessfulCall() { status, err := calls.ProposalStatus(s.mockContractCaller, &proposal.Proposal{}) - s.Equal(relayer.ProposalStatusInactive, status) s.Nil(err) + s.Equal(int64(0), status.ProposedBlock.Int64()) + s.Equal(int64(0), status.YesVotes.Int64()) } func TestPrepareWithdrawInput(t *testing.T) { diff --git a/relayer/message.go b/relayer/message.go index 6148e95b..d07def48 100644 --- a/relayer/message.go +++ b/relayer/message.go @@ -18,10 +18,15 @@ const ( GenericTransfer TransferType = "GenericTransfer" ) -type ProposalStatus uint8 +type ProposalStatus struct { + Status uint8 + YesVotes *big.Int + YesVotesTotal uint8 + ProposedBlock *big.Int +} const ( - ProposalStatusInactive ProposalStatus = iota + ProposalStatusInactive uint8 = iota ProposalStatusActive ProposalStatusPassed // Ready to be executed ProposalStatusExecuted @@ -29,7 +34,7 @@ const ( ) var ( - StatusMap = map[ProposalStatus]string{ProposalStatusInactive: "inactive", ProposalStatusActive: "active", ProposalStatusPassed: "passed", ProposalStatusExecuted: "executed", ProposalStatusCanceled: "canceled"} + StatusMap = map[uint8]string{ProposalStatusInactive: "inactive", ProposalStatusActive: "active", ProposalStatusPassed: "passed", ProposalStatusExecuted: "executed", ProposalStatusCanceled: "canceled"} ) type Message struct { From 038dd722ddd7edfff8e8a7432de66296567701e6 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 11:25:48 +0100 Subject: [PATCH 07/20] Implement voter pending proposal tracking to prevent extra votes --- chains/evm/evmclient/evm-client.go | 16 +++- chains/evm/voter/voter.go | 127 ++++++++++++++++++++++++----- 2 files changed, 120 insertions(+), 23 deletions(-) diff --git a/chains/evm/evmclient/evm-client.go b/chains/evm/evmclient/evm-client.go index 30a66c38..500d0daa 100644 --- a/chains/evm/evmclient/evm-client.go +++ b/chains/evm/evmclient/evm-client.go @@ -23,16 +23,18 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethclient" + "github.com/ethereum/go-ethereum/ethclient/gethclient" "github.com/ethereum/go-ethereum/rpc" "github.com/rs/zerolog/log" ) type EVMClient struct { *ethclient.Client - rpClient *rpc.Client - config *EVMConfig - nonce *big.Int - nonceLock sync.Mutex + gethClient *gethclient.Client + rpClient *rpc.Client + config *EVMConfig + nonce *big.Int + nonceLock sync.Mutex } // DepositLogs struct holds event data with all necessary parameters and a handler response @@ -74,12 +76,17 @@ func NewEVMClientFromParams(url string, privateKey *ecdsa.PrivateKey) (*EVMClien kp := secp256k1.NewKeypair(*privateKey) c := &EVMClient{} c.Client = ethclient.NewClient(rpcClient) + c.gethClient = gethclient.New(rpcClient) c.rpClient = rpcClient c.config = &EVMConfig{} c.config.kp = kp return c, nil } +func (c *EVMClient) SubscribePendingTransactions(ctx context.Context, ch chan<- common.Hash) (*rpc.ClientSubscription, error) { + return c.gethClient.SubscribePendingTransactions(ctx, ch) +} + func (c *EVMClient) Configurate(path string, name string) error { rawCfg, err := GetConfig(path, name) if err != nil { @@ -106,6 +113,7 @@ func (c *EVMClient) Configurate(path string, name string) error { } c.Client = ethclient.NewClient(rpcClient) c.rpClient = rpcClient + c.gethClient = gethclient.New(rpcClient) if generalConfig.LatestBlock { curr, err := c.LatestBlock() diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index 503214fb..ca054385 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -7,11 +7,18 @@ import ( "context" "fmt" "math/big" + "math/rand" + "strings" + "time" "github.com/ChainSafe/chainbridge-core/chains/evm/calls" + "github.com/ChainSafe/chainbridge-core/chains/evm/calls/consts" "github.com/ChainSafe/chainbridge-core/chains/evm/voter/proposal" "github.com/ChainSafe/chainbridge-core/relayer" + "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" + ethereumTypes "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rpc" "github.com/rs/zerolog/log" ) @@ -19,7 +26,11 @@ type ChainClient interface { LatestBlock() (*big.Int, error) RelayerAddress() common.Address CallContract(ctx context.Context, callArgs map[string]interface{}, blockNumber *big.Int) ([]byte, error) + PendingCallContract(ctx context.Context, callArgs map[string]interface{}) ([]byte, error) + PendingTransactionCount(ctx context.Context) (uint, error) ChainID(ctx context.Context) (*big.Int, error) + SubscribePendingTransactions(ctx context.Context, ch chan<- common.Hash) (*rpc.ClientSubscription, error) + TransactionByHash(ctx context.Context, hash common.Hash) (tx *ethereumTypes.Transaction, isPending bool, err error) calls.ClientDispatcher } @@ -28,19 +39,27 @@ type MessageHandler interface { } type EVMVoter struct { - mh MessageHandler - client ChainClient - fabric calls.TxFabric - gasPriceClient calls.GasPricer + mh MessageHandler + client ChainClient + fabric calls.TxFabric + gasPriceClient calls.GasPricer + pendingProposalVotes map[uint64]uint8 } func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) *EVMVoter { - return &EVMVoter{ - mh: mh, - client: client, - fabric: fabric, - gasPriceClient: gasPriceClient, + voter := &EVMVoter{ + mh: mh, + client: client, + fabric: fabric, + gasPriceClient: gasPriceClient, + pendingProposalVotes: make(map[uint64]uint8), } + + ch := make(chan common.Hash) + go voter.trackProposalPendingVotes(ch) + client.SubscribePendingTransactions(context.TODO(), ch) + + return voter } func (v *EVMVoter) VoteProposal(m *relayer.Message) error { @@ -48,22 +67,92 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { if err != nil { return err } + + votedByTheRelayer, err := calls.IsProposalVotedBy(v.client, v.client.RelayerAddress(), prop) + if err != nil { + return err + } + if votedByTheRelayer { + return nil + } + + shouldVoteChn := make(chan bool) + go v.shouldVoteForProposal(shouldVoteChn, prop, time.Sleep) + + shouldVote := <-shouldVoteChn + if !shouldVote { + log.Debug().Msgf("Proposal %+v already has enough votes", prop) + return nil + } + + hash, err := calls.VoteProposal(v.client, v.fabric, v.gasPriceClient, prop) + if err != nil { + return fmt.Errorf("voting failed. Err: %w", err) + } + + log.Debug().Str("hash", hash.String()).Uint64("nonce", prop.DepositNonce).Msgf("Voted") + return nil +} + +func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, sleep func(d time.Duration)) { + sleep(time.Duration(rand.Intn(20)) * time.Millisecond * 500) + + defer delete(v.pendingProposalVotes, prop.DepositNonce) ps, err := calls.ProposalStatus(v.client, prop) if err != nil { - return fmt.Errorf("error getting proposal: %+v status %w", prop, err) + log.Error().Err(err) + shouldVote <- false + return } - votedByTheRelayer, err := calls.IsProposalVotedBy(v.client, v.client.RelayerAddress(), prop) + + if ps.Status == relayer.ProposalStatusExecuted || ps.Status == relayer.ProposalStatusCanceled { + shouldVote <- false + return + } + + threshold, err := calls.GetThreshold(v.client, &prop.BridgeAddress) if err != nil { - return err + log.Error().Err(err) + shouldVote <- false + return + } + + if ps.YesVotesTotal+v.pendingProposalVotes[prop.DepositNonce] >= threshold { + shouldVote <- false + return } - // if this relayer had not voted for proposal and proposal is in Active or Inactive status - // we need to vote for it - if !votedByTheRelayer && (ps == relayer.ProposalStatusActive || ps == relayer.ProposalStatusInactive) { - hash, err := calls.VoteProposal(v.client, v.fabric, v.gasPriceClient, prop) - log.Debug().Str("hash", hash.String()).Uint64("nonce", prop.DepositNonce).Msgf("Voted") + + shouldVote <- true +} + +func (v *EVMVoter) trackProposalPendingVotes(ch chan common.Hash) { + for msg := range ch { + txData, _, err := v.client.TransactionByHash(context.TODO(), msg) + if err != nil { + log.Error().Err(err) + continue + } + + a, err := abi.JSON(strings.NewReader(consts.BridgeABI)) + if err != nil { + log.Error().Err(err) + continue + } + + m, err := a.MethodById(txData.Data()[:4]) + if err != nil { + continue + } + + data, err := m.Inputs.UnpackValues(txData.Data()[4:]) if err != nil { - return fmt.Errorf("voting failed. Err: %w", err) + log.Error().Err(err) + continue + } + + if m.Name == "voteProposal" { + depositNonce := data[1].(uint64) + v.pendingProposalVotes[depositNonce]++ } } - return nil } From 29f5ecbce63ad78ffe51db61f1f3f73989110525 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 15:07:30 +0100 Subject: [PATCH 08/20] Fix proposal status fetching --- chains/evm/calls/bridge.go | 2 +- chains/evm/calls/bridge_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/chains/evm/calls/bridge.go b/chains/evm/calls/bridge.go index 1c55429f..c95fb0be 100644 --- a/chains/evm/calls/bridge.go +++ b/chains/evm/calls/bridge.go @@ -284,7 +284,7 @@ func ProposalStatus(evmCaller ContractCallerClient, p *proposal.Proposal) (relay if err != nil { return relayer.ProposalStatus{}, err } - input, err := a.Pack("getProposal", p.Source, p.DepositNonce, SliceTo32Bytes(p.Data)) + input, err := a.Pack("getProposal", p.Source, p.DepositNonce, p.GetDataHash()) if err != nil { return relayer.ProposalStatus{}, err } diff --git a/chains/evm/calls/bridge_test.go b/chains/evm/calls/bridge_test.go index e1e94115..467f1ea3 100644 --- a/chains/evm/calls/bridge_test.go +++ b/chains/evm/calls/bridge_test.go @@ -65,14 +65,14 @@ func (s *ProposalStatusTestSuite) TestProposalStatusFailedUnpack() { } func (s *ProposalStatusTestSuite) TestProposalStatusSuccessfulCall() { - proposalStatus, _ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") + proposalStatus, _ := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000001c0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000001f") s.mockContractCaller.EXPECT().CallContract(gomock.Any(), gomock.Any(), nil).Return(proposalStatus, nil) status, err := calls.ProposalStatus(s.mockContractCaller, &proposal.Proposal{}) s.Nil(err) - s.Equal(int64(0), status.ProposedBlock.Int64()) - s.Equal(int64(0), status.YesVotes.Int64()) + s.Equal(status.YesVotesTotal, uint8(3)) + s.Equal(status.Status, relayer.ProposalStatusExecuted) } func TestPrepareWithdrawInput(t *testing.T) { From 802c89b49e51838a677108feb4a71c422fad75cb Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 15:07:40 +0100 Subject: [PATCH 09/20] Remove annoying log --- chains/evm/evmclient/evm-client.go | 1 - 1 file changed, 1 deletion(-) diff --git a/chains/evm/evmclient/evm-client.go b/chains/evm/evmclient/evm-client.go index 500d0daa..4907d801 100644 --- a/chains/evm/evmclient/evm-client.go +++ b/chains/evm/evmclient/evm-client.go @@ -162,7 +162,6 @@ func (c *EVMClient) WaitAndReturnTxReceipt(h common.Hash) (*types.Receipt, error for retry > 0 { receipt, err := c.Client.TransactionReceipt(context.Background(), h) if err != nil { - log.Error().Err(err).Msgf("error getting tx receipt %s", h.String()) retry-- time.Sleep(5 * time.Second) continue From bc88536ed32af9e005a2b0f65c99ba27e25a8f2a Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 15:08:13 +0100 Subject: [PATCH 10/20] Increase pending proposal votes by proposal ID --- chains/evm/voter/proposal/proposal.go | 5 +++ chains/evm/voter/voter.go | 49 ++++++++++++++++++++------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/chains/evm/voter/proposal/proposal.go b/chains/evm/voter/proposal/proposal.go index e97a3513..73a7c94f 100644 --- a/chains/evm/voter/proposal/proposal.go +++ b/chains/evm/voter/proposal/proposal.go @@ -31,3 +31,8 @@ type Proposal struct { func (p *Proposal) GetDataHash() common.Hash { return crypto.Keccak256Hash(append(p.HandlerAddress.Bytes(), p.Data...)) } + +// GetID construct proposal unique identifier +func (p *Proposal) GetID() common.Hash { + return crypto.Keccak256Hash(append([]byte{p.Source}, byte(p.DepositNonce))) +} diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index ca054385..6e0492a5 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -22,13 +22,14 @@ import ( "github.com/rs/zerolog/log" ) +const ( + maxShouldVoteChecks = 40 + shouldVoteCheckPeriod = 15 +) + type ChainClient interface { - LatestBlock() (*big.Int, error) RelayerAddress() common.Address CallContract(ctx context.Context, callArgs map[string]interface{}, blockNumber *big.Int) ([]byte, error) - PendingCallContract(ctx context.Context, callArgs map[string]interface{}) ([]byte, error) - PendingTransactionCount(ctx context.Context) (uint, error) - ChainID(ctx context.Context) (*big.Int, error) SubscribePendingTransactions(ctx context.Context, ch chan<- common.Hash) (*rpc.ClientSubscription, error) TransactionByHash(ctx context.Context, hash common.Hash) (tx *ethereumTypes.Transaction, isPending bool, err error) calls.ClientDispatcher @@ -43,7 +44,7 @@ type EVMVoter struct { client ChainClient fabric calls.TxFabric gasPriceClient calls.GasPricer - pendingProposalVotes map[uint64]uint8 + pendingProposalVotes map[common.Hash]uint8 } func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) *EVMVoter { @@ -52,7 +53,7 @@ func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasP client: client, fabric: fabric, gasPriceClient: gasPriceClient, - pendingProposalVotes: make(map[uint64]uint8), + pendingProposalVotes: make(map[common.Hash]uint8), } ch := make(chan common.Hash) @@ -77,7 +78,7 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { } shouldVoteChn := make(chan bool) - go v.shouldVoteForProposal(shouldVoteChn, prop, time.Sleep) + go v.shouldVoteForProposal(shouldVoteChn, prop, time.Sleep, 0) shouldVote := <-shouldVoteChn if !shouldVote { @@ -94,11 +95,13 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { return nil } -func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, sleep func(d time.Duration)) { - sleep(time.Duration(rand.Intn(20)) * time.Millisecond * 500) +func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, sleep func(d time.Duration), tries int) { + sleep(time.Duration(rand.Intn(shouldVoteCheckPeriod)) * time.Second) + propID := prop.GetID() - defer delete(v.pendingProposalVotes, prop.DepositNonce) + defer delete(v.pendingProposalVotes, propID) ps, err := calls.ProposalStatus(v.client, prop) + if err != nil { log.Error().Err(err) shouldVote <- false @@ -117,8 +120,11 @@ func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Pr return } - if ps.YesVotesTotal+v.pendingProposalVotes[prop.DepositNonce] >= threshold { - shouldVote <- false + if ps.YesVotesTotal+v.pendingProposalVotes[propID] >= threshold && tries < maxShouldVoteChecks { + // Wait until proposal status is finalized to prevent missing votes + // in case of dropped txs + tries++ + v.shouldVoteForProposal(shouldVote, prop, sleep, tries) return } @@ -151,8 +157,25 @@ func (v *EVMVoter) trackProposalPendingVotes(ch chan common.Hash) { } if m.Name == "voteProposal" { + source := data[0].(uint8) depositNonce := data[1].(uint64) - v.pendingProposalVotes[depositNonce]++ + prop := proposal.Proposal{ + Source: source, + DepositNonce: depositNonce, + } + + go v.increaseProposalVoteCount(msg, prop.GetID()) } } } + +func (v *EVMVoter) increaseProposalVoteCount(hash common.Hash, propID common.Hash) { + v.pendingProposalVotes[propID]++ + + _, err := v.client.WaitAndReturnTxReceipt(hash) + if err != nil { + log.Error().Err(err) + } + + v.pendingProposalVotes[propID]-- +} From 07060c59f7d27c73c41a879b72889fa5ba3c1f38 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 16:09:17 +0100 Subject: [PATCH 11/20] Rebuild mocks --- Makefile | 3 +- chains/evm/evmgaspricer/mock/gas-pricer.go | 2 +- chains/evm/voter/mock/voter.go | 237 +++++++++++++++++++++ go.mod | 6 + go.sum | 3 + 5 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 chains/evm/voter/mock/voter.go diff --git a/Makefile b/Makefile index 82a31e3a..e5d93551 100644 --- a/Makefile +++ b/Makefile @@ -27,8 +27,9 @@ install-subkey: cargo install --force --git https://github.com/paritytech/substrate subkey genmocks: - mockgen -source=./chains/evm/evmgaspricer/gas-pricer.go -destination=./chains/evm/evmgaspricer/mock/gas-pricer.go + mockgen -source=chains/evm/evmgaspricer/gas-pricer.go -destination=chains/evm/evmgaspricer/mock/gas-pricer.go mockgen -source=chains/evm/calls/utils.go -destination=chains/evm/calls/mock/utils.go + mockgen -destination=chains/evm/voter/mock/voter.go github.com/ChainSafe/chainbridge-core/chains/evm/voter ChainClient,MessageHandler e2e-test: ./scripts/int_tests.sh diff --git a/chains/evm/evmgaspricer/mock/gas-pricer.go b/chains/evm/evmgaspricer/mock/gas-pricer.go index cc644606..b15db0cc 100644 --- a/chains/evm/evmgaspricer/mock/gas-pricer.go +++ b/chains/evm/evmgaspricer/mock/gas-pricer.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: ./chains/evm/evmgaspricer/gas-pricer.go +// Source: chains/evm/evmgaspricer/gas-pricer.go // Package mock_evmgaspricer is a generated GoMock package. package mock_evmgaspricer diff --git a/chains/evm/voter/mock/voter.go b/chains/evm/voter/mock/voter.go new file mode 100644 index 00000000..51a5fd19 --- /dev/null +++ b/chains/evm/voter/mock/voter.go @@ -0,0 +1,237 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/ChainSafe/chainbridge-core/chains/evm/voter (interfaces: ChainClient,MessageHandler) + +// Package mock_voter is a generated GoMock package. +package mock_voter + +import ( + context "context" + big "math/big" + reflect "reflect" + + evmclient "github.com/ChainSafe/chainbridge-core/chains/evm/evmclient" + proposal "github.com/ChainSafe/chainbridge-core/chains/evm/voter/proposal" + relayer "github.com/ChainSafe/chainbridge-core/relayer" + common "github.com/ethereum/go-ethereum/common" + types "github.com/ethereum/go-ethereum/core/types" + rpc "github.com/ethereum/go-ethereum/rpc" + gomock "github.com/golang/mock/gomock" +) + +// MockChainClient is a mock of ChainClient interface. +type MockChainClient struct { + ctrl *gomock.Controller + recorder *MockChainClientMockRecorder +} + +// MockChainClientMockRecorder is the mock recorder for MockChainClient. +type MockChainClientMockRecorder struct { + mock *MockChainClient +} + +// NewMockChainClient creates a new mock instance. +func NewMockChainClient(ctrl *gomock.Controller) *MockChainClient { + mock := &MockChainClient{ctrl: ctrl} + mock.recorder = &MockChainClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockChainClient) EXPECT() *MockChainClientMockRecorder { + return m.recorder +} + +// CallContract mocks base method. +func (m *MockChainClient) CallContract(arg0 context.Context, arg1 map[string]interface{}, arg2 *big.Int) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CallContract", arg0, arg1, arg2) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CallContract indicates an expected call of CallContract. +func (mr *MockChainClientMockRecorder) CallContract(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CallContract", reflect.TypeOf((*MockChainClient)(nil).CallContract), arg0, arg1, arg2) +} + +// From mocks base method. +func (m *MockChainClient) From() common.Address { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "From") + ret0, _ := ret[0].(common.Address) + return ret0 +} + +// From indicates an expected call of From. +func (mr *MockChainClientMockRecorder) From() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "From", reflect.TypeOf((*MockChainClient)(nil).From)) +} + +// LockNonce mocks base method. +func (m *MockChainClient) LockNonce() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "LockNonce") +} + +// LockNonce indicates an expected call of LockNonce. +func (mr *MockChainClientMockRecorder) LockNonce() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockNonce", reflect.TypeOf((*MockChainClient)(nil).LockNonce)) +} + +// RelayerAddress mocks base method. +func (m *MockChainClient) RelayerAddress() common.Address { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RelayerAddress") + ret0, _ := ret[0].(common.Address) + return ret0 +} + +// RelayerAddress indicates an expected call of RelayerAddress. +func (mr *MockChainClientMockRecorder) RelayerAddress() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RelayerAddress", reflect.TypeOf((*MockChainClient)(nil).RelayerAddress)) +} + +// SignAndSendTransaction mocks base method. +func (m *MockChainClient) SignAndSendTransaction(arg0 context.Context, arg1 evmclient.CommonTransaction) (common.Hash, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SignAndSendTransaction", arg0, arg1) + ret0, _ := ret[0].(common.Hash) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SignAndSendTransaction indicates an expected call of SignAndSendTransaction. +func (mr *MockChainClientMockRecorder) SignAndSendTransaction(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SignAndSendTransaction", reflect.TypeOf((*MockChainClient)(nil).SignAndSendTransaction), arg0, arg1) +} + +// SubscribePendingTransactions mocks base method. +func (m *MockChainClient) SubscribePendingTransactions(arg0 context.Context, arg1 chan<- common.Hash) (*rpc.ClientSubscription, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SubscribePendingTransactions", arg0, arg1) + ret0, _ := ret[0].(*rpc.ClientSubscription) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SubscribePendingTransactions indicates an expected call of SubscribePendingTransactions. +func (mr *MockChainClientMockRecorder) SubscribePendingTransactions(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SubscribePendingTransactions", reflect.TypeOf((*MockChainClient)(nil).SubscribePendingTransactions), arg0, arg1) +} + +// TransactionByHash mocks base method. +func (m *MockChainClient) TransactionByHash(arg0 context.Context, arg1 common.Hash) (*types.Transaction, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TransactionByHash", arg0, arg1) + ret0, _ := ret[0].(*types.Transaction) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// TransactionByHash indicates an expected call of TransactionByHash. +func (mr *MockChainClientMockRecorder) TransactionByHash(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TransactionByHash", reflect.TypeOf((*MockChainClient)(nil).TransactionByHash), arg0, arg1) +} + +// UnlockNonce mocks base method. +func (m *MockChainClient) UnlockNonce() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UnlockNonce") +} + +// UnlockNonce indicates an expected call of UnlockNonce. +func (mr *MockChainClientMockRecorder) UnlockNonce() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnlockNonce", reflect.TypeOf((*MockChainClient)(nil).UnlockNonce)) +} + +// UnsafeIncreaseNonce mocks base method. +func (m *MockChainClient) UnsafeIncreaseNonce() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnsafeIncreaseNonce") + ret0, _ := ret[0].(error) + return ret0 +} + +// UnsafeIncreaseNonce indicates an expected call of UnsafeIncreaseNonce. +func (mr *MockChainClientMockRecorder) UnsafeIncreaseNonce() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnsafeIncreaseNonce", reflect.TypeOf((*MockChainClient)(nil).UnsafeIncreaseNonce)) +} + +// UnsafeNonce mocks base method. +func (m *MockChainClient) UnsafeNonce() (*big.Int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnsafeNonce") + ret0, _ := ret[0].(*big.Int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UnsafeNonce indicates an expected call of UnsafeNonce. +func (mr *MockChainClientMockRecorder) UnsafeNonce() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnsafeNonce", reflect.TypeOf((*MockChainClient)(nil).UnsafeNonce)) +} + +// WaitAndReturnTxReceipt mocks base method. +func (m *MockChainClient) WaitAndReturnTxReceipt(arg0 common.Hash) (*types.Receipt, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WaitAndReturnTxReceipt", arg0) + ret0, _ := ret[0].(*types.Receipt) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// WaitAndReturnTxReceipt indicates an expected call of WaitAndReturnTxReceipt. +func (mr *MockChainClientMockRecorder) WaitAndReturnTxReceipt(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitAndReturnTxReceipt", reflect.TypeOf((*MockChainClient)(nil).WaitAndReturnTxReceipt), arg0) +} + +// MockMessageHandler is a mock of MessageHandler interface. +type MockMessageHandler struct { + ctrl *gomock.Controller + recorder *MockMessageHandlerMockRecorder +} + +// MockMessageHandlerMockRecorder is the mock recorder for MockMessageHandler. +type MockMessageHandlerMockRecorder struct { + mock *MockMessageHandler +} + +// NewMockMessageHandler creates a new mock instance. +func NewMockMessageHandler(ctrl *gomock.Controller) *MockMessageHandler { + mock := &MockMessageHandler{ctrl: ctrl} + mock.recorder = &MockMessageHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockMessageHandler) EXPECT() *MockMessageHandlerMockRecorder { + return m.recorder +} + +// HandleMessage mocks base method. +func (m *MockMessageHandler) HandleMessage(arg0 *relayer.Message) (*proposal.Proposal, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HandleMessage", arg0) + ret0, _ := ret[0].(*proposal.Proposal) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HandleMessage indicates an expected call of HandleMessage. +func (mr *MockMessageHandlerMockRecorder) HandleMessage(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HandleMessage", reflect.TypeOf((*MockMessageHandler)(nil).HandleMessage), arg0) +} diff --git a/go.mod b/go.mod index 47e25ead..63786b4a 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( require ( github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6 // indirect + github.com/VictoriaMetrics/fastcache v1.6.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/btcsuite/btcd v0.20.1-beta // indirect github.com/cespare/xxhash/v2 v2.1.1 // indirect @@ -36,18 +37,23 @@ require ( github.com/gorilla/websocket v1.4.2 // indirect github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d // indirect github.com/hashicorp/hcl v1.0.0 // indirect + github.com/holiman/bloomfilter/v2 v2.0.3 // indirect + github.com/holiman/uint256 v1.2.0 // indirect github.com/huin/goupnp v1.0.2 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/jackpal/go-nat-pmp v1.0.2-0.20160603034137-1fa385a6f458 // indirect github.com/magiconair/properties v1.8.5 // indirect + github.com/mattn/go-runewidth v0.0.9 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect github.com/mitchellh/mapstructure v1.4.2 // indirect + github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/pelletier/go-toml v1.9.4 // indirect github.com/pierrec/xxHash v0.1.5 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.2.0 // indirect github.com/prometheus/common v0.26.0 // indirect github.com/prometheus/procfs v0.6.0 // indirect + github.com/prometheus/tsdb v0.7.1 // indirect github.com/rjeczalik/notify v0.9.1 // indirect github.com/shirou/gopsutil v3.21.4-0.20210419000835-c7a38de76ee5+incompatible // indirect github.com/spf13/afero v1.6.0 // indirect diff --git a/go.sum b/go.sum index 7e2d105b..00ffaf4a 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,7 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= +github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8= github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= @@ -200,10 +201,12 @@ github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= +github.com/go-kit/kit v0.9.0 h1:wDJmvq38kDhkVxi50ni9ykkdUr1PKgqKOoi01fa0Mdk= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= +github.com/go-logfmt/logfmt v0.5.0 h1:TrB8swr/68K7m9CcGut2g3UOihhbcbiMAYiuTXdEih4= github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-ole/go-ole v1.2.1 h1:2lOsA72HgjxAuMlKpFiCbHTvu44PIVkZ5hqm3RSdI/E= github.com/go-ole/go-ole v1.2.1/go.mod h1:7FAglXiTm7HKlQRDeOQ6ZNUHidzCWXuZWq/1dTyBNF8= From 84b22d1dab9807bc13bc07d6dedd5ca40b483d40 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 16:09:26 +0100 Subject: [PATCH 12/20] Add voter tests --- chains/evm/voter/voter.go | 16 ++-- chains/evm/voter/voter_test.go | 130 +++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 chains/evm/voter/voter_test.go diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index 6e0492a5..3f21d87a 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -27,6 +27,10 @@ const ( shouldVoteCheckPeriod = 15 ) +var ( + Sleep = time.Sleep +) + type ChainClient interface { RelayerAddress() common.Address CallContract(ctx context.Context, callArgs map[string]interface{}, blockNumber *big.Int) ([]byte, error) @@ -78,7 +82,7 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { } shouldVoteChn := make(chan bool) - go v.shouldVoteForProposal(shouldVoteChn, prop, time.Sleep, 0) + go v.shouldVoteForProposal(shouldVoteChn, prop, 0) shouldVote := <-shouldVoteChn if !shouldVote { @@ -95,13 +99,13 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { return nil } -func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, sleep func(d time.Duration), tries int) { - sleep(time.Duration(rand.Intn(shouldVoteCheckPeriod)) * time.Second) +func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, tries int) { propID := prop.GetID() - defer delete(v.pendingProposalVotes, propID) - ps, err := calls.ProposalStatus(v.client, prop) + Sleep(time.Duration(rand.Intn(shouldVoteCheckPeriod)) * time.Second) + + ps, err := calls.ProposalStatus(v.client, prop) if err != nil { log.Error().Err(err) shouldVote <- false @@ -124,7 +128,7 @@ func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Pr // Wait until proposal status is finalized to prevent missing votes // in case of dropped txs tries++ - v.shouldVoteForProposal(shouldVote, prop, sleep, tries) + v.shouldVoteForProposal(shouldVote, prop, tries) return } diff --git a/chains/evm/voter/voter_test.go b/chains/evm/voter/voter_test.go new file mode 100644 index 00000000..99a4b560 --- /dev/null +++ b/chains/evm/voter/voter_test.go @@ -0,0 +1,130 @@ +package voter_test + +import ( + "encoding/hex" + "errors" + "testing" + "time" + + "github.com/ChainSafe/chainbridge-core/chains/evm/evmgaspricer" + "github.com/ChainSafe/chainbridge-core/chains/evm/evmtransaction" + "github.com/ChainSafe/chainbridge-core/chains/evm/voter" + mock_voter "github.com/ChainSafe/chainbridge-core/chains/evm/voter/mock" + "github.com/ChainSafe/chainbridge-core/chains/evm/voter/proposal" + "github.com/ChainSafe/chainbridge-core/relayer" + "github.com/ethereum/go-ethereum/common" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" +) + +var ( + proposalVotedResponse, _ = hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000001") + proposalNotVotedResponse, _ = hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000") + executedProposalStatus, _ = hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000001c0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000001f") + inactiveProposalStatus, _ = hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000") +) + +type VoterTestSuite struct { + suite.Suite + voter *voter.EVMVoter + mockMessageHandler *mock_voter.MockMessageHandler + mockClient *mock_voter.MockChainClient +} + +func TestRunVoterTestSuite(t *testing.T) { + suite.Run(t, new(VoterTestSuite)) +} + +func (s *VoterTestSuite) SetupSuite() {} +func (s *VoterTestSuite) TearDownSuite() {} +func (s *VoterTestSuite) SetupTest() { + gomockController := gomock.NewController(s.T()) + s.mockMessageHandler = mock_voter.NewMockMessageHandler(gomockController) + s.mockClient = mock_voter.NewMockChainClient(gomockController) + s.mockClient.EXPECT().SubscribePendingTransactions(gomock.Any(), gomock.Any()) + s.voter = voter.NewVoter( + s.mockMessageHandler, + s.mockClient, + evmtransaction.NewTransaction, + &evmgaspricer.LondonGasPriceDeterminant{}, + ) + voter.Sleep = func(d time.Duration) {} +} +func (s *VoterTestSuite) TearDownTest() {} + +func (s *VoterTestSuite) TestVoteProposal_HandleMessageError() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(nil, errors.New("error")) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.NotNil(err) +} + +func (s *VoterTestSuite) TestVoteProposal_IsProposalVotedByError() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ + Source: 0, + DepositNonce: 0, + }, nil) + s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, errors.New("error")) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.NotNil(err) +} + +func (s *VoterTestSuite) TestVoteProposal_ProposalAlreadyVoted() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ + Source: 0, + DepositNonce: 0, + }, nil) + s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(proposalVotedResponse, nil) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.Nil(err) +} + +func (s *VoterTestSuite) TestVoteProposal_ProposalStatusFail() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ + Source: 0, + DepositNonce: 0, + }, nil) + s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(proposalNotVotedResponse, nil) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, errors.New("error")) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.Nil(err) +} + +func (s *VoterTestSuite) TestVoteProposal_ExecutedProposal() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ + Source: 0, + DepositNonce: 0, + }, nil) + s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(proposalNotVotedResponse, nil) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(executedProposalStatus, nil) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.Nil(err) +} + +func (s *VoterTestSuite) TestVoteProposal_InactiveProposal() { + s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ + Source: 0, + DepositNonce: 0, + }, nil) + s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(proposalNotVotedResponse, nil) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(inactiveProposalStatus, nil) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{5}, nil) + + err := s.voter.VoteProposal(&relayer.Message{}) + + s.Nil(err) +} From 16ab0ff630d177921ea186405d5fe4058a436a34 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 16:29:22 +0100 Subject: [PATCH 13/20] Update godocs --- chains/evm/voter/voter.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index 3f21d87a..11f59e0a 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -67,6 +67,8 @@ func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasP return voter } +// VoteProposal checks if relayer already voted and is threshold +// satisfied and casts a vote if it isn't func (v *EVMVoter) VoteProposal(m *relayer.Message) error { prop, err := v.mh.HandleMessage(m) if err != nil { @@ -86,7 +88,7 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { shouldVote := <-shouldVoteChn if !shouldVote { - log.Debug().Msgf("Proposal %+v already has enough votes", prop) + log.Debug().Msgf("Proposal %+v already satisfies threshold", prop) return nil } From f56d5c3a321234c67614e39b0b0e53a4b4b2c7d4 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 16:33:06 +0100 Subject: [PATCH 14/20] Fix typo --- chains/evm/voter/proposal/proposal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chains/evm/voter/proposal/proposal.go b/chains/evm/voter/proposal/proposal.go index 73a7c94f..fb43c308 100644 --- a/chains/evm/voter/proposal/proposal.go +++ b/chains/evm/voter/proposal/proposal.go @@ -32,7 +32,7 @@ func (p *Proposal) GetDataHash() common.Hash { return crypto.Keccak256Hash(append(p.HandlerAddress.Bytes(), p.Data...)) } -// GetID construct proposal unique identifier +// GetID constructs proposal unique identifier func (p *Proposal) GetID() common.Hash { return crypto.Keccak256Hash(append([]byte{p.Source}, byte(p.DepositNonce))) } From ded1a17c64deb3e05fb17027e83cbaafc8ecfd4d Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Mon, 15 Nov 2021 16:36:04 +0100 Subject: [PATCH 15/20] Lint --- chains/evm/voter/voter.go | 11 +++++++---- chains/evm/voter/voter_test.go | 2 +- e2e/evm-evm/example/app/app.go | 10 ++++++++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index 11f59e0a..dcee29ec 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -51,7 +51,7 @@ type EVMVoter struct { pendingProposalVotes map[common.Hash]uint8 } -func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) *EVMVoter { +func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) (*EVMVoter, error) { voter := &EVMVoter{ mh: mh, client: client, @@ -61,10 +61,13 @@ func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasP } ch := make(chan common.Hash) - go voter.trackProposalPendingVotes(ch) - client.SubscribePendingTransactions(context.TODO(), ch) + _, err := client.SubscribePendingTransactions(context.TODO(), ch) + if err != nil { + return nil, err + } - return voter + go voter.trackProposalPendingVotes(ch) + return voter, nil } // VoteProposal checks if relayer already voted and is threshold diff --git a/chains/evm/voter/voter_test.go b/chains/evm/voter/voter_test.go index 99a4b560..467b44e6 100644 --- a/chains/evm/voter/voter_test.go +++ b/chains/evm/voter/voter_test.go @@ -42,7 +42,7 @@ func (s *VoterTestSuite) SetupTest() { s.mockMessageHandler = mock_voter.NewMockMessageHandler(gomockController) s.mockClient = mock_voter.NewMockChainClient(gomockController) s.mockClient.EXPECT().SubscribePendingTransactions(gomock.Any(), gomock.Any()) - s.voter = voter.NewVoter( + s.voter, _ = voter.NewVoter( s.mockMessageHandler, s.mockClient, evmtransaction.NewTransaction, diff --git a/e2e/evm-evm/example/app/app.go b/e2e/evm-evm/example/app/app.go index f3a3271c..32260759 100644 --- a/e2e/evm-evm/example/app/app.go +++ b/e2e/evm-evm/example/app/app.go @@ -48,8 +48,11 @@ func Run() error { mh := voter.NewEVMMessageHandler(evm1Client, common.HexToAddress(evm1Cfg.SharedEVMConfig.Bridge)) mh.RegisterMessageHandler(common.HexToAddress(evm1Cfg.SharedEVMConfig.Erc20Handler), voter.ERC20MessageHandler) mh.RegisterMessageHandler(common.HexToAddress(evm1Cfg.SharedEVMConfig.GenericHandler), voter.GenericMessageHandler) - evmeVoter := voter.NewVoter(mh, evm1Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm1Client, nil)) + evmeVoter, err := voter.NewVoter(mh, evm1Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm1Client, nil)) + if err != nil { + panic(err) + } evm1Chain := evm.NewEVMChain(evm1Listener, evmeVoter, db, *evm1Cfg.SharedEVMConfig.GeneralChainConfig.Id, &evm1Cfg.SharedEVMConfig) ////EVM2 setup @@ -69,8 +72,11 @@ func Run() error { mhEVM := voter.NewEVMMessageHandler(evm2Client, common.HexToAddress(evm2Config.SharedEVMConfig.Bridge)) mhEVM.RegisterMessageHandler(common.HexToAddress(evm2Config.SharedEVMConfig.Erc20Handler), voter.ERC20MessageHandler) mhEVM.RegisterMessageHandler(common.HexToAddress(evm2Config.SharedEVMConfig.GenericHandler), voter.GenericMessageHandler) - evm2Voter := voter.NewVoter(mhEVM, evm2Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm2Client, nil)) + evm2Voter, err := voter.NewVoter(mhEVM, evm2Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm2Client, nil)) + if err != nil { + panic(err) + } evm2Chain := evm.NewEVMChain(evm2Listener, evm2Voter, db, *evm2Config.SharedEVMConfig.GeneralChainConfig.Id, &evm2Config.SharedEVMConfig) r := relayer.NewRelayer(relayerConfig.RelayerConfig{PrometheusEndpoint: "/metrics", PrometheusPort: 2112}, []relayer.RelayedChain{evm2Chain, evm1Chain}) From 27525c3184517c45d9336da48ff899eeede2c0d5 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Wed, 17 Nov 2021 13:16:55 +0100 Subject: [PATCH 16/20] Add godocs, remove channel from shouldVoteForProposal and add fallback constructor when subscription is not supported --- chains/evm/voter/voter.go | 55 +++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index dcee29ec..78275e3e 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -51,7 +51,11 @@ type EVMVoter struct { pendingProposalVotes map[common.Hash]uint8 } -func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) (*EVMVoter, error) { +// NewVoterWithSubscription creates EVMVoter with pending proposal subscription +// that listens to pending voteProposal transactions and avoids wasting gas +// on sending votes for transactions that will fail. +// Currently, officialy supported only by Geth nodes. +func NewVoterWithSubscription(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer, subscribeToPendingTxs bool) (*EVMVoter, error) { voter := &EVMVoter{ mh: mh, client: client, @@ -67,11 +71,25 @@ func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasP } go voter.trackProposalPendingVotes(ch) + return voter, nil } +// NewVoter creates EVMVoter without pending proposal subscription. +// It is a fallback for nodes that don't support pending transaction subscription +// and will vote on proposals that already satisfy threshold thus wasting gas. +func NewVoter(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) *EVMVoter { + return &EVMVoter{ + mh: mh, + client: client, + fabric: fabric, + gasPriceClient: gasPriceClient, + pendingProposalVotes: make(map[common.Hash]uint8), + } +} + // VoteProposal checks if relayer already voted and is threshold -// satisfied and casts a vote if it isn't +// satisfied and casts a vote if it isn't. func (v *EVMVoter) VoteProposal(m *relayer.Message) error { prop, err := v.mh.HandleMessage(m) if err != nil { @@ -86,10 +104,12 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { return nil } - shouldVoteChn := make(chan bool) - go v.shouldVoteForProposal(shouldVoteChn, prop, 0) + shouldVote, err := v.shouldVoteForProposal(prop, 0) + if err != nil { + log.Error().Err(err) + return err + } - shouldVote := <-shouldVoteChn if !shouldVote { log.Debug().Msgf("Proposal %+v already satisfies threshold", prop) return nil @@ -104,7 +124,10 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { return nil } -func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Proposal, tries int) { +// shouldVoteForProposal checks if proposal already has threshold with pending +// proposal votes from other relayers. +// Only works in conjuction with NewVoterWithSubscription. +func (v *EVMVoter) shouldVoteForProposal(prop *proposal.Proposal, tries int) (bool, error) { propID := prop.GetID() defer delete(v.pendingProposalVotes, propID) @@ -112,34 +135,30 @@ func (v *EVMVoter) shouldVoteForProposal(shouldVote chan bool, prop *proposal.Pr ps, err := calls.ProposalStatus(v.client, prop) if err != nil { - log.Error().Err(err) - shouldVote <- false - return + return false, err } if ps.Status == relayer.ProposalStatusExecuted || ps.Status == relayer.ProposalStatusCanceled { - shouldVote <- false - return + return false, nil } threshold, err := calls.GetThreshold(v.client, &prop.BridgeAddress) if err != nil { - log.Error().Err(err) - shouldVote <- false - return + return false, err } if ps.YesVotesTotal+v.pendingProposalVotes[propID] >= threshold && tries < maxShouldVoteChecks { // Wait until proposal status is finalized to prevent missing votes // in case of dropped txs tries++ - v.shouldVoteForProposal(shouldVote, prop, tries) - return + return v.shouldVoteForProposal(prop, tries) } - shouldVote <- true + return true, nil } +// trackProposalPendingVotes tracks pending voteProposal txs from +// other relayers. func (v *EVMVoter) trackProposalPendingVotes(ch chan common.Hash) { for msg := range ch { txData, _, err := v.client.TransactionByHash(context.TODO(), msg) @@ -178,6 +197,8 @@ func (v *EVMVoter) trackProposalPendingVotes(ch chan common.Hash) { } } +// increaseProposalVoteCount increases pending proposal vote for target proposal +// and decreases it when transaction is mined. func (v *EVMVoter) increaseProposalVoteCount(hash common.Hash, propID common.Hash) { v.pendingProposalVotes[propID]++ From 2e22af37c315b6fa6a490d29bd49e88d36f9c1a4 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Wed, 17 Nov 2021 13:17:38 +0100 Subject: [PATCH 17/20] Remove extra argument --- chains/evm/voter/voter.go | 3 +-- e2e/evm-evm/example/app/app.go | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index 78275e3e..bdf12143 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -55,7 +55,7 @@ type EVMVoter struct { // that listens to pending voteProposal transactions and avoids wasting gas // on sending votes for transactions that will fail. // Currently, officialy supported only by Geth nodes. -func NewVoterWithSubscription(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer, subscribeToPendingTxs bool) (*EVMVoter, error) { +func NewVoterWithSubscription(mh MessageHandler, client ChainClient, fabric calls.TxFabric, gasPriceClient calls.GasPricer) (*EVMVoter, error) { voter := &EVMVoter{ mh: mh, client: client, @@ -69,7 +69,6 @@ func NewVoterWithSubscription(mh MessageHandler, client ChainClient, fabric call if err != nil { return nil, err } - go voter.trackProposalPendingVotes(ch) return voter, nil diff --git a/e2e/evm-evm/example/app/app.go b/e2e/evm-evm/example/app/app.go index 32260759..c1a4e8be 100644 --- a/e2e/evm-evm/example/app/app.go +++ b/e2e/evm-evm/example/app/app.go @@ -49,7 +49,7 @@ func Run() error { mh.RegisterMessageHandler(common.HexToAddress(evm1Cfg.SharedEVMConfig.Erc20Handler), voter.ERC20MessageHandler) mh.RegisterMessageHandler(common.HexToAddress(evm1Cfg.SharedEVMConfig.GenericHandler), voter.GenericMessageHandler) - evmeVoter, err := voter.NewVoter(mh, evm1Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm1Client, nil)) + evmeVoter, err := voter.NewVoterWithSubscription(mh, evm1Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm1Client, nil)) if err != nil { panic(err) } @@ -73,7 +73,7 @@ func Run() error { mhEVM.RegisterMessageHandler(common.HexToAddress(evm2Config.SharedEVMConfig.Erc20Handler), voter.ERC20MessageHandler) mhEVM.RegisterMessageHandler(common.HexToAddress(evm2Config.SharedEVMConfig.GenericHandler), voter.GenericMessageHandler) - evm2Voter, err := voter.NewVoter(mhEVM, evm2Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm2Client, nil)) + evm2Voter, err := voter.NewVoterWithSubscription(mhEVM, evm2Client, evmtransaction.NewTransaction, evmgaspricer.NewLondonGasPriceClient(evm2Client, nil)) if err != nil { panic(err) } From 7bad54b0c03e4243251d5c3eafaf8ba278fad101 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Wed, 17 Nov 2021 13:26:14 +0100 Subject: [PATCH 18/20] Fix linter --- chains/evm/voter/voter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chains/evm/voter/voter_test.go b/chains/evm/voter/voter_test.go index 467b44e6..99a4b560 100644 --- a/chains/evm/voter/voter_test.go +++ b/chains/evm/voter/voter_test.go @@ -42,7 +42,7 @@ func (s *VoterTestSuite) SetupTest() { s.mockMessageHandler = mock_voter.NewMockMessageHandler(gomockController) s.mockClient = mock_voter.NewMockChainClient(gomockController) s.mockClient.EXPECT().SubscribePendingTransactions(gomock.Any(), gomock.Any()) - s.voter, _ = voter.NewVoter( + s.voter = voter.NewVoter( s.mockMessageHandler, s.mockClient, evmtransaction.NewTransaction, From 1a23893c1e35ff7fbf03f2932cd269fbf11601c5 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Wed, 17 Nov 2021 13:57:23 +0100 Subject: [PATCH 19/20] Fix tests --- chains/evm/voter/voter_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/chains/evm/voter/voter_test.go b/chains/evm/voter/voter_test.go index 99a4b560..47482d06 100644 --- a/chains/evm/voter/voter_test.go +++ b/chains/evm/voter/voter_test.go @@ -41,7 +41,6 @@ func (s *VoterTestSuite) SetupTest() { gomockController := gomock.NewController(s.T()) s.mockMessageHandler = mock_voter.NewMockMessageHandler(gomockController) s.mockClient = mock_voter.NewMockChainClient(gomockController) - s.mockClient.EXPECT().SubscribePendingTransactions(gomock.Any(), gomock.Any()) s.voter = voter.NewVoter( s.mockMessageHandler, s.mockClient, @@ -97,7 +96,7 @@ func (s *VoterTestSuite) TestVoteProposal_ProposalStatusFail() { err := s.voter.VoteProposal(&relayer.Message{}) - s.Nil(err) + s.NotNil(err) } func (s *VoterTestSuite) TestVoteProposal_ExecutedProposal() { @@ -114,7 +113,7 @@ func (s *VoterTestSuite) TestVoteProposal_ExecutedProposal() { s.Nil(err) } -func (s *VoterTestSuite) TestVoteProposal_InactiveProposal() { +func (s *VoterTestSuite) TestVoteProposal_GetThresholdFail() { s.mockMessageHandler.EXPECT().HandleMessage(gomock.Any()).Return(&proposal.Proposal{ Source: 0, DepositNonce: 0, @@ -122,9 +121,9 @@ func (s *VoterTestSuite) TestVoteProposal_InactiveProposal() { s.mockClient.EXPECT().RelayerAddress().Return(common.Address{}) s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(proposalNotVotedResponse, nil) s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return(inactiveProposalStatus, nil) - s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{5}, nil) + s.mockClient.EXPECT().CallContract(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, errors.New("error")) err := s.voter.VoteProposal(&relayer.Message{}) - s.Nil(err) + s.NotNil(err) } From 41488e219a232df1bb03cb6bc38cf7a06a1bdb04 Mon Sep 17 00:00:00 2001 From: Matija Petrunic Date: Fri, 19 Nov 2021 11:45:41 +0100 Subject: [PATCH 20/20] Improve voter pending tx subscription godocs --- chains/evm/voter/voter.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/chains/evm/voter/voter.go b/chains/evm/voter/voter.go index bdf12143..ea715b84 100644 --- a/chains/evm/voter/voter.go +++ b/chains/evm/voter/voter.go @@ -125,11 +125,14 @@ func (v *EVMVoter) VoteProposal(m *relayer.Message) error { // shouldVoteForProposal checks if proposal already has threshold with pending // proposal votes from other relayers. -// Only works in conjuction with NewVoterWithSubscription. +// Only works properly in conjuction with NewVoterWithSubscription as without a subscription +// no pending txs would be received and pending vote count would be 0. func (v *EVMVoter) shouldVoteForProposal(prop *proposal.Proposal, tries int) (bool, error) { propID := prop.GetID() defer delete(v.pendingProposalVotes, propID) + // random delay to prevent all relayers checking for pending votes + // at the same time and all of them sending another tx Sleep(time.Duration(rand.Intn(shouldVoteCheckPeriod)) * time.Second) ps, err := calls.ProposalStatus(v.client, prop) @@ -157,7 +160,8 @@ func (v *EVMVoter) shouldVoteForProposal(prop *proposal.Proposal, tries int) (bo } // trackProposalPendingVotes tracks pending voteProposal txs from -// other relayers. +// other relayers and increases count of pending votes in pendingProposalVotes map +// by proposal unique id. func (v *EVMVoter) trackProposalPendingVotes(ch chan common.Hash) { for msg := range ch { txData, _, err := v.client.TransactionByHash(context.TODO(), msg)