Skip to content

[compiler] Add format string validation for assert macros#18533

Merged
calintat merged 1 commit intomainfrom
calin/format-string-validation
Feb 12, 2026
Merged

[compiler] Add format string validation for assert macros#18533
calintat merged 1 commit intomainfrom
calin/format-string-validation

Conversation

@calintat
Copy link
Contributor

@calintat calintat commented Jan 29, 2026

Description

Follow up to #18412 (comment)

This commit adds compile-time validation for format strings used in assert!, assert_eq!, and assert_ne! macros in the Move compiler. Th compiler now detects mismatched braces, invalid placeholders, and ensures the number of {} placeholders matches the number of arguments provided. This prevents runtime errors by catching issues early.

How Has This Been Tested?

Added new tests for invalid forms.

Key Areas to Review

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Note

Medium Risk
Changes Move compiler macro expansion to reject previously-accepted assert!/assert_eq!/assert_ne! format strings (brace/placeholder/arg-count issues), which may surface new compile errors for existing code.

Overview
Compiler behavior change: assert!, assert_eq!, and assert_ne! now validate format strings at expansion time, detecting unmatched/invalid braces, counting {} placeholders (with {{/}} escaping), and enforcing that placeholder count matches the number of provided format arguments.

Tests: Extends macro tests with an escaped-brace formatting case and adds new negative tests/expected diagnostics for malformed format strings and argument-count mismatches.

Written by Cursor Bugbot for commit 3dfb957. This will update automatically on new commits. Configure here.

@calintat calintat force-pushed the calin/format-string-validation branch from d3679e2 to d0a0c00 Compare January 29, 2026 14:28
@calintat calintat marked this pull request as ready for review January 29, 2026 15:23
@calintat calintat requested review from vineethk and wrwg January 29, 2026 15:24
@calintat calintat force-pushed the calin/format-string-validation branch from d0a0c00 to c587d8a Compare February 5, 2026 13:32
@calintat calintat requested a review from vineethk February 5, 2026 13:32
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@calintat calintat force-pushed the calin/format-string-validation branch from c587d8a to fe82dd6 Compare February 5, 2026 16:09
@calintat calintat force-pushed the calin/format-string-validation branch from fe82dd6 to c7033cf Compare February 6, 2026 15:35
@calintat calintat enabled auto-merge (squash) February 6, 2026 15:35
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@calintat calintat force-pushed the calin/format-string-validation branch from c7033cf to 3661b31 Compare February 10, 2026 12:45
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@calintat calintat force-pushed the calin/format-string-validation branch from 3661b31 to 60f0e99 Compare February 12, 2026 12:49
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@calintat calintat force-pushed the calin/format-string-validation branch from 60f0e99 to 3dfb957 Compare February 12, 2026 13:30
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

✅ Forge suite compat success on 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca

Compatibility test results for 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca (PR)
1. Check liveness of validators at old version: 3d74b598d151879ab419e0c4377370b39b5c491f
compatibility::simple-validator-upgrade::liveness-check : committed: 13665.98 txn/s, latency: 2550.59 ms, (p50: 2700 ms, p70: 2800, p90: 3200 ms, p99: 3900 ms), latency samples: 446340
2. Upgrading first Validator to new version: 3dfb9576830e30272b32308dfce8ad347b7097ca
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6013.49 txn/s, latency: 5586.79 ms, (p50: 6200 ms, p70: 6300, p90: 6400 ms, p99: 6500 ms), latency samples: 208980
3. Upgrading rest of first batch to new version: 3dfb9576830e30272b32308dfce8ad347b7097ca
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6065.77 txn/s, latency: 5610.98 ms, (p50: 6200 ms, p70: 6400, p90: 6500 ms, p99: 6500 ms), latency samples: 210260
4. upgrading second batch to new version: 3dfb9576830e30272b32308dfce8ad347b7097ca
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10180.71 txn/s, latency: 3227.48 ms, (p50: 3200 ms, p70: 3600, p90: 4300 ms, p99: 4600 ms), latency samples: 340440
5. check swarm health
Compatibility test for 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 3dfb9576830e30272b32308dfce8ad347b7097ca

