more nla spec#19554
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
86876de to
b6a8548
Compare
209f85c to
9ca5b14
Compare
d5603eb to
d4348ed
Compare
44f6bb9 to
b8e9743
Compare
b02b9d0 to
e589ac5
Compare
764aa3b to
312b029
Compare
1b8e691 to
a970055
Compare
3a8d83d to
5d90597
Compare
5d90597 to
603d9b1
Compare
603d9b1 to
b535269
Compare
b535269 to
e74f967
Compare
e74f967 to
d83c49b
Compare
d83c49b to
eaa08be
Compare
There was a problem hiding this comment.
Stale comment
Aptos Security Bugbot has reviewed your changes and found no new issue.
All prior threads have been resolved. The changes remain confined to Move Prover formal-verification artifacts (specs, Boogie prelude) with no impact on on-chain runtime behavior.
Sent by Cursor Automation: Security Review Bot
6a40536 to
0a8791f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca6638c. Configure here.
| spec new_with_config { | ||
| pragma verify = false; | ||
| pragma opaque; | ||
| aborts_if [abstract] inner_max_degree != 0 |
There was a problem hiding this comment.
Why do we still have abstract here? (In contrast to PR description) Here and elsewhere
There was a problem hiding this comment.
Removed all [abstract] in the spec.
| && spec_len(self) == spec_len(old(self)) - 1 | ||
| && (forall k: K where k != key: | ||
| spec_contains_key(self, k) == spec_contains_key(old(self), k)) |
There was a problem hiding this comment.
Maybe can pull this here and below into a helper function?
FWIW we can now use old in spec functions: spec fun f(a: address) { R[a] == old(R[a]) } should be possible, and f can then be used only in ensures (two-state predicates). Not sure how well tested.
There was a problem hiding this comment.
Added a spec function. However, old only works for global resources (at least for now) so the added spec function still requires passing both old and new value to it.
0d026df to
603158c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5d9b2d6. Configure here.
There was a problem hiding this comment.
Stale comment
Aptos Security Bugbot has reviewed your changes and found no new issue.
All changed files are Move Prover formal specification artifacts (
big_ordered_map.spec.move,transaction_context.spec.move,math64.moveloop invariants,math_fixed.spec.move,string.spec.move,native.bplBoogie prelude, and the auto-generatedhead.mrb). No on-chain execution semantics are altered.Sent by Cursor Automation: Security Review Bot
There was a problem hiding this comment.
Stale comment
Aptos Security Bugbot has reviewed your changes and found no new issue.
All changes are Move Prover formal specification additions (
specblocks, loop invariants, Boogie prelude stubs) with no runtime behavior changes. All prior automation findings have been addressed.Sent by Cursor Automation: Security Review Bot
There was a problem hiding this comment.
Aptos Security Bugbot has reviewed your changes and found no new issue.
All changed files are Move Prover formal specifications (big_ordered_map.spec.move, transaction_context.spec.move, math64.move, math_fixed.spec.move, string.spec.move, native.bpl) and the cached-packages binary. These changes carry no runtime impact and introduce no security-relevant logic.
Sent by Cursor Automation: Security Review Bot
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|




Description
Loosely-coupled improvements to the formal-verification surface, all on top of
teng/handle-intrinsics-for-spec-infer.1. BigOrderedMap specs
New spec blocks (callers gain abstract postconditions enabling existence / FIFO-progress / frame-condition reasoning):
front_key,back_key,remove_or_nonepop_back(was previously empty), full ensures mirroringpop_frontiter_modify,iter_remove— declare the value-mutation / removal frame condition through the closure / iterator interfaceinternal_find_with_path,iter_with_path_get_iterinternal_leaf_new_begin_iter,internal_leaf_iter_is_end,internal_leaf_borrow_value,internal_leaf_iter_borrow_entries_and_next_leaf_indexNew / expanded
ensureson previously-empty specs:iter_is_end,iter_borrow,iter_borrow_mut,iter_borrow_key,iter_is_begin,iter_next,iter_prevpop_front,pop_backinternal_find,internal_lower_bound,internal_new_begin_iter,internal_new_end_iternew_with_config,new_with_reusable,new_with_type_size_hints— gainaborts_ifon inner/leaf max-degree bounds plus empty-resultensures.Shared
spec_unchanged_except_at(self, key)predicate — captures the frame condition "for all keys other thankey, containment and value are unchanged betweenold(self)andself." Used inremove,remove_or_none,upsert,pop_front,pop_back,iter_modify,iter_remove. Collapses verboseforall k where k != keyclauses into a single named predicate, applied uniformly across single-key mutating operations.Drops the
[abstract]modifier from existingensures/aborts_ifclauses acrossremove,keys,new_from,upsert,borrow_front,borrow_back,prev_key,next_key,compute_length. Each of these specs still carriespragma verify = false, so the contracts remain trusted; removing[abstract]enforces the convention "one trust lever per spec block."2. Move-Prover prelude fix
Adds the missing
$1_vector_move_range{{S}}procedure. Before this fix, any verification path that lowered tovector::move_rangefailed Boogie with:This forced
pragma verify = falseon every function that transitively calledvector::move_range. The stub models the operation via existingSliceVec/ConcatVecprimitives and relies on Move's guarantee that the two&mut vector<T>arguments cannot alias. Defensive< 0bounds checks (against possibly-negative Boogie ints) follow the convention used in$1_vector_insert.3. NLA-adjacent stdlib fixes
math64::floor_log2: added two loop invariants ((res as u64) + 2*(n as u64) <= 64andn == 0 ==> (res as u64) <= 63) so the loop discharges.math_fixed::sqrt: addedensures x.get_raw_value() == 0 ==> result.get_raw_value() == 0;so callers can rule out zero outputs without descending into non-linear arithmetic.string::utf8: tightened from previous trustedaborts_if [abstract] falseto honestaborts_if !spec_internal_check_utf8(bytes). Callers using static ASCII literals must discharge UTF-8 validity (typically via a module-levelaxiom std::string::spec_internal_check_utf8(b"...")).4.
transaction_context.spec.moveMarks
monotonically_increasing_counter_internal,monotonically_increasing_counter_internal_for_test_only, andmonotonically_increasing_counterasaborts_if [abstract] falseso callers can treat the counter as total.How Has This Been Tested?
aptos move prove --package-dir aptos-move/framework/aptos-framework --filter big_ordered_mappasses. All BOM spec changes arepragma verify = false(trusted contracts on top of the intrinsic / opaque body), so they impose no verification load themselves while exposing stronger postconditions to downstream callers.Key Areas to Review
native.bplmove_range procedure: confirm the abort conditions (removal_position + length > LenVec(from_v),insert_position > LenVec(to_v)) exactly match Move-runtime semantics, and that the no-aliasing assumption is safe at every current and plausibly-future call site.math64::floor_log2loop invariants: confirm the invariants are strong enough to discharge whatever downstream proof obligations motivated adding them.spec_unchanged_except_atpredicate — the centerpiece refactor. Confirm semantics by inspection ofremove,pop_front/pop_back,iter_modify/iter_remove.monotonically_increasing_countertrust: confirm the over-claim against the documented overflow abort path is acceptable for current callers.Note
Medium Risk
Changes are spec/prelude-only (no on-chain runtime logic), but trusted contracts and the
move_rangeBoogie model must match Move semantics; the counter’saborts_if falseover-claims vs documented overflow.Overview
Strengthens Move Prover contracts across BigOrderedMap, stdlib math/string helpers, transaction context, and the Boogie vector prelude so downstream proofs can rely on richer postconditions without re-proving bodies.
BigOrderedMap gains trusted (
pragma verify = false) specs for iterators, bounds search,front_key/back_key,remove_or_none,pop_*, path-based find/remove, and leaf helpers. A sharedspec_unchanged_except_atframe predicate replaces repeatedforall k != keyclauses on single-key mutators. Many specs drop the[abstract]modifier onensures/aborts_ifwhile staying trusted viaverify = false. Constructors document degree bounds and empty-map results;removepostconditions are tightened (including length on delete).The prover prelude adds
$1_vector_move_range, fixing undeclared-procedure failures when verification lowersvector::move_range.math64::floor_log2gets loop invariants;math_fixed::sqrtdocuments zero-in/zero-out;string::utf8aborts on invalid UTF-8 instead of a blanket non-abort.monotonically_increasing_counter(and internal variants) are marked non-aborting under an abstract trust assumption (documented overflow caveat).Reviewed by Cursor Bugbot for commit b47a790. Bugbot is set up for automated code reviews on this repo. Configure here.