From de0708b40171225d00fc3fe59717efd3280c20c2 Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Wed, 31 Jul 2024 17:15:17 +0200 Subject: [PATCH] fix(x/tx): concurrent map writes when calling GetSigners (#21073) --- simapp/app.go | 4 ++++ x/tx/CHANGELOG.md | 4 ++++ x/tx/signing/context.go | 12 ++++++++---- x/tx/signing/context_test.go | 15 +++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index 092435e4781c..6c2424e29c60 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -214,6 +214,10 @@ func NewSimApp( signingCtx := interfaceRegistry.SigningContext() txConfig := authtx.NewTxConfig(appCodec, signingCtx.AddressCodec(), signingCtx.ValidatorAddressCodec(), authtx.DefaultSignModes) + if err := signingCtx.Validate(); err != nil { + panic(err) + } + std.RegisterLegacyAminoCodec(legacyAmino) std.RegisterInterfaces(interfaceRegistry) diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index af095740513d..4a13d399cab3 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -33,6 +33,10 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ## [Unreleased] +### Improvements + +* [#21073](https://github.com/cosmos/cosmos-sdk/pull/21073) In Context use sync.Map `getSignersFuncs` map from concurrent writes, we also call Validate when creating the Context. + ## [v0.13.3](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.3) - 2024-04-22 ### Improvements diff --git a/x/tx/signing/context.go b/x/tx/signing/context.go index 7aca6fd2b1cd..f44be0118cca 100644 --- a/x/tx/signing/context.go +++ b/x/tx/signing/context.go @@ -3,6 +3,7 @@ package signing import ( "errors" "fmt" + "sync" cosmos_proto "github.com/cosmos/cosmos-proto" gogoproto "github.com/cosmos/gogoproto/proto" @@ -29,7 +30,7 @@ type Context struct { typeResolver protoregistry.MessageTypeResolver addressCodec address.Codec validatorAddressCodec address.Codec - getSignersFuncs map[protoreflect.FullName]GetSignersFunc + getSignersFuncs sync.Map customGetSignerFuncs map[protoreflect.FullName]GetSignersFunc maxRecursionDepth int } @@ -110,7 +111,7 @@ func NewContext(options Options) (*Context, error) { typeResolver: protoTypes, addressCodec: options.AddressCodec, validatorAddressCodec: options.ValidatorAddressCodec, - getSignersFuncs: map[protoreflect.FullName]GetSignersFunc{}, + getSignersFuncs: sync.Map{}, customGetSignerFuncs: customGetSignerFuncs, maxRecursionDepth: options.MaxRecursionDepth, } @@ -334,14 +335,17 @@ func (c *Context) getGetSignersFn(messageDescriptor protoreflect.MessageDescript if ok { return f, nil } - f, ok = c.getSignersFuncs[messageDescriptor.FullName()] + + loadedFn, ok := c.getSignersFuncs.Load(messageDescriptor.FullName()) if !ok { var err error f, err = c.makeGetSignersFunc(messageDescriptor) if err != nil { return nil, err } - c.getSignersFuncs[messageDescriptor.FullName()] = f + c.getSignersFuncs.Store(messageDescriptor.FullName(), f) + } else { + f = loadedFn.(GetSignersFunc) } return f, nil diff --git a/x/tx/signing/context_test.go b/x/tx/signing/context_test.go index d8c2b370333a..88f0bf4aadf9 100644 --- a/x/tx/signing/context_test.go +++ b/x/tx/signing/context_test.go @@ -51,6 +51,21 @@ var deeplyNestedRepeatedSigner = &testpb.DeeplyNestedRepeatedSigner{ }, } +func TestGetGetSignersFnConcurrent(t *testing.T) { + ctx, err := NewContext(Options{ + AddressCodec: dummyAddressCodec{}, + ValidatorAddressCodec: dummyValidatorAddressCodec{}, + }) + require.NoError(t, err) + + desc := (&testpb.RepeatedSigner{}).ProtoReflect().Descriptor() + for i := 0; i < 50; i++ { + go func() { + _, _ = ctx.getGetSignersFn(desc) + }() + } +} + func TestGetSigners(t *testing.T) { ctx, err := NewContext(Options{ AddressCodec: dummyAddressCodec{},