Skip to content

Commit

Permalink
[acl] Modified circular dep check uniqueness to message type/name (s…
Browse files Browse the repository at this point in the history 
…ei-protocol#133)

## Describe your changes and provide context
This modifies the circular dependency checker from contract address
uniqueness to the specific message type and name for the contract
address. This way, we can have a path that goes:
contract A - execute - deposit_funds -> 
contract B - execute - some_message -> 
contract A execute update_user_balance 

and not be called out as a cycle since it goes to a different message
and has potentially different dependencies.

## Testing performed to validate your change
Added unit tests
  • Loading branch information
@udpatil
udpatil authored Jan 12, 2023
1 parent b4b7987 commit 737fb7a
Show file tree
Hide file tree
Showing 2 changed files with 316 additions and 2 deletions.
10 changes: 8 additions & 2 deletions x/accesscontrol/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,16 @@ func (k Keeper) GetRawWasmDependencyMapping(ctx sdk.Context, contractAddress sdk
return &dependencyMapping, nil
}

func GetCircularDependencyIdentifier(contractAddr sdk.AccAddress, msgInfo *types.WasmMessageInfo) string {
separator := ";"
identifier := contractAddr.String() + separator + msgInfo.MessageType.String() + separator + msgInfo.MessageName
return identifier
}

func (k Keeper) GetWasmDependencyAccessOps(ctx sdk.Context, contractAddress sdk.AccAddress, senderBech string, msgInfo *types.WasmMessageInfo, circularDepLookup ContractReferenceLookupMap) ([]acltypes.AccessOperation, error) {
uniqueIdentifier := contractAddress.String()
uniqueIdentifier := GetCircularDependencyIdentifier(contractAddress, msgInfo)
if _, ok := circularDepLookup[uniqueIdentifier]; ok {
// we've already seen this contract, we should simply return synchronous access Ops
// we've already seen this identifier, we should simply return synchronous access Ops
return types.SynchronousAccessOps(), nil
}
// add to our lookup so we know we've seen this identifier
Expand Down
308 changes: 308 additions & 0 deletions x/accesscontrol/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,314 @@ func TestWasmDependencyMappingWithContractReferenceSelectorCircularDependency(t
require.Equal(t, types.NewAccessOperationSet(expectedAccessOps), types.NewAccessOperationSet(deps))
}

func TestWasmDependencyMappingWithContractReferenceNonCircularDependency(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})

wasmContractAddresses := simapp.AddTestAddrsIncremental(app, ctx, 4, sdk.NewInt(30000000))
wasmContractAddress := wasmContractAddresses[0]
interContractAddress := wasmContractAddresses[1]
inter2ContractAddress := wasmContractAddresses[2]
otherAddr := wasmContractAddresses[3]

// create a dummy mapping of a bank balance write for the sender address (eg. performing some action like depositing funds)
// also performs a bank write to an address specified by the JSON body (following same schema as contract A for now)
interContractMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_BANK_BALANCES,
AccessType: acltypes.AccessType_WRITE,
IdentifierTemplate: "02%s",
},
SelectorType: acltypes.AccessOperationSelectorType_SENDER_LENGTH_PREFIXED_ADDRESS,
},
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
BaseContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: inter2ContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "message_b",
},
},
ContractAddress: interContractAddress.String(),
}
// set the dependency mapping
err := app.AccessControlKeeper.SetWasmDependencyMapping(ctx, interContractMapping)
require.NoError(t, err)

// create a dummy mapping of a bank balance write for the sender address (eg. performing some action like depositing funds)
// also performs a bank write to an address specified by the JSON body (following same schema as contract A for now)
inter2ContractMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_ORACLE_EXCHANGE_RATE,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "*",
},
},
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
BaseContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: wasmContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "other_msg",
},
},
ContractAddress: inter2ContractAddress.String(),
}
// set the dependency mapping
err = app.AccessControlKeeper.SetWasmDependencyMapping(ctx, inter2ContractMapping)
require.NoError(t, err)

// this mapping creates a reference to the inter-contract dependency
wasmMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
ExecuteContractReferences: []*acltypes.WasmContractReferences{
{
MessageName: "send",
ContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: interContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "message_a",
},
},
},
},
ExecuteAccessOps: []*acltypes.WasmAccessOperations{
{
MessageName: "other_msg",
WasmOperations: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_STAKING,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "stakingIdentifier",
},
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
},
},
ContractAddress: wasmContractAddress.String(),
}
// set the dependency mapping
err = app.AccessControlKeeper.SetWasmDependencyMapping(ctx, wasmMapping)
require.NoError(t, err)

// test getting the dependency mapping
mapping, err := app.AccessControlKeeper.GetRawWasmDependencyMapping(ctx, wasmContractAddress)
require.NoError(t, err)
require.Equal(t, wasmMapping, *mapping)

// test getting a dependency mapping with selector that expands the inter-contract reference into the contract's dependencies
require.NoError(t, err)
info, _ := types.NewExecuteMessageInfo([]byte(fmt.Sprintf("{\"send\":{\"address\":\"%s\",\"amount\":10}}", otherAddr.String())))
deps, err := app.AccessControlKeeper.GetWasmDependencyAccessOps(
ctx,
wasmContractAddress,
otherAddr.String(),
info,
make(aclkeeper.ContractReferenceLookupMap),
)
require.NoError(t, err)
require.Equal(t, 4, types.NewAccessOperationSet(deps).Size())
expectedAccessOps := []acltypes.AccessOperation{
{
ResourceType: acltypes.ResourceType_KV_BANK_BALANCES,
AccessType: acltypes.AccessType_WRITE,
IdentifierTemplate: fmt.Sprintf("02%s", hex.EncodeToString(address.MustLengthPrefix(wasmContractAddress))),
},
{
ResourceType: acltypes.ResourceType_KV_ORACLE_EXCHANGE_RATE,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "*",
},
{
ResourceType: acltypes.ResourceType_KV_STAKING,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "stakingIdentifier",
},
*types.CommitAccessOp(),
}
require.Equal(t, types.NewAccessOperationSet(expectedAccessOps), types.NewAccessOperationSet(deps))
}

