Skip to content

Commit 710063b

Browse files
facundomedicajulienrbrt
authored andcommitted
fix(x/tx): concurrent map writes when calling GetSigners (#21073)
1 parent 0aff4f2 commit 710063b

4 files changed

Lines changed: 31 additions & 4 deletions

File tree

simapp/app.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,10 @@ func NewSimApp(
215215
signingCtx := interfaceRegistry.SigningContext()
216216
txConfig := authtx.NewTxConfig(appCodec, signingCtx.AddressCodec(), signingCtx.ValidatorAddressCodec(), authtx.DefaultSignModes)
217217

218+
if err := signingCtx.Validate(); err != nil {
219+
panic(err)
220+
}
221+
218222
std.RegisterLegacyAminoCodec(legacyAmino)
219223
std.RegisterInterfaces(interfaceRegistry)
220224

x/tx/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos-
3333

3434
## [Unreleased]
3535

36+
### Improvements
37+
38+
* [#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.
39+
3640
## [v0.13.3](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.3) - 2024-04-22
3741

3842
### Improvements

x/tx/signing/context.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package signing
33
import (
44
"errors"
55
"fmt"
6+
"sync"
67

78
cosmos_proto "github.com/cosmos/cosmos-proto"
89
gogoproto "github.com/cosmos/gogoproto/proto"
@@ -29,7 +30,7 @@ type Context struct {
2930
typeResolver protoregistry.MessageTypeResolver
3031
addressCodec address.Codec
3132
validatorAddressCodec address.Codec
32-
getSignersFuncs map[protoreflect.FullName]GetSignersFunc
33+
getSignersFuncs sync.Map
3334
customGetSignerFuncs map[protoreflect.FullName]GetSignersFunc
3435
maxRecursionDepth int
3536
}
@@ -110,7 +111,7 @@ func NewContext(options Options) (*Context, error) {
110111
typeResolver: protoTypes,
111112
addressCodec: options.AddressCodec,
112113
validatorAddressCodec: options.ValidatorAddressCodec,
113-
getSignersFuncs: map[protoreflect.FullName]GetSignersFunc{},
114+
getSignersFuncs: sync.Map{},
114115
customGetSignerFuncs: customGetSignerFuncs,
115116
maxRecursionDepth: options.MaxRecursionDepth,
116117
}
@@ -334,14 +335,17 @@ func (c *Context) getGetSignersFn(messageDescriptor protoreflect.MessageDescript
334335
if ok {
335336
return f, nil
336337
}
337-
f, ok = c.getSignersFuncs[messageDescriptor.FullName()]
338+
339+
loadedFn, ok := c.getSignersFuncs.Load(messageDescriptor.FullName())
338340
if !ok {
339341
var err error
340342
f, err = c.makeGetSignersFunc(messageDescriptor)
341343
if err != nil {
342344
return nil, err
343345
}
344-
c.getSignersFuncs[messageDescriptor.FullName()] = f
346+
c.getSignersFuncs.Store(messageDescriptor.FullName(), f)
347+
} else {
348+
f = loadedFn.(GetSignersFunc)
345349
}
346350

347351
return f, nil

x/tx/signing/context_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,21 @@ var deeplyNestedRepeatedSigner = &testpb.DeeplyNestedRepeatedSigner{
5151
},
5252
}
5353

54+
func TestGetGetSignersFnConcurrent(t *testing.T) {
55+
ctx, err := NewContext(Options{
56+
AddressCodec: dummyAddressCodec{},
57+
ValidatorAddressCodec: dummyValidatorAddressCodec{},
58+
})
59+
require.NoError(t, err)
60+
61+
desc := (&testpb.RepeatedSigner{}).ProtoReflect().Descriptor()
62+
for i := 0; i < 50; i++ {
63+
go func() {
64+
_, _ = ctx.getGetSignersFn(desc)
65+
}()
66+
}
67+
}
68+
5469
func TestGetSigners(t *testing.T) {
5570
ctx, err := NewContext(Options{
5671
AddressCodec: dummyAddressCodec{},

0 commit comments

Comments
 (0)