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.

Sign up for GitHub

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

fix: has relayer permission #279

Merged
merged 4 commits into from
Oct 7, 2024
Merged

fix: has relayer permission #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
727c787
allow all relayers do relaying if not set specifically
beer-1 Oct 7, 2024
66650e2
bump version
beer-1 Oct 7, 2024
7753e63
wrap the condition
beer-1 Oct 7, 2024
31f3ca7
bump opinit version
beer-1 Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
vmtypes "github.com/initia-labs/movevm/types"
)

const upgradeName = "0.5.5"
const upgradeName = "0.5.6"

// RegisterUpgradeHandlers returns upgrade handlers
func (app *InitiaApp) RegisterUpgradeHandlers(cfg module.Configurator) {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
github.com/gorilla/mux v1.8.1
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/hashicorp/go-metrics v0.5.3
github.com/initia-labs/OPinit v0.5.4
github.com/initia-labs/OPinit v0.5.5
// we also need to update `LIBMOVEVM_VERSION` of Dockerfile#9
github.com/initia-labs/movevm v0.5.0
github.com/noble-assets/forwarding/v2 v2.0.0-20240521090705-86712c4c9e43
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,8 @@ github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANyt
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod h1:qj24IKcXYK6Iy9ceXlo3Tc+vtHo9lIhSX5JddghvEPo=
github.com/initia-labs/OPinit v0.5.4 h1:5p/YwEsmKdQdlOl6swULHeENvftqaHfm/bpS+h7+zLg=
github.com/initia-labs/OPinit v0.5.4/go.mod h1:lx1amLMszculwPu8ln+btJno38UV28fd2nP7XC88ZeE=
github.com/initia-labs/OPinit v0.5.5 h1:1AxaZ7NXJThIkDBBvIvsU2RZEqXVSW5nEL0ewU0A1bQ=
github.com/initia-labs/OPinit v0.5.5/go.mod h1:lx1amLMszculwPu8ln+btJno38UV28fd2nP7XC88ZeE=
github.com/initia-labs/OPinit/api v0.5.1 h1:zwyJf7HtKJCKvLJ1R9PjVfJO1L+d/jKoeFyT7WTLHFI=
github.com/initia-labs/OPinit/api v0.5.1/go.mod h1:gHK6DEWb3/DqQD5LjKirUx9jilAh2UioXanoQdgqVfU=
github.com/initia-labs/cometbft v0.0.0-20240923045653-ba99eb347236 h1:+HmPQ1uptOe4r5oQHuHMG5zF1F3maNoEba5uiTUMnlk=
Expand Down
2 changes: 1 addition & 1 deletion x/ibc/perm/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (k Keeper) HasAdminPermission(ctx context.Context, portID, channelID string
// HasRelayerPermission checks if the relayer has permission to relay packets on the channel.
func (k Keeper) HasRelayerPermission(ctx context.Context, portID, channelID string, relayer sdk.AccAddress) (bool, error) {
permRelayers, err := k.ChannelStates.Get(ctx, collections.Join(portID, channelID))
if err != nil && errors.Is(err, collections.ErrNotFound) {
if (err != nil && errors.Is(err, collections.ErrNotFound)) || (err == nil && len(permRelayers.Relayers) == 0) {
// if no permissioned relayers are set, all relayers are allowed
return true, nil
Comment on lines +129 to 131
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Add specific tests and documentation for the updated relayer permission logic

The current test coverage for HasRelayerPermission does not include scenarios where permRelayers exists but has no relayers. Additionally, the documentation lacks detailed explanations of this new conditional logic.

  • Tests Needed:

    • Verify that when permRelayers exists but len(permRelayers.Relayers) == 0, all relayers are allowed.

  • Documentation Enhancements:

    • Explain the reasoning behind allowing all relayers when permRelayers is empty.
    • Detail the implications of this condition to prevent potential security concerns.
🔗 Analysis chain

Clarify the intention behind broadening relayer permission conditions

The conditional logic for determining relayer permissions has been expanded. Now, all relayers are allowed not only when no permissioned relayers are found (error case) but also when the permRelayers object exists but has no relayers.

This change has the following implications:

  1. It broadens the conditions under which all relayers are allowed.
  2. It treats an empty list of permissioned relayers the same as having no permissioned relayers set.

Please clarify:

  1. Is this the intended behavior?
  2. Does this align with the module's specifications or requirements?

Consider adding a comment to explain this logic, for example:

// Allow all relayers if no permissioned relayers are set or if the list of permissioned relayers is empty
if (err != nil && errors.Is(err, collections.ErrNotFound)) || (err == nil && len(permRelayers.Relayers) == 0) {
    return true, nil
}

To ensure this change aligns with the module's intended behavior, please run the following verification script:

This script will help us understand the test coverage, usage, and documentation around the HasRelayerPermission method, which can provide context for whether this change aligns with the overall module design.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage and impact of HasRelayerPermission

# Test 1: Check for any tests related to HasRelayerPermission
echo "Checking for tests related to HasRelayerPermission:"
rg --type go -i "test.*hasrelayerpermission" -A 10

# Test 2: Look for other usages of HasRelayerPermission in the codebase
echo "Checking for other usages of HasRelayerPermission:"
rg --type go "HasRelayerPermission" -A 5

# Test 3: Search for any documentation or comments related to relayer permissions
echo "Searching for documentation or comments about relayer permissions:"
rg --type go -i "relayer.*permission" -A 5

Length of output: 24688

} else if err != nil {
Expand Down
Loading