func TestWasmDependencyMappingWithContractReferenceCircularDependencyWithContractOverlap(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})

wasmContractAddresses := simapp.AddTestAddrsIncremental(app, ctx, 4, sdk.NewInt(30000000))
wasmContractAddress := wasmContractAddresses[0]
interContractAddress := wasmContractAddresses[1]
inter2ContractAddress := wasmContractAddresses[2]
otherAddr := wasmContractAddresses[3]

// create a dummy mapping of a bank balance write for the sender address (eg. performing some action like depositing funds)
// also performs a bank write to an address specified by the JSON body (following same schema as contract A for now)
interContractMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_BANK_BALANCES,
AccessType: acltypes.AccessType_WRITE,
IdentifierTemplate: "02%s",
},
SelectorType: acltypes.AccessOperationSelectorType_SENDER_LENGTH_PREFIXED_ADDRESS,
},
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
BaseContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: inter2ContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "message_b",
},
},
ContractAddress: interContractAddress.String(),
}
// set the dependency mapping
err := app.AccessControlKeeper.SetWasmDependencyMapping(ctx, interContractMapping)
require.NoError(t, err)

// create a dummy mapping of a bank balance write for the sender address (eg. performing some action like depositing funds)
// also performs a bank write to an address specified by the JSON body (following same schema as contract A for now)
inter2ContractMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_ORACLE_EXCHANGE_RATE,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "*",
},
},
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
BaseContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: wasmContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "other_msg",
},
},
ContractAddress: inter2ContractAddress.String(),
}
// set the dependency mapping
err = app.AccessControlKeeper.SetWasmDependencyMapping(ctx, inter2ContractMapping)
require.NoError(t, err)

// In this mapping, we will have a cycle that goes through the wasm contract because it goes via a different execute message,
// but will get caught at interContract1 because of `message_a`
wasmMapping := acltypes.WasmDependencyMapping{
BaseAccessOps: []*acltypes.WasmAccessOperation{
{
Operation: types.CommitAccessOp(),
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
ExecuteAccessOps: []*acltypes.WasmAccessOperations{
{
MessageName: "other_msg",
WasmOperations: []*acltypes.WasmAccessOperation{
{
Operation: &acltypes.AccessOperation{
ResourceType: acltypes.ResourceType_KV_STAKING,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "stakingIdentifier",
},
SelectorType: acltypes.AccessOperationSelectorType_NONE,
},
},
},
},
ExecuteContractReferences: []*acltypes.WasmContractReferences{
{
MessageName: "send",
ContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: interContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "message_a",
},
},
},
{
MessageName: "other_msg",
ContractReferences: []*acltypes.WasmContractReference{
{
ContractAddress: interContractAddress.String(),
MessageType: acltypes.WasmMessageSubtype_EXECUTE,
MessageName: "message_a",
},
},
},
},
ContractAddress: wasmContractAddress.String(),
}
// set the dependency mapping
err = app.AccessControlKeeper.SetWasmDependencyMapping(ctx, wasmMapping)
require.NoError(t, err)

// test getting the dependency mapping
mapping, err := app.AccessControlKeeper.GetRawWasmDependencyMapping(ctx, wasmContractAddress)
require.NoError(t, err)
require.Equal(t, wasmMapping, *mapping)

// test getting a dependency mapping with selector that expands the inter-contract reference into the contract's dependencies
require.NoError(t, err)
info, _ := types.NewExecuteMessageInfo([]byte(fmt.Sprintf("{\"send\":{\"address\":\"%s\",\"amount\":10}}", otherAddr.String())))
deps, err := app.AccessControlKeeper.GetWasmDependencyAccessOps(
ctx,
wasmContractAddress,
otherAddr.String(),
info,
make(aclkeeper.ContractReferenceLookupMap),
)
require.NoError(t, err)
require.Equal(t, 5, types.NewAccessOperationSet(deps).Size())
expectedAccessOps := []acltypes.AccessOperation{
{
ResourceType: acltypes.ResourceType_KV_BANK_BALANCES,
AccessType: acltypes.AccessType_WRITE,
IdentifierTemplate: fmt.Sprintf("02%s", hex.EncodeToString(address.MustLengthPrefix(wasmContractAddress))),
},
{
ResourceType: acltypes.ResourceType_KV_ORACLE_EXCHANGE_RATE,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "*",
},
{
ResourceType: acltypes.ResourceType_KV_STAKING,
AccessType: acltypes.AccessType_READ,
IdentifierTemplate: "stakingIdentifier",
},
{AccessType: acltypes.AccessType_UNKNOWN, ResourceType: acltypes.ResourceType_ANY, IdentifierTemplate: "*"},
*types.CommitAccessOp(),
}
require.Equal(t, types.NewAccessOperationSet(expectedAccessOps), types.NewAccessOperationSet(deps))
}

func TestWasmDependencyMappingWithContractReferenceDNE(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})
Expand Down

0 comments on commit 737fb7a

Please sign in to comment.