Skip to content

Commit

Permalink
fix(x/tx): concurrent map writes when calling GetSigners (#21073)
Browse files Browse the repository at this point in the history
(cherry picked from commit de0708b)
  • Loading branch information
facundomedica authored and mergify[bot] committed Jul 31, 2024
1 parent 1b27e4f commit dae01c6
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
4 changes: 4 additions & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,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)

Expand Down
4 changes: 4 additions & 0 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions x/tx/signing/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package signing
import (
"errors"
"fmt"
"sync"

cosmos_proto "github.com/cosmos/cosmos-proto"
gogoproto "github.com/cosmos/gogoproto/proto"
Expand All @@ -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
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions x/tx/signing/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down

0 comments on commit dae01c6

Please sign in to comment.