Skip to content

Commit 5337340

Browse files
bond00729claude
andcommitted
feat(link): gate type-scoped compaction behind V19/BlockLinks
The type-scoped compaction fix changes which messages a LinkCompactState deletes, which changes the trie state root — so deploying it ungated would fork a fixed node from the network on historical data. nindexer confirms the risk is real: stray "unfollow"-typed links exist in history (a client bug; "unfollow" squeaks past the <=8-byte link-type check) and ~25k follow compact states exist, so a follow compaction that swept an unfollow link under the old type-blind code would keep it under the fix. Gate the behavior on a new EngineVersion::V19 / ProtocolFeature::BlockLinks: Store::merge and merge_compact_state take scope_compaction_by_type, which the engine derives from the message's version (is_enabled(BlockLinks)). Pre-V19 stays type-blind (preserves historical state roots); V19+ scopes deletion to the compact state's own link type so a follow compaction can't delete block links. The Store layer stays version-agnostic — it receives the bool, the engine decides it. Mainnet/testnet V19 activation is a placeholder pending rollout coordination; devnet runs V19 from genesis. link_store tests pin both regimes (scoped keeps other link types, type-blind deletes them). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7cb274c commit 5337340

12 files changed

Lines changed: 319 additions & 67 deletions

src/storage/store/account/cast_store_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ mod tests {
5959
err_message: String,
6060
) {
6161
let mut txn = RocksDbTransactionBatch::new();
62-
let result = store.merge(&message, &mut txn);
62+
let result = store.merge(&message, &mut txn, false);
6363
assert!(result.is_err());
6464
let error = result.unwrap_err();
6565
assert_eq!(error.code, err_code);
@@ -73,7 +73,7 @@ mod tests {
7373
deleted_messages: Vec<message::Message>,
7474
) {
7575
let mut txn = RocksDbTransactionBatch::new();
76-
let result = store.merge(message, &mut txn).unwrap();
76+
let result = store.merge(message, &mut txn, false).unwrap();
7777
assert_eq!(result.r#type(), HubEventType::MergeMessage);
7878
match &result.body {
7979
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -96,7 +96,7 @@ mod tests {
9696
let mut events = Vec::new();
9797

9898
for message in messages {
99-
let result = store.merge(message, &mut txn).unwrap();
99+
let result = store.merge(message, &mut txn, false).unwrap();
100100
events.push(result.clone());
101101
assert_eq!(result.r#type(), HubEventType::MergeMessage);
102102
match &result.body {
@@ -1271,7 +1271,7 @@ mod tests {
12711271
);
12721272

12731273
let mut txn1 = RocksDbTransactionBatch::new();
1274-
let _result1 = store.merge(&cast_remove_earlier, &mut txn1).unwrap();
1274+
let _result1 = store.merge(&cast_remove_earlier, &mut txn1, false).unwrap();
12751275
db.commit(txn1).unwrap();
12761276

12771277
merge_message_failure(

src/storage/store/account/link_store_tests.rs

Lines changed: 131 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ mod tests {
5050
message: &message::Message,
5151
) {
5252
let mut txn = RocksDbTransactionBatch::new();
53-
let result = store.merge(&message, &mut txn).unwrap();
53+
let result = store.merge(&message, &mut txn, false).unwrap();
5454
assert_eq!(result.r#type(), HubEventType::MergeMessage);
5555
match &result.body {
5656
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -64,14 +64,41 @@ mod tests {
6464
db.commit(txn).unwrap();
6565
}
6666

67+
// Shared fixture for the compaction tests: a follow compact state at ts 2000 that
68+
// references neither TARGET_FID nor any block target.
69+
fn non_target_follow_compact_state() -> message::Message {
70+
messages_factory::links::create_link_compact_state(
71+
FID_FOR_TEST,
72+
LINK_TYPE_FOLLOW,
73+
vec![FID_FOR_TEST + 100],
74+
Some(2000),
75+
None,
76+
)
77+
}
78+
79+
// Merge a compact state at the given scope and commit. (merge_message_success can't be
80+
// reused: compaction deletes messages, which that helper asserts does not happen.)
81+
fn merge_compact_state_with_scope(
82+
store: &Store<LinkStore>,
83+
db: &Arc<RocksDB>,
84+
message: &message::Message,
85+
scope_compaction_by_type: bool,
86+
) {
87+
let mut txn = RocksDbTransactionBatch::new();
88+
store
89+
.merge(message, &mut txn, scope_compaction_by_type)
90+
.unwrap();
91+
db.commit(txn).unwrap();
92+
}
93+
6794
fn merge_message_with_conflicts(
6895
store: &Store<LinkStore>,
6996
db: &Arc<RocksDB>,
7097
message: &message::Message,
7198
deleted_messages: Vec<message::Message>,
7299
) {
73100
let mut txn = RocksDbTransactionBatch::new();
74-
let result = store.merge(&message, &mut txn).unwrap();
101+
let result = store.merge(&message, &mut txn, false).unwrap();
75102
assert_eq!(result.r#type(), HubEventType::MergeMessage);
76103
match &result.body {
77104
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -92,7 +119,7 @@ mod tests {
92119
err_message: &str,
93120
) {
94121
let mut txn = RocksDbTransactionBatch::new();
95-
let result = store.merge(&message, &mut txn);
122+
let result = store.merge(&message, &mut txn, false);
96123
assert!(result.is_err());
97124
let err = result.err().unwrap();
98125
assert_eq!(err.code, err_code);
@@ -2279,14 +2306,8 @@ mod tests {
22792306
merge_message_success(&store, &db, &block_add);
22802307

22812308
// A later follow compact state that does not reference TARGET_FID at all.
2282-
let follow_compact_state = messages_factory::links::create_link_compact_state(
2283-
FID_FOR_TEST,
2284-
LINK_TYPE_FOLLOW,
2285-
vec![FID_FOR_TEST + 100],
2286-
Some(2000),
2287-
None,
2288-
);
2289-
merge_message_success(&store, &db, &follow_compact_state);
2309+
let follow_compact_state = non_target_follow_compact_state();
2310+
merge_compact_state_with_scope(&store, &db, &follow_compact_state, true);
22902311

22912312
// The block link must still be active, because the follow compaction must
22922313
// not touch links of a different type.
@@ -2319,25 +2340,113 @@ mod tests {
23192340
merge_message_success(&store, &db, &block_remove);
23202341

23212342
// A later follow compact state.
2322-
let follow_compact_state = messages_factory::links::create_link_compact_state(
2343+
let follow_compact_state = non_target_follow_compact_state();
2344+
merge_compact_state_with_scope(&store, &db, &follow_compact_state, true);
2345+
2346+
// The block remove tombstone must survive the follow compaction.
2347+
let retrieved = LinkStore::get_link_remove(
2348+
&store,
2349+
FID_FOR_TEST,
2350+
LINK_TYPE_BLOCK.to_string(),
2351+
Some(Target::TargetFid(TARGET_FID)),
2352+
)
2353+
.unwrap()
2354+
.unwrap();
2355+
assert_eq!(retrieved, block_remove);
2356+
}
2357+
2358+
#[test]
2359+
fn test_legacy_follow_compaction_deletes_other_link_type_adds() {
2360+
// Pre-V19 (type-blind) behavior: with scope_compaction_by_type = false, a "follow"
2361+
// compaction deletes a "block" add whose target is not in the compact state. This is
2362+
// the historical behavior preserved on replay, so a fixed node doesn't fork.
2363+
let (store, db, _temp_dir) = create_test_store();
2364+
2365+
let block_add = messages_factory::links::create_link_add(
2366+
FID_FOR_TEST,
2367+
LINK_TYPE_BLOCK,
2368+
TARGET_FID,
2369+
Some(1000),
2370+
None,
2371+
);
2372+
merge_message_success(&store, &db, &block_add);
2373+
2374+
let follow_compact_state = non_target_follow_compact_state();
2375+
merge_compact_state_with_scope(&store, &db, &follow_compact_state, false);
2376+
2377+
let retrieved = LinkStore::get_link_add(
2378+
&store,
2379+
FID_FOR_TEST,
2380+
LINK_TYPE_BLOCK.to_string(),
2381+
Some(Target::TargetFid(TARGET_FID)),
2382+
)
2383+
.unwrap();
2384+
assert!(retrieved.is_none());
2385+
}
2386+
2387+
#[test]
2388+
fn test_legacy_follow_compaction_deletes_other_link_type_removes() {
2389+
// Pre-V19 (type-blind) behavior: with scope_compaction_by_type = false, a "follow"
2390+
// compaction also deletes a "block" remove tombstone.
2391+
let (store, db, _temp_dir) = create_test_store();
2392+
2393+
let block_remove = messages_factory::links::create_link_remove(
2394+
FID_FOR_TEST,
2395+
LINK_TYPE_BLOCK,
2396+
TARGET_FID,
2397+
Some(1000),
2398+
None,
2399+
);
2400+
merge_message_success(&store, &db, &block_remove);
2401+
2402+
let follow_compact_state = non_target_follow_compact_state();
2403+
merge_compact_state_with_scope(&store, &db, &follow_compact_state, false);
2404+
2405+
let retrieved = LinkStore::get_link_remove(
2406+
&store,
2407+
FID_FOR_TEST,
2408+
LINK_TYPE_BLOCK.to_string(),
2409+
Some(Target::TargetFid(TARGET_FID)),
2410+
)
2411+
.unwrap();
2412+
assert!(retrieved.is_none());
2413+
}
2414+
2415+
#[test]
2416+
fn test_block_compaction_does_not_delete_follow_links() {
2417+
// The gate is symmetric: a "block" compact state must only compact "block" links and
2418+
// leave "follow" links intact (the direction blocks actually ship for).
2419+
let (store, db, _temp_dir) = create_test_store();
2420+
2421+
// A follow add, older than the block compact state, whose target is not listed.
2422+
let follow_add = messages_factory::links::create_link_add(
23232423
FID_FOR_TEST,
23242424
LINK_TYPE_FOLLOW,
2425+
TARGET_FID,
2426+
Some(1000),
2427+
None,
2428+
);
2429+
merge_message_success(&store, &db, &follow_add);
2430+
2431+
let block_compact_state = messages_factory::links::create_link_compact_state(
2432+
FID_FOR_TEST,
2433+
LINK_TYPE_BLOCK,
23252434
vec![FID_FOR_TEST + 100],
23262435
Some(2000),
23272436
None,
23282437
);
2329-
merge_message_success(&store, &db, &follow_compact_state);
2438+
merge_compact_state_with_scope(&store, &db, &block_compact_state, true);
23302439

2331-
// The block remove tombstone must survive the follow compaction.
2332-
let retrieved = LinkStore::get_link_remove(
2440+
// The follow link survives: a block compaction must not touch follow links.
2441+
let retrieved = LinkStore::get_link_add(
23332442
&store,
23342443
FID_FOR_TEST,
2335-
LINK_TYPE_BLOCK.to_string(),
2444+
LINK_TYPE_FOLLOW.to_string(),
23362445
Some(Target::TargetFid(TARGET_FID)),
23372446
)
23382447
.unwrap()
23392448
.unwrap();
2340-
assert_eq!(retrieved, block_remove);
2449+
assert_eq!(retrieved, follow_add);
23412450
}
23422451

23432452
#[test]
@@ -2369,19 +2478,11 @@ mod tests {
23692478
merge_message_success(&store, &db, &follow_remove);
23702479

23712480
// A later follow compact state that references neither target.
2372-
let follow_compact_state = messages_factory::links::create_link_compact_state(
2373-
FID_FOR_TEST,
2374-
LINK_TYPE_FOLLOW,
2375-
vec![FID_FOR_TEST + 100],
2376-
Some(2000),
2377-
None,
2378-
);
2379-
// Merge the compact state directly: it deletes the non-target follows, and we assert
2380-
// on the resulting store state below rather than on the (scan-order-dependent) list of
2381-
// deleted messages, which is an implementation detail rather than the contract here.
2382-
let mut txn = RocksDbTransactionBatch::new();
2383-
store.merge(&follow_compact_state, &mut txn).unwrap();
2384-
db.commit(txn).unwrap();
2481+
let follow_compact_state = non_target_follow_compact_state();
2482+
// The compaction deletes the non-target follows; we assert on the resulting store
2483+
// state below rather than on the (scan-order-dependent) list of deleted messages,
2484+
// which is an implementation detail rather than the contract here.
2485+
merge_compact_state_with_scope(&store, &db, &follow_compact_state, true);
23852486

23862487
// The non-target follow add must be deleted by the compaction.
23872488
let retrieved_add = LinkStore::get_link_add(

src/storage/store/account/reaction_store_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ mod tests {
3131
message: &message::Message,
3232
) {
3333
let mut txn = RocksDbTransactionBatch::new();
34-
let result = store.merge(&message, &mut txn).unwrap();
34+
let result = store.merge(&message, &mut txn, false).unwrap();
3535
assert_eq!(result.r#type(), HubEventType::MergeMessage);
3636
match &result.body {
3737
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -52,7 +52,7 @@ mod tests {
5252
deleted_messages: Vec<message::Message>,
5353
) {
5454
let mut txn = RocksDbTransactionBatch::new();
55-
let result = store.merge(&message, &mut txn).unwrap();
55+
let result = store.merge(&message, &mut txn, false).unwrap();
5656
assert_eq!(result.r#type(), HubEventType::MergeMessage);
5757
match &result.body {
5858
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -73,7 +73,7 @@ mod tests {
7373
err_message: &str,
7474
) {
7575
let mut txn = RocksDbTransactionBatch::new();
76-
let result = store.merge(&message, &mut txn);
76+
let result = store.merge(&message, &mut txn, false);
7777
assert!(result.is_err());
7878
let error = result.unwrap_err();
7979
assert_eq!(error.code, err_code);

src/storage/store/account/storage_lend_store.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ impl StorageLendStore {
366366
txn: &mut RocksDbTransactionBatch,
367367
) -> Result<Vec<HubEvent>, HubError> {
368368
let mut events = vec![];
369-
events.push(store.merge(message, txn)?);
369+
events.push(store.merge(message, txn, false)?);
370370
match message.data.as_ref().unwrap().body.as_ref().unwrap() {
371371
proto::message_data::Body::LendStorageBody(lend_storage_body) => {
372372
// Prune out the lend messages where storage is revoked so they don't consume storage.

src/storage/store/account/storage_lend_store_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ mod tests {
2929
message: &message::Message,
3030
) {
3131
let mut txn = RocksDbTransactionBatch::new();
32-
let result = store.merge(&message, &mut txn).unwrap();
32+
let result = store.merge(&message, &mut txn, false).unwrap();
3333
assert_eq!(result.r#type(), HubEventType::MergeMessage);
3434
match &result.body {
3535
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -50,7 +50,7 @@ mod tests {
5050
deleted_messages: Vec<message::Message>,
5151
) {
5252
let mut txn = RocksDbTransactionBatch::new();
53-
let result = store.merge(&message, &mut txn).unwrap();
53+
let result = store.merge(&message, &mut txn, false).unwrap();
5454
assert_eq!(result.r#type(), HubEventType::MergeMessage);
5555
match &result.body {
5656
Some(hub_event::Body::MergeMessageBody(body)) => {
@@ -71,7 +71,7 @@ mod tests {
7171
err_message: &str,
7272
) {
7373
let mut txn = RocksDbTransactionBatch::new();
74-
let result = store.merge(&message, &mut txn);
74+
let result = store.merge(&message, &mut txn, false);
7575
assert!(result.is_err());
7676
let error = result.unwrap_err();
7777
assert_eq!(error.code, err_code);

src/storage/store/account/store.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,7 @@ impl<T: StoreDef + Clone> Store<T> {
564564
&self,
565565
message: &Message,
566566
txn: &mut RocksDbTransactionBatch,
567+
scope_compaction_by_type: bool,
567568
) -> Result<HubEvent, HubError> {
568569
if !self.store_def.is_add_type(message)
569570
&& !(self.store_def.remove_type_supported() && self.store_def.is_remove_type(message))
@@ -579,7 +580,7 @@ impl<T: StoreDef + Clone> Store<T> {
579580
let ts_hash = make_ts_hash(message.data.as_ref().unwrap().timestamp, &message.hash)?;
580581

581582
if self.store_def().is_compact_state_type(message) {
582-
self.merge_compact_state(message, txn)
583+
self.merge_compact_state(message, txn, scope_compaction_by_type)
583584
} else if self.store_def.is_add_type(message) {
584585
self.merge_add(&ts_hash, message, txn)
585586
} else {
@@ -649,6 +650,7 @@ impl<T: StoreDef + Clone> Store<T> {
649650
&self,
650651
message: &Message,
651652
txn: &mut RocksDbTransactionBatch,
653+
scope_compaction_by_type: bool,
652654
) -> Result<HubEvent, HubError> {
653655
let mut merge_conflicts = vec![];
654656

@@ -682,9 +684,8 @@ impl<T: StoreDef + Clone> Store<T> {
682684
let (fid, compact_state_timestamp, target_fids, compact_state_type) =
683685
self.read_compact_state_details(message)?;
684686

685-
// Go over all the messages for this Fid of the SAME link type as the compact state
686-
// (a "follow" compaction must not touch other-typed links, e.g. "block") that are
687-
// older than the compact state message and
687+
// Go over all messages for this Fid older than the compact state (when type-scoped,
688+
// only those of the compact state's own link type — see below) and
688689
// 1. Delete all remove messages
689690
// 2. Delete all add messages that are not in the target_fids list
690691
let prefix = &make_message_primary_key(fid, self.store_def.postfix(), None);
@@ -701,12 +702,14 @@ impl<T: StoreDef + Clone> Store<T> {
701702
return Ok(true);
702703
}
703704

704-
// Only compact links of the same type as the compact state message.
705-
// A "follow" compaction must not delete other-typed links, e.g. "block".
706705
if let Some(Body::LinkBody(link_body)) =
707706
message.data.as_ref().and_then(|data| data.body.as_ref())
708707
{
709-
if link_body.r#type == compact_state_type {
708+
// When scope_compaction_by_type is set, only compact links of the compact
709+
// state's own type (so a "follow" compaction can't delete "block" links);
710+
// otherwise compaction is type-blind. The caller sets this from the engine
711+
// version (ProtocolFeature::BlockLinks).
712+
if !scope_compaction_by_type || link_body.r#type == compact_state_type {
710713
if self.store_def.is_remove_type(&message) {
711714
merge_conflicts.push(message);
712715
} else if self.store_def.is_add_type(&message) {

0 commit comments

Comments
 (0)