Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: audit #354

Merged
merged 9 commits into from
Mar 4, 2025
9 changes: 2 additions & 7 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ on:
- "**.go"
- "go.mod"
- "go.sum"
pull_request:
branches:
- "main"
paths:
- "**.go"
- "go.mod"
- "go.sum"

env:
REGISTRY: ghcr.io
Expand Down Expand Up @@ -61,6 +54,8 @@ jobs:
push: ${{ startsWith(github.ref, 'refs/tags') }} # push image only for tags
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
env:
VERSION: ${{ github.ref_name }}

node:
if: ${{ startsWith(github.ref, 'refs/tags/') }}
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/keeper/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (k Keeper) AllocateTokens(ctx context.Context, totalPreviousPower int64, bo

// allocate rewards proportionally to reward power
for _, rewardWeight := range rewardWeights {
poolFraction := rewardWeight.Weight.Quo(weightsSum)
poolFraction := rewardWeight.Weight.QuoTruncate(weightsSum)
poolReward := feesCollected.MulDecTruncate(voteMultiplier).MulDecTruncate(poolFraction)

poolDenom := rewardWeight.Denom
Expand Down
9 changes: 8 additions & 1 deletion x/distribution/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,14 @@
if balances.IsZero() {
k.authKeeper.SetModuleAccount(ctx, moduleAcc)
}
if !balances.Equal(moduleHoldingsInt) {

// It is hard to block fungible asset transfer from move side,
// so we can't guarantee the ModuleAccount is always equal to the sum
// of the distribution rewards and community pool. Instead, we decide to check
// if the ModuleAccount balance is greater than or equal to the sum of
// the distribution rewards and community pool.
broken := !balances.IsAllGTE(moduleHoldingsInt)
if broken {

Check warning on line 132 in x/distribution/keeper/genesis.go

View check run for this annotation

Codecov / codecov/patch

x/distribution/keeper/genesis.go#L131-L132

Added lines #L131 - L132 were not covered by tests
panic(fmt.Sprintf("distribution module balance does not match the module holdings: %s <-> %s", balances, moduleHoldingsInt))
}
}
Expand Down
16 changes: 15 additions & 1 deletion x/evidence/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import (
"context"

"cosmossdk.io/errors"
"cosmossdk.io/x/evidence/types"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

type msgServer struct {
Expand All @@ -21,9 +24,20 @@

// SubmitEvidence implements the MsgServer.SubmitEvidence method.
func (ms msgServer) SubmitEvidence(goCtx context.Context, msg *types.MsgSubmitEvidence) (*types.MsgSubmitEvidenceResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
if _, err := ms.addressCodec.StringToBytes(msg.Submitter); err != nil {
return nil, sdkerrors.ErrInvalidAddress.Wrapf("invalid submitter address: %s", err)
}

Check warning on line 29 in x/evidence/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/evidence/keeper/msg_server.go#L27-L29

Added lines #L27 - L29 were not covered by tests

evidence := msg.GetEvidence()
if evidence == nil {
return nil, errors.Wrap(types.ErrInvalidEvidence, "missing evidence")
}

Check warning on line 34 in x/evidence/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/evidence/keeper/msg_server.go#L32-L34

Added lines #L32 - L34 were not covered by tests

if err := evidence.ValidateBasic(); err != nil {
return nil, errors.Wrapf(types.ErrInvalidEvidence, "failed basic validation: %s", err)
}

Check warning on line 38 in x/evidence/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/evidence/keeper/msg_server.go#L36-L38

Added lines #L36 - L38 were not covered by tests

ctx := sdk.UnwrapSDKContext(goCtx)

Check warning on line 40 in x/evidence/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/evidence/keeper/msg_server.go#L40

Added line #L40 was not covered by tests
if err := ms.Keeper.SubmitEvidence(ctx, evidence); err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions x/ibc-hooks/move-hooks/receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (h MoveHooks) onRecvIcs20Packet(
data transfertypes.FungibleTokenPacketData,
) ibcexported.Acknowledgement {
isMoveRouted, hookData, err := validateAndParseMemo(data.GetMemo())
if !isMoveRouted || hookData.Message == nil {
if !isMoveRouted || (err == nil && hookData.Message == nil) {
return im.App.OnRecvPacket(ctx, packet, relayer)
} else if err != nil {
return newEmitErrorAcknowledgement(err)
Expand Down Expand Up @@ -74,7 +74,7 @@ func (h MoveHooks) onRecvIcs721Packet(
data nfttransfertypes.NonFungibleTokenPacketData,
) ibcexported.Acknowledgement {
isMoveRouted, hookData, err := validateAndParseMemo(data.GetMemo())
if !isMoveRouted || hookData.Message == nil {
if !isMoveRouted || (err == nil && hookData.Message == nil) {
return im.App.OnRecvPacket(ctx, packet, relayer)
} else if err != nil {
return newEmitErrorAcknowledgement(err)
Expand Down
18 changes: 17 additions & 1 deletion x/intertx/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@
icaMsg := icacontrollertypes.NewMsgRegisterInterchainAccount(msg.ConnectionId, msg.Owner, msg.Version)
icaMsg.Ordering = msg.Ordering

if _, err := k.icaControllerMsgServer.RegisterInterchainAccount(ctx, icaMsg); err != nil {
// validate the icaMsg
err := icaMsg.ValidateBasic()
if err != nil {
return nil, err
}

Check warning on line 44 in x/intertx/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/intertx/keeper/msg_server.go#L43-L44

Added lines #L43 - L44 were not covered by tests

// send the icaMsg to the ica controller
_, err = k.icaControllerMsgServer.RegisterInterchainAccount(ctx, icaMsg)
if err != nil {
return nil, err
}

Expand All @@ -64,6 +72,14 @@

relativeTimeoutTimestamp := uint64((time.Minute * 5).Nanoseconds())
icaMsg := icacontrollertypes.NewMsgSendTx(msg.Owner, msg.ConnectionId, uint64(relativeTimeoutTimestamp), packetData)

// validate the icaMsg
err = icaMsg.ValidateBasic()
if err != nil {
return nil, err
}

Check warning on line 80 in x/intertx/keeper/msg_server.go

View check run for this annotation

Codecov / codecov/patch

x/intertx/keeper/msg_server.go#L79-L80

Added lines #L79 - L80 were not covered by tests

// send the icaMsg to the ica controller
_, err = k.icaControllerMsgServer.SendTx(ctx, icaMsg)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion x/move/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (k Keeper) Initialize(
}
}

return k.handleExecuteResponse(sdkCtx, sdkCtx.GasMeter(), execRes)
return nil
}

// InitGenesis sets supply information for genesis.
Expand Down
27 changes: 15 additions & 12 deletions x/move/keeper/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,13 @@
ctx, commit := parentCtx.CacheContext()
defer func() {
if r := recover(); r != nil {
err = fmt.Errorf("panic: %v", r)
switch r.(type) {
case storetypes.ErrorOutOfGas:
// propagate out of gas error
panic(r)
default:
err = fmt.Errorf("panic: %v", r)

Check warning on line 399 in x/move/keeper/handler.go

View check run for this annotation

Codecov / codecov/patch

x/move/keeper/handler.go#L394-L399

Added lines #L394 - L399 were not covered by tests
}
}

success := err == nil
Expand Down Expand Up @@ -580,28 +586,25 @@
return vmtypes.ViewOutput{}, 0, err
}

executionCounter, err := k.ExecutionCounter.Next(ctx)
ac := types.NextAccountNumber(ctx, k.authKeeper)
ec, err := k.ExecutionCounter.Next(ctx)
if err != nil {
return vmtypes.ViewOutput{}, 0, err
}

api := NewApi(k, ctx)
env := types.NewEnv(
ctx,
types.NextAccountNumber(ctx, k.authKeeper),
executionCounter,
)

sdkCtx := sdk.UnwrapSDKContext(ctx)
gasMeter := sdkCtx.GasMeter()
gasForRuntime := k.computeGasForRuntime(ctx, gasMeter)

// delegate gas metering to move vm
sdkCtx = sdkCtx.WithGasMeter(storetypes.NewInfiniteGasMeter())

gasBalance := gasForRuntime
viewRes, err := k.initiaMoveVM.ExecuteViewFunction(
&gasBalance,
types.NewVMStore(ctx, k.VMStore),
api,
env,
types.NewVMStore(sdkCtx, k.VMStore),
NewApi(k, sdkCtx),
types.NewEnv(sdkCtx, ac, ec),
payload,
)

Expand Down
2 changes: 1 addition & 1 deletion x/move/types/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@
timestampBefore, timestampAfter, timestamp math.Int,
) (math.LegacyDec, math.LegacyDec, error) {
if timestampBefore.GT(timestamp) {
return math.LegacyZeroDec(), math.LegacyZeroDec(), ErrInvalidDexConfig
return weightCoinABefore, weightCoinBBefore, nil

Check warning on line 415 in x/move/types/connector.go

View check run for this annotation

Codecov / codecov/patch

x/move/types/connector.go#L415

Added line #L415 was not covered by tests
}

if timestamp.GTE(timestampAfter) {
Expand Down
Loading