[API] Add filter support for view and simulate #11666

banool · 2024-01-16T20:21:53Z

Description

This is one short term solution to the overall problem of nodes being overwhelmed by computationally expensive functions to the point where they can't even state sync.

Test Plan

View Functions

Start local testnet:

cargo run -p aptos -- node run-local-testnet --force-restart --assume-yes --with-indexer-api

Stop it, add 0x1::coin::balance to the config in the blocklist:

  view_filter:
    blocklist:
      - address: "0x1"
        module: "coin"
        function_name: "balance"

Start local testnet again:

cargo run -p aptos -- node run-local-testnet --with-indexer-api

Try to call view function from the explorer, get this response:

Error: {"message":"Function 0000000000000000000000000000000000000000000000000000000000000001::coin::balance is not allowed","error_code":"invalid_input","vm_error_code":null}

Other view functions work fine.

I also tested allowlist and the no list and it worked as expected.

Simulation

Same procedure at first as above but we add this config:

  simulation_filter:
    rules:
      - Deny:
          EntryFunction:
            - "0000000000000000000000000000000000000000000000000000000000000001"
            - vesting
            - vest

Calling the function with the CLI you get this error:

{
  "Error": "API error: API error Error(InvalidInput): Transaction not allowed by simulation filter"
}

trunk-io · 2024-01-16T20:21:57Z

⏱️ 8h 55m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-unit-tests	1h 35m	🟩 ⬜ ⬜ 🟩 🟩
forge-framework-upgrade-test / forge	1h 19m	🟥 🟥
windows-build	1h 17m	🟩 🟥 🟩 🟩 🟩
rust-smoke-tests	1h 2m	🟩 🟩
execution-performance / single-node-performance	42m	🟩 🟩
forge-compat-test / forge	33m	🟩 🟩
forge-e2e-test / forge	26m	🟩 🟩
rust-lints	23m	🟥 ⬜ ⬜ 🟩 🟩
rust-images / rust-all	18m	🟩 🟩
run-tests-main-branch	17m	🟩 ⬜ 🟩 🟩 🟥
cli-e2e-tests / run-cli-tests	17m	🟩 🟩
check	16m	🟩 ⬜ ⬜ 🟩 🟩
general-lints	11m	🟩 ⬜ ⬜ 🟩 🟩
check-dynamic-deps	10m	🟩 🟩 🟩 🟩 🟩
node-api-compatibility-tests / node-api-compatibility-tests	2m	🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩
file_change_determinator	57s	🟩 🟩 🟩 🟩 🟩
file_change_determinator	52s	🟩 🟩 🟩 🟩 🟩
permission-check	25s	🟩 🟩 🟩 🟩 🟩
file_change_determinator	19s	🟩 🟩
execution-performance / file_change_determinator	19s	🟩 🟩
permission-check	18s	🟩 🟩 🟩 🟩
permission-check	15s	🟩 🟩 🟩 🟩 🟩
execution-performance / sequential-execution-performance	15s	🟩 🟩
execution-performance / parallel-execution-performance	14s	🟩 🟩
permission-check	13s	🟩 🟩 🟩 🟩 🟩
permission-check	5s	🟩 🟩
determine-docker-build-metadata	4s	🟩 🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
forge-compat-test / forge	19m	13m
cli-e2e-tests / run-cli-tests	10m	8m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

gregnazario

Why not just disable view functions on the node?

banool · 2024-01-16T22:45:08Z

We want something more granular. If one view function is being called a lot and hurting the node we'd like to disable that function specifically without killing all other view functions. We can't realistically disable view functions on our public node APIs without causing massive damage to ecosystem projects.

bchocho

LGTM, one comment

bchocho · 2024-01-17T01:26:06Z

api/src/view_function.rs

