Skip to content

Commit

Permalink
fix: deadlock when querying group members (backport cosmos#12342) (co…
Browse files Browse the repository at this point in the history 
…smos#12381)
  • Loading branch information
@mergify @JeancarloBarrios
mergify[bot] authored and JeancarloBarrios committed Sep 28, 2024
1 parent d0b6d09 commit 31e79b2
Show file tree
Hide file tree
Showing 5 changed files with 2,165 additions and 90 deletions.
10 changes: 10 additions & 0 deletions x/group/internal/orm/indexer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ func checkUniqueIndexKey(store storetypes.KVStore, secondaryIndexKeyBytes []byte
return nil
}

// checkUniqueIndexKey checks that the given secondary index key is unique
func checkUniqueIndexKey(store sdk.KVStore, secondaryIndexKeyBytes []byte) error {
it := store.Iterator(PrefixRange(secondaryIndexKeyBytes))
defer it.Close()
if it.Valid() {
return errors.ErrORMUniqueConstraint
}
return nil
}

// multiKeyAddFunc allows multiple entries for a key
func multiKeyAddFunc(store storetypes.KVStore, secondaryIndexKey interface{}, rowID RowID) error {
secondaryIndexKeyBytes, err := keyPartBytes(secondaryIndexKey, false)
Expand Down
17 changes: 5 additions & 12 deletions x/group/internal/orm/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,8 @@ func (a table) Has(store storetypes.KVStore, key RowID) bool {
if len(key) == 0 {
return false
}
pStore := prefixstore.New(store, a.prefix[:])
has, err := pStore.Has(key)
if err != nil {
panic(err)
}
return has
pStore := prefix.NewStore(store, a.prefix[:])
return pStore.Has(key)
}

// GetOne load the object persisted for the given RowID into the dest parameter.
Expand Down Expand Up @@ -311,12 +307,9 @@ func (a table) Import(store storetypes.KVStore, data interface{}, _ uint64) erro
return nil
}

func (a table) keys(store storetypes.KVStore) [][]byte {
pStore := prefixstore.New(store, a.prefix[:])
it, err := pStore.ReverseIterator(nil, nil)
if err != nil {
panic(err)
}
func (a table) keys(store sdk.KVStore) [][]byte {
pStore := prefix.NewStore(store, a.prefix[:])
it := pStore.Iterator(nil, nil)
defer it.Close()

var keys [][]byte
Expand Down
8 changes: 2 additions & 6 deletions x/group/internal/orm/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,8 @@ func NewTypeSafeRowGetter(prefixKey [2]byte, model reflect.Type, cdc codec.Codec
return err
}

pStore := prefixstore.New(store, prefixKey[:])
bz, err := pStore.Get(rowID)
if err != nil {
return err
}

pStore := prefix.NewStore(store, prefixKey[:])
bz := pStore.Get(rowID)
if len(bz) == 0 {
return sdkerrors.ErrNotFound
}
Expand Down
94 changes: 23 additions & 71 deletions x/group/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func (k Keeper) GetGroupPolicySeq(ctx sdk.Context) uint64 {
}

// proposalsByVPEnd returns all proposals whose voting_period_end is after the `endTime` time argument.
func (k Keeper) proposalsByVPEnd(ctx context.Context, endTime time.Time) (proposals []group.Proposal, err error) {
func (k Keeper) proposalsByVPEnd(ctx sdk.Context, endTime time.Time) (proposals []group.Proposal, err error) {
timeBytes := sdk.FormatTimeBytes(endTime)
it, err := k.proposalsByVotingPeriodEnd.PrefixScan(k.KVStoreService.OpenKVStore(ctx), nil, timeBytes)
if err != nil {
Expand Down Expand Up @@ -286,7 +286,7 @@ func (k Keeper) pruneProposal(ctx context.Context, proposalID uint64) error {

// abortProposals iterates through all proposals by group policy index
// and marks submitted proposals as aborted.
func (k Keeper) abortProposals(ctx context.Context, groupPolicyAddr sdk.AccAddress) error {
func (k Keeper) abortProposals(ctx sdk.Context, groupPolicyAddr sdk.AccAddress) error {
proposals, err := k.proposalsByGroupPolicy(ctx, groupPolicyAddr)
if err != nil {
return err
Expand All @@ -297,7 +297,7 @@ func (k Keeper) abortProposals(ctx context.Context, groupPolicyAddr sdk.AccAddre
if proposalInfo.Status == group.PROPOSAL_STATUS_SUBMITTED {
proposalInfo.Status = group.PROPOSAL_STATUS_ABORTED

if err := k.proposalTable.Update(k.KVStoreService.OpenKVStore(ctx), proposalInfo.Id, &proposalInfo); err != nil {
if err := k.proposalTable.Update(ctx.KVStore(k.key), proposalInfo.Id, &proposalInfo); err != nil {
return err
}
}
Expand All @@ -306,8 +306,8 @@ func (k Keeper) abortProposals(ctx context.Context, groupPolicyAddr sdk.AccAddre
}

// proposalsByGroupPolicy returns all proposals for a given group policy.
func (k Keeper) proposalsByGroupPolicy(ctx context.Context, groupPolicyAddr sdk.AccAddress) ([]group.Proposal, error) {
proposalIt, err := k.proposalByGroupPolicyIndex.Get(k.KVStoreService.OpenKVStore(ctx), groupPolicyAddr.Bytes())
func (k Keeper) proposalsByGroupPolicy(ctx sdk.Context, groupPolicyAddr sdk.AccAddress) ([]group.Proposal, error) {
proposalIt, err := k.proposalByGroupPolicyIndex.Get(ctx.KVStore(k.key), groupPolicyAddr.Bytes())
if err != nil {
return nil, err
}
Expand All @@ -330,14 +330,14 @@ func (k Keeper) proposalsByGroupPolicy(ctx context.Context, groupPolicyAddr sdk.
}

// pruneVotes prunes all votes for a proposal from state.
func (k Keeper) pruneVotes(ctx context.Context, proposalID uint64) error {
func (k Keeper) pruneVotes(ctx sdk.Context, proposalID uint64) error {
votes, err := k.votesByProposal(ctx, proposalID)
if err != nil {
return err
return nil, err
}

for _, v := range votes {
err = k.voteTable.Delete(k.KVStoreService.OpenKVStore(ctx), &v)
err = k.voteTable.Delete(ctx.KVStore(k.key), &v)
if err != nil {
return err
}
Expand All @@ -347,8 +347,8 @@ func (k Keeper) pruneVotes(ctx context.Context, proposalID uint64) error {
}

// votesByProposal returns all votes for a given proposal.
func (k Keeper) votesByProposal(ctx context.Context, proposalID uint64) ([]group.Vote, error) {
it, err := k.voteByProposalIndex.Get(k.KVStoreService.OpenKVStore(ctx), proposalID)
func (k Keeper) votesByProposal(ctx sdk.Context, proposalID uint64) ([]group.Vote, error) {
it, err := k.voteByProposalIndex.Get(ctx.KVStore(k.key), proposalID)
if err != nil {
return nil, err
}
Expand All @@ -372,9 +372,8 @@ func (k Keeper) votesByProposal(ctx context.Context, proposalID uint64) ([]group
// PruneProposals prunes all proposals that are expired, i.e. whose
// `voting_period + max_execution_period` is greater than the current block
// time.
func (k Keeper) PruneProposals(ctx context.Context) error {
endTime := k.HeaderService.HeaderInfo(ctx).Time.Add(-k.config.MaxExecutionPeriod)
proposals, err := k.proposalsByVPEnd(ctx, endTime)
func (k Keeper) PruneProposals(ctx sdk.Context) error {
proposals, err := k.proposalsByVPEnd(ctx, ctx.BlockTime().Add(-k.config.MaxExecutionPeriod))
if err != nil {
return nil
}
Expand All @@ -383,16 +382,6 @@ func (k Keeper) PruneProposals(ctx context.Context) error {
if err != nil {
return err
}
// Emit event for proposal finalized with its result
if err := k.EventService.EventManager(ctx).Emit(
&group.EventProposalPruned{
ProposalId: proposal.Id,
Status: proposal.Status,
TallyResult: &proposal.FinalTallyResult,
},
); err != nil {
return err
}
}

return nil
Expand All @@ -401,77 +390,40 @@ func (k Keeper) PruneProposals(ctx context.Context) error {
// TallyProposalsAtVPEnd iterates over all proposals whose voting period
// has ended, tallies their votes, prunes them, and updates the proposal's
// `FinalTallyResult` field.
func (k Keeper) TallyProposalsAtVPEnd(ctx context.Context) error {
proposals, err := k.proposalsByVPEnd(ctx, k.HeaderService.HeaderInfo(ctx).Time)
func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
proposals, err := k.proposalsByVPEnd(ctx, ctx.BlockTime())
if err != nil {
return nil
}
for _, proposal := range proposals {
policyInfo, err := k.getGroupPolicyInfo(ctx, proposal.GroupPolicyAddress)
if err != nil {
return errorsmod.Wrap(err, "group policy")
return sdkerrors.Wrap(err, "group policy")
}

electorate, err := k.getGroupInfo(ctx, policyInfo.GroupId)
if err != nil {
return errorsmod.Wrap(err, "group")
return sdkerrors.Wrap(err, "group")
}

proposalID := proposal.Id
if proposal.Status == group.PROPOSAL_STATUS_ABORTED || proposal.Status == group.PROPOSAL_STATUS_WITHDRAWN {
proposalID := proposal.Id
if err := k.pruneProposal(ctx, proposalID); err != nil {
return err
}
if err := k.pruneVotes(ctx, proposalID); err != nil {
return err
}
// Emit event for proposal finalized with its result
if err := k.EventService.EventManager(ctx).Emit(
&group.EventProposalPruned{
ProposalId: proposal.Id,
Status: proposal.Status,
},
); err != nil {
return err
}
} else if proposal.Status == group.PROPOSAL_STATUS_SUBMITTED {
if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil {
return errorsmod.Wrap(err, "doTallyAndUpdate")
} else {
err = k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo)
if err != nil {
return sdkerrors.Wrap(err, "doTallyAndUpdate")
}

if err := k.proposalTable.Update(k.KVStoreService.OpenKVStore(ctx), proposal.Id, &proposal); err != nil {
return errorsmod.Wrap(err, "proposal update")
if err := k.proposalTable.Update(ctx.KVStore(k.key), proposal.Id, &proposal); err != nil {
return sdkerrors.Wrap(err, "proposal update")
}
}
// Note: We do nothing if the proposal has been marked as ACCEPTED or
// REJECTED.
}
return nil
}

// assertMetadataLength returns an error if given metadata length
// is greater than defined MaxMetadataLen in the module configuration
func (k Keeper) assertMetadataLength(metadata, description string) error {
if uint64(len(metadata)) > k.config.MaxMetadataLen {
return errors.ErrMetadataTooLong.Wrap(description)
}
return nil
}

// assertSummaryLength returns an error if given summary length
// is greater than defined MaxProposalSummaryLen in the module configuration
func (k Keeper) assertSummaryLength(summary string) error {
if uint64(len(summary)) > k.config.MaxProposalSummaryLen {
return errors.ErrSummaryTooLong
}
return nil
}

// assertTitleLength returns an error if given summary length
// is greater than defined MaxProposalTitleLen in the module configuration
func (k Keeper) assertTitleLength(title string) error {
if uint64(len(title)) > k.config.MaxProposalTitleLen {
return errors.ErrTitleTooLong
}
return nil
}
Loading

0 comments on commit 31e79b2

Please sign in to comment.