From abce770c04b368c13be2b511b646c6288bfbac4c Mon Sep 17 00:00:00 2001 From: Mdaiki0730 Date: Fri, 22 Mar 2024 11:56:46 +0900 Subject: [PATCH 1/4] Compare the signer of the normal message and the signer of MsgExcute --- x/zkauth/keeper/keeper.go | 5 ++++- x/zkauth/keeper/msg_server.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/x/zkauth/keeper/keeper.go b/x/zkauth/keeper/keeper.go index 02bce0f3d5..65eda1a94c 100644 --- a/x/zkauth/keeper/keeper.go +++ b/x/zkauth/keeper/keeper.go @@ -52,13 +52,16 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName)) } -func (k Keeper) DispatchMsgs(ctx sdk.Context, msgs []sdk.Msg) ([][]byte, error) { +func (k Keeper) DispatchMsgs(ctx sdk.Context, msgs []sdk.Msg, author sdk.AccAddress) ([][]byte, error) { results := make([][]byte, 0, len(msgs)) for i, msg := range msgs { signers := msg.GetSigners() if len(signers) != 1 { return nil, sdkerrors.ErrInvalidRequest.Wrap("authorization can be given to msg with only one signer") } + if !author.Equals(signers[0]) { + return nil, sdkerrors.ErrUnauthorized.Wrapf("bad signer; expected %s, got %s", author, signers[0]) + } if err := msg.ValidateBasic(); err != nil { return nil, err } diff --git a/x/zkauth/keeper/msg_server.go b/x/zkauth/keeper/msg_server.go index d0d27329b2..71651f6007 100644 --- a/x/zkauth/keeper/msg_server.go +++ b/x/zkauth/keeper/msg_server.go @@ -27,7 +27,10 @@ func (k msgServer) Execution(goCtx context.Context, msg *types.MsgExecution) (*t return nil, err } - results, err := k.DispatchMsgs(ctx, msgs) + // zksigner is assumed to be one + zksigner := msg.GetSigners()[0] + + results, err := k.DispatchMsgs(ctx, msgs, zksigner) if err != nil { return nil, err } From e06a40b41c3b114f10a4303743738e16c0bef8eb Mon Sep 17 00:00:00 2001 From: Mdaiki0730 Date: Fri, 22 Mar 2024 12:11:27 +0900 Subject: [PATCH 2/4] Add CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48f16b8174..16bf011551 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/zkauth) [\#1280](https://github.com/Finschia/finschia-sdk/pull/1280) Fetch jwk considering goroutine exit ### Bug Fixes +* (x/zkauth) [\#1291](https://github.com/Finschia/finschia-sdk/pull/1291) Compare the signer of the normal message and the signer of MsgExcute ### Removed From 5118e5332f94ab961df69453ef8ba078aa7f3c67 Mon Sep 17 00:00:00 2001 From: Mdaiki0730 Date: Fri, 22 Mar 2024 16:40:03 +0900 Subject: [PATCH 3/4] Modification of test --- x/zkauth/keeper/keeper_test.go | 26 +++++++++++++++++++++----- x/zkauth/keeper/msg_server_test.go | 20 +++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/x/zkauth/keeper/keeper_test.go b/x/zkauth/keeper/keeper_test.go index cfda8a2d36..3df69bd808 100644 --- a/x/zkauth/keeper/keeper_test.go +++ b/x/zkauth/keeper/keeper_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "context" + "encoding/base64" "encoding/json" "net/http" "net/http/httptest" @@ -81,9 +82,10 @@ func TestDispatchMsgs(t *testing.T) { testApp := testutil.ZkAuthKeeper(t) app, k, ctx := testApp.Simapp, testApp.ZKAuthKeeper, testApp.Ctx - addrs := simapp.AddTestAddrs(app, ctx, 2, sdk.NewInt(100)) - fromAddr := addrs[0] - toAddr := addrs[1] + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(100)) + fromAddr := sdk.MustAccAddressFromBech32("link1f7j46mgxr3d5tgn3qsnn9ep8j77yr8w4ks0f3lpzz9rhnwja9vcqej33ld") + toAddr := addrs[0] + err := simapp.FundAccount(app, ctx, fromAddr, sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))) newCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 5)) @@ -93,13 +95,27 @@ func TestDispatchMsgs(t *testing.T) { ToAddress: toAddr.String(), } - zkAuthSig := types.ZKAuthSignature{} + testProofB64 := "eyJwaV9hIjpbIjM0MjM1MjQ3MjQzMDgyMDE5Mzc5NTQyMDUwMjA3MTMyNDg0MTg5OTQ5NjEzODk1OTcxODgxNjk1ODg4Njk5Mzg0MDgyMjI1NjE2MzAiLCI4OTQ1NzMxODIxOTg1NDU0MTcwNDA5MjQ2MjA4MDUxMzYwMzcxNzQ0NjUxOTYwNjg2MjQ5Njg1NjExMDI0NzQ4MjQ5MDk2NjUyNDAwIiwiMSJdLCJwaV9iIjpbWyIxNjE3NzgwNjczNzQ1MTg2MDY3NDk3NTAxNjI1MjExODM3NDU0NDU5NjE5NzcxMDAyNjYzMjAxMTg2MTg4OTM4NDkxMDc3MTU5ODUwIiwiMTAzODU2MDMyMjQzMDc2MDQxMjM5NjI2NzAyMzA3NDcxNDM2NDc2ODAxOTAwNTQzNzU3OTE5NjAxNjYwMTY2NzMyNTcwNjE5ODExNjgiXSxbIjYyMTE1NjA5NTUzMTE1NzgxNDcwMzEzMjAzMTE1MzAyODM5MzgwMDY5MTIyMjAwMTUwODM1MTIxODg2MDI5MzU0MjA4Mjk5MjY1NCIsIjEzODUwMDYzMTA5ODg2MTE3MDcwNzgzMzIxNDk1ODI4MzQ2MTg3ODQ3OTM3OTIyMDMyNTI2NzU3MTU2MDg0MjUzMTU4NTc0NjgyMTYyIl0sWyIxIiwiMCJdXSwicGlfYyI6WyIxMDE0MTM1NTQwNTE1MDg5MzQ1MjY3NTUwMTE4Mjk4MTY1MzY5NDI1MzM0MzMyMjAyNzM1OTgwNTU0NzYxODgyODE4NjU3NDU5MTA4IiwiNDkzMjY5MjI2OTcyOTIyNDQ4NjI1MDI0MDAyMjI4NTQ4NzE1MjYzNTkwNjAzMzA1ODYwNjgwMjI2Nzg4Nzg5NjE2MTU2NTM4MzYiLCIxIl19" + testProof, err := base64.StdEncoding.DecodeString(testProofB64) + require.NoError(t, err) + + zkAuthSig := types.ZKAuthSignature{ + ZkAuthInputs: &types.ZKAuthInputs{ + ProofPoints: testProof, + IssBase64: "aHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29t", + HeaderBase64: "eyJhbGciOiJSUzI1NiIsImtpZCI6ImFkZjVlNzEwZWRmZWJlY2JlZmE5YTYxNDk1NjU0ZDAzYzBiOGVkZjgiLCJ0eXAiOiJKV1QifQ", + AddressSeed: "16929294337693897740349056748881744503581363933815798702166939594477679063350", + }, + MaxBlockHeight: 104, + } msgs := types.NewMsgExecution([]sdk.Msg{&bankMsg}, zkAuthSig) execMsgs, err := msgs.GetMessages() require.NoError(t, err) - result, err := k.DispatchMsgs(ctx, execMsgs) + // zksigner is assumed to be one + zksigner := msgs.GetSigners()[0] + result, err := k.DispatchMsgs(ctx, execMsgs, zksigner) require.NoError(t, err) require.NotNil(t, result) diff --git a/x/zkauth/keeper/msg_server_test.go b/x/zkauth/keeper/msg_server_test.go index bfe0abd2ed..f0fa0c240e 100644 --- a/x/zkauth/keeper/msg_server_test.go +++ b/x/zkauth/keeper/msg_server_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "encoding/base64" "testing" "github.com/stretchr/testify/require" @@ -23,8 +24,9 @@ func TestExecution(t *testing.T) { msgServer, app, ctx := setupMsgServer(t) newCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 5)) addrs := simapp.AddTestAddrs(app, ctx, 2, sdk.NewInt(100)) - fromAddr := addrs[0] - toAddr := addrs[1] + fromAddr := sdk.MustAccAddressFromBech32("link1f7j46mgxr3d5tgn3qsnn9ep8j77yr8w4ks0f3lpzz9rhnwja9vcqej33ld") + toAddr := addrs[0] + err := simapp.FundAccount(app, ctx, fromAddr, sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))) bankMsg := banktypes.MsgSend{ Amount: newCoins, @@ -32,7 +34,19 @@ func TestExecution(t *testing.T) { ToAddress: toAddr.String(), } - zkAuthSig := types.ZKAuthSignature{} + testProofB64 := "eyJwaV9hIjpbIjM0MjM1MjQ3MjQzMDgyMDE5Mzc5NTQyMDUwMjA3MTMyNDg0MTg5OTQ5NjEzODk1OTcxODgxNjk1ODg4Njk5Mzg0MDgyMjI1NjE2MzAiLCI4OTQ1NzMxODIxOTg1NDU0MTcwNDA5MjQ2MjA4MDUxMzYwMzcxNzQ0NjUxOTYwNjg2MjQ5Njg1NjExMDI0NzQ4MjQ5MDk2NjUyNDAwIiwiMSJdLCJwaV9iIjpbWyIxNjE3NzgwNjczNzQ1MTg2MDY3NDk3NTAxNjI1MjExODM3NDU0NDU5NjE5NzcxMDAyNjYzMjAxMTg2MTg4OTM4NDkxMDc3MTU5ODUwIiwiMTAzODU2MDMyMjQzMDc2MDQxMjM5NjI2NzAyMzA3NDcxNDM2NDc2ODAxOTAwNTQzNzU3OTE5NjAxNjYwMTY2NzMyNTcwNjE5ODExNjgiXSxbIjYyMTE1NjA5NTUzMTE1NzgxNDcwMzEzMjAzMTE1MzAyODM5MzgwMDY5MTIyMjAwMTUwODM1MTIxODg2MDI5MzU0MjA4Mjk5MjY1NCIsIjEzODUwMDYzMTA5ODg2MTE3MDcwNzgzMzIxNDk1ODI4MzQ2MTg3ODQ3OTM3OTIyMDMyNTI2NzU3MTU2MDg0MjUzMTU4NTc0NjgyMTYyIl0sWyIxIiwiMCJdXSwicGlfYyI6WyIxMDE0MTM1NTQwNTE1MDg5MzQ1MjY3NTUwMTE4Mjk4MTY1MzY5NDI1MzM0MzMyMjAyNzM1OTgwNTU0NzYxODgyODE4NjU3NDU5MTA4IiwiNDkzMjY5MjI2OTcyOTIyNDQ4NjI1MDI0MDAyMjI4NTQ4NzE1MjYzNTkwNjAzMzA1ODYwNjgwMjI2Nzg4Nzg5NjE2MTU2NTM4MzYiLCIxIl19" + testProof, err := base64.StdEncoding.DecodeString(testProofB64) + require.NoError(t, err) + + zkAuthSig := types.ZKAuthSignature{ + ZkAuthInputs: &types.ZKAuthInputs{ + ProofPoints: testProof, + IssBase64: "aHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29t", + HeaderBase64: "eyJhbGciOiJSUzI1NiIsImtpZCI6ImFkZjVlNzEwZWRmZWJlY2JlZmE5YTYxNDk1NjU0ZDAzYzBiOGVkZjgiLCJ0eXAiOiJKV1QifQ", + AddressSeed: "16929294337693897740349056748881744503581363933815798702166939594477679063350", + }, + MaxBlockHeight: 104, + } msgs := types.NewMsgExecution([]sdk.Msg{&bankMsg}, zkAuthSig) From 760851a63198f513ed8f995708d36cb72293d1de Mon Sep 17 00:00:00 2001 From: Mdaiki0730 Date: Mon, 25 Mar 2024 15:23:53 +0900 Subject: [PATCH 4/4] Add invalid signer case to test --- x/zkauth/keeper/keeper_test.go | 103 +++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/x/zkauth/keeper/keeper_test.go b/x/zkauth/keeper/keeper_test.go index 3df69bd808..590ddcaa0d 100644 --- a/x/zkauth/keeper/keeper_test.go +++ b/x/zkauth/keeper/keeper_test.go @@ -14,6 +14,7 @@ import ( "github.com/Finschia/finschia-sdk/simapp" sdk "github.com/Finschia/finschia-sdk/types" + sdkerrors "github.com/Finschia/finschia-sdk/types/errors" banktypes "github.com/Finschia/finschia-sdk/x/bank/types" "github.com/Finschia/finschia-sdk/x/zkauth/testutil" "github.com/Finschia/finschia-sdk/x/zkauth/types" @@ -79,49 +80,67 @@ func TestFetchJwk(t *testing.T) { } func TestDispatchMsgs(t *testing.T) { - testApp := testutil.ZkAuthKeeper(t) - app, k, ctx := testApp.Simapp, testApp.ZKAuthKeeper, testApp.Ctx - - addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(100)) - fromAddr := sdk.MustAccAddressFromBech32("link1f7j46mgxr3d5tgn3qsnn9ep8j77yr8w4ks0f3lpzz9rhnwja9vcqej33ld") - toAddr := addrs[0] - err := simapp.FundAccount(app, ctx, fromAddr, sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))) - - newCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 5)) - - bankMsg := banktypes.MsgSend{ - Amount: newCoins, - FromAddress: fromAddr.String(), - ToAddress: toAddr.String(), - } - - testProofB64 := "eyJwaV9hIjpbIjM0MjM1MjQ3MjQzMDgyMDE5Mzc5NTQyMDUwMjA3MTMyNDg0MTg5OTQ5NjEzODk1OTcxODgxNjk1ODg4Njk5Mzg0MDgyMjI1NjE2MzAiLCI4OTQ1NzMxODIxOTg1NDU0MTcwNDA5MjQ2MjA4MDUxMzYwMzcxNzQ0NjUxOTYwNjg2MjQ5Njg1NjExMDI0NzQ4MjQ5MDk2NjUyNDAwIiwiMSJdLCJwaV9iIjpbWyIxNjE3NzgwNjczNzQ1MTg2MDY3NDk3NTAxNjI1MjExODM3NDU0NDU5NjE5NzcxMDAyNjYzMjAxMTg2MTg4OTM4NDkxMDc3MTU5ODUwIiwiMTAzODU2MDMyMjQzMDc2MDQxMjM5NjI2NzAyMzA3NDcxNDM2NDc2ODAxOTAwNTQzNzU3OTE5NjAxNjYwMTY2NzMyNTcwNjE5ODExNjgiXSxbIjYyMTE1NjA5NTUzMTE1NzgxNDcwMzEzMjAzMTE1MzAyODM5MzgwMDY5MTIyMjAwMTUwODM1MTIxODg2MDI5MzU0MjA4Mjk5MjY1NCIsIjEzODUwMDYzMTA5ODg2MTE3MDcwNzgzMzIxNDk1ODI4MzQ2MTg3ODQ3OTM3OTIyMDMyNTI2NzU3MTU2MDg0MjUzMTU4NTc0NjgyMTYyIl0sWyIxIiwiMCJdXSwicGlfYyI6WyIxMDE0MTM1NTQwNTE1MDg5MzQ1MjY3NTUwMTE4Mjk4MTY1MzY5NDI1MzM0MzMyMjAyNzM1OTgwNTU0NzYxODgyODE4NjU3NDU5MTA4IiwiNDkzMjY5MjI2OTcyOTIyNDQ4NjI1MDI0MDAyMjI4NTQ4NzE1MjYzNTkwNjAzMzA1ODYwNjgwMjI2Nzg4Nzg5NjE2MTU2NTM4MzYiLCIxIl19" - testProof, err := base64.StdEncoding.DecodeString(testProofB64) - require.NoError(t, err) - - zkAuthSig := types.ZKAuthSignature{ - ZkAuthInputs: &types.ZKAuthInputs{ - ProofPoints: testProof, - IssBase64: "aHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29t", - HeaderBase64: "eyJhbGciOiJSUzI1NiIsImtpZCI6ImFkZjVlNzEwZWRmZWJlY2JlZmE5YTYxNDk1NjU0ZDAzYzBiOGVkZjgiLCJ0eXAiOiJKV1QifQ", - AddressSeed: "16929294337693897740349056748881744503581363933815798702166939594477679063350", + testCases := map[string]struct { + from sdk.AccAddress + err error + }{ + "valid signer": { + from: sdk.MustAccAddressFromBech32("link1f7j46mgxr3d5tgn3qsnn9ep8j77yr8w4ks0f3lpzz9rhnwja9vcqej33ld"), + }, + "invalid from": { + from: sdk.MustAccAddressFromBech32("link1apw93nvwxj3t4tvwf6h0n3t84lpyfmswzryxs0z49vlgr8r8xegs03lsnt"), + err: sdkerrors.ErrUnauthorized, }, - MaxBlockHeight: 104, } - msgs := types.NewMsgExecution([]sdk.Msg{&bankMsg}, zkAuthSig) - - execMsgs, err := msgs.GetMessages() - require.NoError(t, err) - // zksigner is assumed to be one - zksigner := msgs.GetSigners()[0] - result, err := k.DispatchMsgs(ctx, execMsgs, zksigner) - - require.NoError(t, err) - require.NotNil(t, result) - - fromBalance := app.BankKeeper.GetBalance(ctx, fromAddr, "stake") - require.True(t, fromBalance.Equal(sdk.NewInt64Coin("stake", 95))) - toBalance := app.BankKeeper.GetBalance(ctx, toAddr, "stake") - require.True(t, toBalance.Equal(sdk.NewInt64Coin("stake", 105))) + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + testApp := testutil.ZkAuthKeeper(t) + app, k, ctx := testApp.Simapp, testApp.ZKAuthKeeper, testApp.Ctx + + addrs := simapp.AddTestAddrs(app, ctx, 1, sdk.NewInt(100)) + + fromAddr := tc.from + toAddr := addrs[0] + err := simapp.FundAccount(app, ctx, fromAddr, sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))) + + newCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 5)) + + bankMsg := banktypes.MsgSend{ + Amount: newCoins, + FromAddress: fromAddr.String(), + ToAddress: toAddr.String(), + } + testProofB64 := "eyJwaV9hIjpbIjM0MjM1MjQ3MjQzMDgyMDE5Mzc5NTQyMDUwMjA3MTMyNDg0MTg5OTQ5NjEzODk1OTcxODgxNjk1ODg4Njk5Mzg0MDgyMjI1NjE2MzAiLCI4OTQ1NzMxODIxOTg1NDU0MTcwNDA5MjQ2MjA4MDUxMzYwMzcxNzQ0NjUxOTYwNjg2MjQ5Njg1NjExMDI0NzQ4MjQ5MDk2NjUyNDAwIiwiMSJdLCJwaV9iIjpbWyIxNjE3NzgwNjczNzQ1MTg2MDY3NDk3NTAxNjI1MjExODM3NDU0NDU5NjE5NzcxMDAyNjYzMjAxMTg2MTg4OTM4NDkxMDc3MTU5ODUwIiwiMTAzODU2MDMyMjQzMDc2MDQxMjM5NjI2NzAyMzA3NDcxNDM2NDc2ODAxOTAwNTQzNzU3OTE5NjAxNjYwMTY2NzMyNTcwNjE5ODExNjgiXSxbIjYyMTE1NjA5NTUzMTE1NzgxNDcwMzEzMjAzMTE1MzAyODM5MzgwMDY5MTIyMjAwMTUwODM1MTIxODg2MDI5MzU0MjA4Mjk5MjY1NCIsIjEzODUwMDYzMTA5ODg2MTE3MDcwNzgzMzIxNDk1ODI4MzQ2MTg3ODQ3OTM3OTIyMDMyNTI2NzU3MTU2MDg0MjUzMTU4NTc0NjgyMTYyIl0sWyIxIiwiMCJdXSwicGlfYyI6WyIxMDE0MTM1NTQwNTE1MDg5MzQ1MjY3NTUwMTE4Mjk4MTY1MzY5NDI1MzM0MzMyMjAyNzM1OTgwNTU0NzYxODgyODE4NjU3NDU5MTA4IiwiNDkzMjY5MjI2OTcyOTIyNDQ4NjI1MDI0MDAyMjI4NTQ4NzE1MjYzNTkwNjAzMzA1ODYwNjgwMjI2Nzg4Nzg5NjE2MTU2NTM4MzYiLCIxIl19" + testProof, err := base64.StdEncoding.DecodeString(testProofB64) + require.NoError(t, err) + + // The zkaddress generated from this signature is link1f7j46mgxr3d5tgn3qsnn9ep8j77yr8w4ks0f3lpzz9rhnwja9vcqej33ld. + zkAuthSig := types.ZKAuthSignature{ + ZkAuthInputs: &types.ZKAuthInputs{ + ProofPoints: testProof, + IssBase64: "aHR0cHM6Ly9hY2NvdW50cy5nb29nbGUuY29t", + HeaderBase64: "eyJhbGciOiJSUzI1NiIsImtpZCI6ImFkZjVlNzEwZWRmZWJlY2JlZmE5YTYxNDk1NjU0ZDAzYzBiOGVkZjgiLCJ0eXAiOiJKV1QifQ", + AddressSeed: "16929294337693897740349056748881744503581363933815798702166939594477679063350", + }, + MaxBlockHeight: 104, + } + msgs := types.NewMsgExecution([]sdk.Msg{&bankMsg}, zkAuthSig) + + execMsgs, err := msgs.GetMessages() + require.NoError(t, err) + // zksigner is assumed to be one + zksigner := msgs.GetSigners()[0] + _, err = k.DispatchMsgs(ctx, execMsgs, zksigner) + require.ErrorIs(t, err, tc.err) + if tc.err != nil { + return + } + + fromBalance := app.BankKeeper.GetBalance(ctx, fromAddr, "stake") + require.True(t, fromBalance.Equal(sdk.NewInt64Coin("stake", 95))) + toBalance := app.BankKeeper.GetBalance(ctx, toAddr, "stake") + require.True(t, toBalance.Equal(sdk.NewInt64Coin("stake", 105))) + }) + } }