+        view_function.module.name().as_str(),
+        view_function.function.as_str(),
+    ) {
+        return Err(BasicErrorWith404::bad_request_with_code_no_info(


Should it be a 403?

github-actions · 2024-01-17T18:03:10Z

✅ Forge suite `realistic_env_max_load` success on `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`

two traffics test: inner traffic : committed: 7873 txn/s, latency: 4927 ms, (p50: 4500 ms, p90: 6000 ms, p99: 14700 ms), latency samples: 3401340
two traffics test : committed: 100 txn/s, latency: 2208 ms, (p50: 2100 ms, p90: 2400 ms, p99: 11100 ms), latency samples: 1820
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.298, avg: 0.214", "QsPosToProposal: max: 0.230, avg: 0.154", "ConsensusProposalToOrdered: max: 0.620, avg: 0.556", "ConsensusOrderedToCommit: max: 0.472, avg: 0.446", "ConsensusProposalToCommit: max: 1.090, avg: 1.002"]
Max round gap was 1 [limit 4] at version 1679562. Max no progress secs was 4.487212 [limit 10] at version 1679562.
Test Ok

github-actions · 2024-01-17T18:09:17Z

✅ Forge suite `compat` success on `aptos-node-v1.8.3` ==> `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`

Compatibility test results for aptos-node-v1.8.3 ==> 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b (PR)
1. Check liveness of validators at old version: aptos-node-v1.8.3
compatibility::simple-validator-upgrade::liveness-check : committed: 4794 txn/s, latency: 6815 ms, (p50: 6900 ms, p90: 10500 ms, p99: 12700 ms), latency samples: 177400
2. Upgrading first Validator to new version: 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1787 txn/s, latency: 16276 ms, (p50: 19200 ms, p90: 22300 ms, p99: 22700 ms), latency samples: 92940
3. Upgrading rest of first batch to new version: 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1784 txn/s, latency: 16158 ms, (p50: 19300 ms, p90: 22300 ms, p99: 22600 ms), latency samples: 92780
4. upgrading second batch to new version: 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3431 txn/s, latency: 8826 ms, (p50: 9900 ms, p90: 11900 ms, p99: 12400 ms), latency samples: 144140
5. check swarm health
Compatibility test for aptos-node-v1.8.3 ==> 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b passed
Test Ok

github-actions · 2024-01-17T18:30:43Z

❌ Forge suite `framework_upgrade` failure on `aptos-node-v1.8.3` ==> `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`

Compatibility test results for aptos-node-v1.8.3 ==> 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b (PR)
Upgrade the nodes to version: 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b
Test Failed: API error: Unknown error error sending request for url (http://aptos-node-3-validator.forge-framework-upgrade-pr-11666.svc:8080/v1/estimate_gas_price): error trying to connect: dns error: failed to lookup address information: Name or service not known

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: __libc_start_main
  15: <unknown>
Trailing Log Lines:
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: __libc_start_main
  15: <unknown>


Swarm logs can be found here: See fgi output for more information.
thread 'main' panicked at testsuite/forge/src/backend/k8s/swarm.rs:676:18:
called `Result::unwrap()` on an `Err` value: ApiError: namespaces "forge-framework-upgrade-pr-11666" not found: NotFound (ErrorResponse { status: "Failure", message: "namespaces \"forge-framework-upgrade-pr-11666\" not found", reason: "NotFound", code: 404 })

Caused by:
    namespaces "forge-framework-upgrade-pr-11666" not found: NotFound

Stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: __libc_start_main
  16: <unknown>
stack backtrace:
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Debugging output:

gregnazario · 2024-01-19T21:43:45Z

api/src/transactions.rs

+            ) {
+                return Err(SubmitTransactionError::forbidden_with_code(
+                    "Transaction not allowed by simulation filter",
+                    AptosErrorCode::InvalidInput,


Arguably, it could possibly be a different code, but that's fine. InvalidInput always requires user intervention.

banool force-pushed the banool/view-simulate-filter branch from bd75d6f to 999d6b3 Compare January 16, 2024 22:16

banool marked this pull request as ready for review January 16, 2024 22:17

banool requested review from JoshLind, gregnazario and 0xmaayan as code owners January 16, 2024 22:17

banool requested review from sitalkedia and bchocho and removed request for gregnazario, JoshLind and 0xmaayan January 16, 2024 22:17

banool force-pushed the banool/view-simulate-filter branch from 999d6b3 to 76257fa Compare January 16, 2024 22:19

banool enabled auto-merge (squash) January 16, 2024 22:20

This comment has been minimized.

Sign in to view

gregnazario reviewed Jan 16, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

bchocho approved these changes Jan 17, 2024

View reviewed changes

[API] Add filter support for view and simulate

0c517d8

banool force-pushed the banool/view-simulate-filter branch from 76257fa to 0c517d8 Compare January 17, 2024 17:40

This comment has been minimized.

Sign in to view

gregnazario approved these changes Jan 19, 2024

View reviewed changes

banool merged commit bc4b3fc into main Jan 19, 2024
40 of 44 checks passed

banool deleted the banool/view-simulate-filter branch January 19, 2024 21:50

banool added a commit that referenced this pull request Jan 19, 2024

[API] Add filter support for view and simulate (#11666)

eb3cc2b

This was referenced Jan 19, 2024

Cherry pick bc4b3fcd449f3376f9e891245d9fb31e11016c13 into aptos-release-v1.9 #11714

Merged

[API] Add tests for view function and txn simulation filters #11796

Merged

banool added a commit that referenced this pull request Jan 30, 2024

[API] Add filter support for view and simulate (#11666) (#11714)

53a437e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[API] Add filter support for view and simulate #11666

[API] Add filter support for view and simulate #11666

banool commented Jan 16, 2024 •

edited

Loading

trunk-io bot commented Jan 16, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gregnazario left a comment •

edited

Loading

This comment has been minimized.

This comment has been minimized.

banool commented Jan 16, 2024 •

edited

Loading

This comment has been minimized.

bchocho left a comment

bchocho Jan 17, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 17, 2024

github-actions bot commented Jan 17, 2024

github-actions bot commented Jan 17, 2024

gregnazario Jan 19, 2024

[API] Add filter support for view and simulate #11666

[API] Add filter support for view and simulate #11666

Conversation

banool commented Jan 16, 2024 • edited Loading

Description

Test Plan

View Functions

Simulation

trunk-io bot commented Jan 16, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gregnazario left a comment • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

banool commented Jan 16, 2024 • edited Loading

This comment has been minimized.

bchocho left a comment

Choose a reason for hiding this comment

bchocho Jan 17, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 17, 2024

✅ Forge suite realistic_env_max_load success on 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b

github-actions bot commented Jan 17, 2024

✅ Forge suite compat success on aptos-node-v1.8.3 ==> 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b

github-actions bot commented Jan 17, 2024

❌ Forge suite framework_upgrade failure on aptos-node-v1.8.3 ==> 0c517d87e7ee0cb32ac9db0a5b3b648150f9687b

gregnazario Jan 19, 2024

Choose a reason for hiding this comment

banool commented Jan 16, 2024 •

edited

Loading

trunk-io bot commented Jan 16, 2024 •

edited

Loading

gregnazario left a comment •

edited

Loading

banool commented Jan 16, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`

✅ Forge suite `compat` success on `aptos-node-v1.8.3` ==> `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`

❌ Forge suite `framework_upgrade` failure on `aptos-node-v1.8.3` ==> `0c517d87e7ee0cb32ac9db0a5b3b648150f9687b`