Forge report malformed: Expecting property name enclosed in double quotes: line 20 column 1 (char 452)
'{\n  "metrics": [\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "submitted_txn",\n      "value": 5095640.0\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "expired_txn",\n      "value": 20.0\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "avg_tps",\n      "value": 13706.415117438164\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n[2026-02-12T14:07:46Z INFO  aptos_forge::report] Test Ok\n      "metric": "avg_latency",\n      "value": 2747.901600197817\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "p50_latency",\n      "value": 2700.0\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "p90_latency",\n      "value": 3000.0\n    },\n    {\n      "test_name": "two traffics test: inner traffic",\n      "metric": "p99_latency",\n      "value": 3500.0\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "submitted_txn",\n      "value": 42740.0\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "expired_txn",\n      "value": 0.0\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "avg_tps",\n      "value": 100.00020503107083\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "avg_latency",\n      "value": 778.4170454545455\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "p50_latency",\n      "value": 700.0\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "p90_latency",\n      "value": 900.0\n    },\n    {\n      "test_name": "two traffics test",\n      "metric": "p99_latency",\n      "value": 1100.0\n    }\n  ],\n  "text": "two traffics test: inner traffic : committed: 13706.42 txn/s, submitted: 13706.47 txn/s, expired: 0.05 txn/s, latency: 2747.90 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3500 ms), latency samples: 5095620\\ntwo traffics test : committed: 100.00 txn/s, latency: 778.42 ms, (p50: 700 ms, p70: 800, p90: 900 ms, p99: 1100 ms), latency samples: 1760\\nLatency breakdown for phase 0: [\\"MempoolToBlockCreation: max: 2.195, avg: 2.133\\", \\"ConsensusProposalToOrdered: max: 0.168, avg: 0.166\\", \\"ConsensusOrderedToCommit: max: 0.070, avg: 0.068\\", \\"ConsensusProposalToCommit: max: 0.238, avg: 0.234\\"]\\nMax non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.49s no progress at version 27021 (avg 0.07s) [limit 15].\\nMax epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.31s no progress at version 2373224 (avg 0.31s) [limit 16].\\nTest Ok"\n}'
Trailing Log Lines:
networkchaos.chaos-mesh.org "4-gcp--as-southeast1-to-3-gcp--us-east4-netem" deleted from forge-e2e-pr-18533 namespace
test CompositeNetworkTest ... ok
Test Statistics: 
two traffics test: inner traffic : committed: 13706.42 txn/s, submitted: 13706.47 txn/s, expired: 0.05 txn/s, latency: 2747.90 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3500 ms), latency samples: 5095620
two traffics test : committed: 100.00 txn/s, latency: 778.42 ms, (p50: 700 ms, p70: 800, p90: 900 ms, p99: 1100 ms), latency samples: 1760
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.195, avg: 2.133", "ConsensusProposalToOrdered: max: 0.168, avg: 0.166", "ConsensusOrderedToCommit: max: 0.070, avg: 0.068", "ConsensusProposalToCommit: max: 0.238, avg: 0.234"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.49s no progress at version 27021 (avg 0.07s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.31s no progress at version 2373224 (avg 0.31s) [limit 16].
Test Ok

=== BEGIN JUNIT ===
<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="forge" tests="1" failures="0" errors="0" uuid="dcc33af0-cb84-4c9c-907e-fa2a5631ca7c">
    <testsuite name="local" tests="1" disabled="0" errors="0" failures="0">
        <testcase name="CompositeNetworkTest(network:multi-region-network-emulation(two traffics test)) with ">
        </testcase>
    </testsuite>
</testsuites>
=== END JUNIT ===
[2026-02-12T14:07:46Z INFO  aptos_forge::backend::k8s::cluster_helper] Deleting namespace forge-e2e-pr-18533: Some(NamespaceStatus { conditions: None, phase: Some("Terminating") })
[2026-02-12T14:07:46Z INFO  aptos_forge::backend::k8s::cluster_helper] aptos-node resources for Forge removed in namespace: forge-e2e-pr-18533

test result: ok. 1 passed; 0 soft failed; 0 hard failed; 0 filtered out

Debugging output:
NAME                                         READY   STATUS      RESTARTS   AGE
aptos-node-0-fullnode-eforge9445d912-0       1/1     Running     0          12m
aptos-node-0-validator-0                     1/1     Running     0          12m
aptos-node-1-fullnode-eforge9445d912-0       1/1     Running     0          12m
aptos-node-1-validator-0                     1/1     Running     0          12m
aptos-node-2-fullnode-eforge9445d912-0       1/1     Running     0          12m
aptos-node-2-validator-0                     1/1     Running     0          12m
aptos-node-3-fullnode-eforge9445d912-0       1/1     Running     0          12m
aptos-node-3-validator-0                     1/1     Running     0          12m
aptos-node-4-fullnode-eforge9445d912-0       1/1     Running     0          12m
aptos-node-4-validator-0                     1/1     Running     0          12m
aptos-node-5-validator-0                     1/1     Running     0          12m
aptos-node-6-validator-0                     1/1     Running     0          12m
forge-testnet-deployer-rcxkg                 0/1     Completed   0          13m
genesis-aptos-genesis-eforge9445d912-dl2v5   0/1     Completed   0          12m

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca

Compatibility test results for 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca (PR)
Upgrade the nodes to version: 3dfb9576830e30272b32308dfce8ad347b7097ca
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2274.38 txn/s, submitted: 2280.34 txn/s, failed submission: 5.96 txn/s, expired: 5.96 txn/s, latency: 1311.25 ms, (p50: 1200 ms, p70: 1500, p90: 1800 ms, p99: 2400 ms), latency samples: 206021
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2318.15 txn/s, submitted: 2325.87 txn/s, failed submission: 7.73 txn/s, expired: 7.73 txn/s, latency: 1266.90 ms, (p50: 1200 ms, p70: 1400, p90: 1800 ms, p99: 2200 ms), latency samples: 210040
5. check swarm health
Compatibility test for 3d74b598d151879ab419e0c4377370b39b5c491f ==> 3dfb9576830e30272b32308dfce8ad347b7097ca passed
Upgrade the remaining nodes to version: 3dfb9576830e30272b32308dfce8ad347b7097ca
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1535.90 txn/s, submitted: 1541.24 txn/s, failed submission: 5.34 txn/s, expired: 5.34 txn/s, latency: 1912.10 ms, (p50: 1200 ms, p70: 1400, p90: 1800 ms, p99: 11800 ms), latency samples: 138041
Test Ok

@calintat calintat merged commit ed08225 into main Feb 12, 2026
49 checks passed
@calintat calintat deleted the calin/format-string-validation branch February 12, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants