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)

# Conflicts:
#	x/tx/CHANGELOG.md
#	x/tx/signing/context_test.go
  • Loading branch information
facundomedica authored and mergify[bot] committed Jul 31, 2024
1 parent 91d412c commit 30e3aaa
Show file tree
Hide file tree
Showing 4 changed files with 151 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 @@ -209,6 +209,10 @@ func NewSimApp(
legacyAmino := codec.NewLegacyAmino()
txConfig := tx.NewTxConfig(appCodec, tx.DefaultSignModes)

if err := signingCtx.Validate(); err != nil {
panic(err)
}

std.RegisterLegacyAminoCodec(legacyAmino)
std.RegisterInterfaces(interfaceRegistry)

Expand Down
84 changes: 84 additions & 0 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,90 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

<<<<<<< HEAD
=======
### 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

* [#20049](https://github.com/cosmos/cosmos-sdk/pull/20049) Sort JSON attributes for `inline_json` encoder.

## [v0.13.2](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.2) - 2024-04-12

### Features

* [#19786](https://github.com/cosmos/cosmos-sdk/pull/19786)/[#19919](https://github.com/cosmos/cosmos-sdk/pull/19919) Add "inline_json" option to Amino JSON encoder.

### Improvements

* [#19845](https://github.com/cosmos/cosmos-sdk/pull/19845) Use hybrid resolver instead of only protov2 registry

### Bug Fixes

* [#19955](https://github.com/cosmos/cosmos-sdk/pull/19955) Don't shadow Amino marshalling error messages

## [v0.13.1](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.1) - 2024-03-05

### Features

* [#19618](https://github.com/cosmos/cosmos-sdk/pull/19618) Add enum as string option to encoder.

### Improvements

* [#18857](https://github.com/cosmos/cosmos-sdk/pull/18857) Moved `FormatCoins` from `core/coins` to this package under `signing/textual`.

### Bug Fixes

* [#19265](https://github.com/cosmos/cosmos-sdk/pull/19265) Reject denoms that contain a comma.

## [v0.13.0](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.0) - 2023-12-19

### Improvements

* [#18740](https://github.com/cosmos/cosmos-sdk/pull/18740) Support nested messages when fetching signers up to a default depth of 32.

## v0.12.0

### Improvements

* [#18309](https://github.com/cosmos/cosmos-sdk/pull/18309) Update encoder so that amino types default to msg type url.

## v0.11.0

### Improvements

* [#17787](https://github.com/cosmos/cosmos-sdk/pull/17787) Drop tip support.

## v0.10.0

### Features

* [#17681](https://github.com/cosmos/cosmos-sdk/pull/17681) Add encoder `DefineTypeEncoding` method for defining custom type encodings.
* [#17600](https://github.com/cosmos/cosmos-sdk/pull/17600) Add encoder `DefineScalarEncoding` method for defining custom scalar encodings.
* [#17600](https://github.com/cosmos/cosmos-sdk/pull/17600) Add indent option to encoder.

## v0.9.1

### Improvements

* [#16936](https://github.com/cosmos/cosmos-sdk/pull/16936) Remove extra whitespace when marshalling module accounts.

## v0.9.0

### Bug Fixes

* [#16681](https://github.com/cosmos/cosmos-sdk/pull/16681): Catch and fix `(*Decoder).Decode` crash from invalid length prefix in Tx bytes.

### Improvements

* [#16846](https://github.com/cosmos/cosmos-sdk/pull/16846): Harmonize interface `signing.TypeResolver` with the rest of the codebase (orm and client/v2).
* [#16684](https://github.com/cosmos/cosmos-sdk/pull/16684): Use `io.WriteString`+`fmt.Fprintf` to remove unnecessary `string`->`[]byte` roundtrip.

>>>>>>> de0708b40 (fix(x/tx): concurrent map writes when calling GetSigners (#21073))
## v0.8.0

### 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"
"google.golang.org/protobuf/proto"
Expand All @@ -23,7 +24,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
}

Expand Down Expand Up @@ -95,7 +96,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,
}

Expand Down Expand Up @@ -323,14 +324,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
55 changes: 55 additions & 0 deletions x/tx/signing/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,61 @@ import (
"cosmossdk.io/x/tx/internal/testpb"
)

<<<<<<< HEAD
=======
var deeplyNestedRepeatedSigner = &testpb.DeeplyNestedRepeatedSigner{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner{
{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner_Inner{
{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner_Inner_Bottom{
{
Signer: []string{hex.EncodeToString([]byte("foo")), hex.EncodeToString([]byte("bar"))},
},
},
},
},
},
{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner_Inner{
{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner_Inner_Bottom{
{
Signer: []string{hex.EncodeToString([]byte("baz"))},
},
},
},
{
Inner: []*testpb.DeeplyNestedRepeatedSigner_Inner_Inner_Bottom{
{
Signer: []string{hex.EncodeToString([]byte("qux")), hex.EncodeToString([]byte("fuz"))},
},
{
Signer: []string{hex.EncodeToString([]byte("bing")), hex.EncodeToString([]byte("bap"))},
},
},
},
},
},
},
}

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)
}()
}
}

>>>>>>> de0708b40 (fix(x/tx): concurrent map writes when calling GetSigners (#21073))
func TestGetSigners(t *testing.T) {
ctx, err := NewContext(Options{
AddressCodec: dummyAddressCodec{},
Expand Down

0 comments on commit 30e3aaa

Please sign in to comment.