Skip to content

Commit

Permalink
fix: [quantstamp-20] clear plugin data early in uninstallPlugin (#25)…
Browse files Browse the repository at this point in the history
… and clean github lint action
  • Loading branch information
fangting-alchemy authored Dec 21, 2023
1 parent ef789cd commit 10330fe
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
24 changes: 10 additions & 14 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,15 @@ jobs:
name: Run Linters
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly-5be158ba6dc7c798a6f032026fe60fc01686b33b

- name: "Check out the repo"
uses: "actions/checkout@v3"
with:
submodules: "recursive"

- name: "Install Foundry"
uses: "foundry-rs/foundry-toolchain@v1"
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly-5be158ba6dc7c798a6f032026fe60fc01686b33b

- name: "Install Pnpm"
uses: "pnpm/action-setup@v2"
Expand All @@ -37,16 +33,14 @@ jobs:
cache: "pnpm"
node-version: "lts/*"

- name: "Install the Node.js dependencies"
- name: "Install Node.js dependencies"
run: "pnpm install"

- run: forge install

- run: forge fmt --check

- name: "Lint the contracts"
run: "pnpm lint"

# check-inspect:
# name: Verify Inspections
# runs-on: ubuntu-latest
Expand All @@ -70,7 +64,8 @@ jobs:
name: Run Forge Tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: "Check out the repo"
uses: actions/checkout@v3
with:
submodules: recursive

Expand All @@ -92,7 +87,8 @@ jobs:
name: Run Forge Tests [lite build]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: "Check out the repo"
uses: actions/checkout@v3
with:
submodules: recursive

Expand Down
30 changes: 15 additions & 15 deletions src/account/PluginManagerInternals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -651,18 +651,20 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
revert PluginNotInstalled(args.plugin);
}

PluginData memory pluginData = storage_.pluginData[args.plugin];

// Check manifest hash.
if (!_isValidPluginManifest(args.manifest, storage_.pluginData[args.plugin].manifestHash)) {
if (!_isValidPluginManifest(args.manifest, pluginData.manifestHash)) {
revert InvalidPluginManifest();
}

// Ensure that there are no dependent plugins.
if (storage_.pluginData[args.plugin].dependentCount != 0) {
if (pluginData.dependentCount != 0) {
revert PluginDependencyViolation(args.plugin);
}

// Remove this plugin as a dependent from its dependencies.
FunctionReference[] memory dependencies = storage_.pluginData[args.plugin].dependencies;
FunctionReference[] memory dependencies = pluginData.dependencies;
uint256 length = dependencies.length;
for (uint256 i = 0; i < length;) {
FunctionReference dependency = dependencies[i];
Expand All @@ -676,6 +678,9 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

// Remove the plugin metadata from the account.
delete storage_.pluginData[args.plugin];

// Remove components according to the manifest, in reverse order (by component type) of their installation.

// Remove pre and post permitted call hooks
Expand Down Expand Up @@ -786,11 +791,9 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

// Remove permitted external call permissions
if (args.manifest.permitAnyExternalAddress) {
// Only clear if it was set during install time
storage_.pluginData[args.plugin].anyExternalAddressPermitted = false;
} else {
// Remove permitted external call permissions, anyExternalAddressPermitted is cleared when pluginData being
// deleted
if (!args.manifest.permitAnyExternalAddress) {
// Only clear the specific permitted external calls if "permit any" flag was not set.
length = args.manifest.permittedExternalCalls.length;
for (uint256 i = 0; i < length;) {
Expand Down Expand Up @@ -823,9 +826,9 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}

// Remove injected hooks
length = storage_.pluginData[args.plugin].injectedHooks.length;
length = pluginData.injectedHooks.length;
for (uint256 i = 0; i < length;) {
StoredInjectedHook memory hook = storage_.pluginData[args.plugin].injectedHooks[i];
StoredInjectedHook memory hook = pluginData.injectedHooks[i];

// Decrement the dependent count for the plugin providing the hook.
storage_.pluginData[hook.providingPlugin].dependentCount -= 1;
Expand Down Expand Up @@ -877,13 +880,13 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {

// Call onHookUnapply on all injected hooks
bool callbacksSucceeded = true;
length = storage_.pluginData[args.plugin].injectedHooks.length;
length = pluginData.injectedHooks.length;
bool hasUnapplyHookData = hookUnapplyData.length != 0;
if (hasUnapplyHookData && hookUnapplyData.length != length) {
revert ArrayLengthMismatch();
}
for (uint256 i = 0; i < length;) {
StoredInjectedHook memory hook = storage_.pluginData[args.plugin].injectedHooks[i];
StoredInjectedHook memory hook = pluginData.injectedHooks[i];

/* solhint-disable no-empty-blocks */
try IPlugin(hook.providingPlugin).onHookUnapply{gas: args.callbackGasLimit}(
Expand All @@ -908,9 +911,6 @@ abstract contract PluginManagerInternals is IPluginManager, AccountStorageV1 {
}
}

// Remove the plugin metadata from the account.
delete storage_.pluginData[args.plugin];

// Clear the plugin storage for the account.
// solhint-disable-next-line no-empty-blocks
try IPlugin(args.plugin).onUninstall{gas: args.callbackGasLimit}(uninstallData) {}
Expand Down

0 comments on commit 10330fe

Please sign in to comment.