-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: custom get signers #16340
feat: custom get signers #16340
Changes from 23 commits
b20a1df
badd826
00eefcf
d8290fe
7a6bc71
a275687
eefa8ae
ccdb3c0
bbfe4aa
d8022de
a44534a
a1db0df
3cac445
c088db2
ee6fa69
ea17470
5dce52b
49a9013
2f4772e
c6af01c
b01d359
49d6ccd
66aedff
9e7a2bb
de3816c
eb72031
9138f2c
410935f
bdbace3
0d6f08f
6955df2
5445754
e3dd1c3
3947ced
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
package tx | ||
|
||
import ( | ||
"testing" | ||
|
||
bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" | ||
"cosmossdk.io/depinject" | ||
"cosmossdk.io/log" | ||
"cosmossdk.io/x/tx/signing" | ||
"github.com/stretchr/testify/require" | ||
|
||
codectypes "github.com/cosmos/cosmos-sdk/codec/types" | ||
"github.com/cosmos/cosmos-sdk/testutil/configurator" | ||
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" | ||
) | ||
|
||
func SetCustomGetSigners(registry codectypes.InterfaceRegistry) { | ||
signing.DefineCustomGetSigners(registry.SigningContext(), func(msg *bankv1beta1.SendAuthorization) ([][]byte, error) { | ||
// arbitrary logic | ||
signer := msg.AllowList[1] | ||
return [][]byte{[]byte(signer)}, nil | ||
}) | ||
} | ||
|
||
func TestDefineCustomGetSigners(t *testing.T) { | ||
var interfaceRegistry codectypes.InterfaceRegistry | ||
_, err := simtestutil.SetupAtGenesis( | ||
depinject.Configs( | ||
configurator.NewAppConfig( | ||
configurator.ParamsModule(), | ||
configurator.AuthModule(), | ||
configurator.StakingModule(), | ||
configurator.BankModule(), | ||
configurator.ConsensusModule(), | ||
), | ||
depinject.Supply(log.NewNopLogger()), | ||
depinject.Invoke(SetCustomGetSigners), | ||
), | ||
&interfaceRegistry, | ||
) | ||
require.NoError(t, err) | ||
require.NotNil(t, interfaceRegistry) | ||
|
||
sendAuth := &bankv1beta1.SendAuthorization{ | ||
AllowList: []string{"foo", "bar"}, | ||
} | ||
signers, err := interfaceRegistry.SigningContext().GetSigners(sendAuth) | ||
require.NoError(t, err) | ||
require.Equal(t, [][]byte{[]byte("bar")}, signers) | ||
|
||
// reset without invoker, no custom signer. | ||
_, err = simtestutil.SetupAtGenesis( | ||
depinject.Configs( | ||
configurator.NewAppConfig( | ||
configurator.ParamsModule(), | ||
configurator.AuthModule(), | ||
configurator.StakingModule(), | ||
configurator.BankModule(), | ||
configurator.ConsensusModule(), | ||
), | ||
depinject.Supply(log.NewNopLogger()), | ||
), | ||
&interfaceRegistry, | ||
) | ||
require.NoError(t, err) | ||
require.NotNil(t, interfaceRegistry) | ||
|
||
_, err = interfaceRegistry.SigningContext().GetSigners(sendAuth) | ||
require.ErrorContains(t, err, "use DefineCustomGetSigners") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,4 +104,4 @@ message Ballot { | |
string voter = 2; | ||
reserved 3; | ||
repeated WeightedBallotOption options = 4; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,9 @@ import ( | |
"google.golang.org/protobuf/reflect/protoreflect" | ||
"google.golang.org/protobuf/reflect/protoregistry" | ||
|
||
msgv1 "cosmossdk.io/api/cosmos/msg/v1" | ||
"cosmossdk.io/core/address" | ||
|
||
msgv1 "cosmossdk.io/api/cosmos/msg/v1" | ||
) | ||
|
||
// Context is a context for retrieving the list of signers from a | ||
|
@@ -85,7 +86,7 @@ type getSignersFunc func(proto.Message) ([][]byte, error) | |
func getSignersFieldNames(descriptor protoreflect.MessageDescriptor) ([]string, error) { | ||
signersFields := proto.GetExtension(descriptor.Options(), msgv1.E_Signer).([]string) | ||
if len(signersFields) == 0 { | ||
return nil, fmt.Errorf("no cosmos.msg.v1.signer option found for message %s", descriptor.FullName()) | ||
return nil, fmt.Errorf("no cosmos.msg.v1.signer option found for message %s; use DefineCustomGetSigners to specify a custom getter", descriptor.FullName()) | ||
} | ||
|
||
return signersFields, nil | ||
|
@@ -316,11 +317,26 @@ func (c *Context) ValidatorAddressCodec() address.Codec { | |
return c.validatorAddressCodec | ||
} | ||
|
||
// FileResolver returns the proto file resolver used by the context. | ||
// FileResolver returns the protobuf file resolver used by the context. | ||
func (c *Context) FileResolver() ProtoFileResolver { | ||
return c.fileResolver | ||
} | ||
|
||
// TypeResolver returns the protobuf type resolver used by the context. | ||
func (c *Context) TypeResolver() protoregistry.MessageTypeResolver { | ||
return c.typeResolver | ||
} | ||
|
||
// DefineCustomGetSigners defines a custom GetSigners function for a given | ||
// message type. It is defined as a function rather than a method on Context | ||
// because of how go generics work. | ||
// | ||
// NOTE: if a custom signers function is defined, the message type used to | ||
// define this function MUST be the concrete type passed to GetSigners, | ||
// otherwise a runtime type error will occur. | ||
func DefineCustomGetSigners[T proto.Message](ctx *Context, getSigners func(T) ([][]byte, error)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this should be done when constructing a context by setting options. We shouldn't really allow this after initialization There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I explored how to do this with depinject, with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we can refactor the API. If that sounds like the right direction, let's first figure out how this works with the signing context options struct and then we can figure out depinject |
||
t := *new(T) | ||
ctx.getSignersFuncs[t.ProtoReflect().Descriptor().FullName()] = func(msg proto.Message) ([][]byte, error) { | ||
return getSigners(msg.(T)) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no validation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With current app wiring and depinject, I couldn't find anyway to call Validate after an invoker is called. If it's called before the app will error during start up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... can we call validate in an invoker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but
DefineCustomGetSigners
is also called in an invoker and order matters, right?