Skip to content

Commit d23add0

Browse files
KSlashhethan
andauthored
harden message claims and retry handling (#473)
* fix: add bip68 mask check for timelock * mark send-challenge as rpc-debug-endpoints * prevent instance upserts from overwriting progressed state * fix: harden message claims and retry handling * refactor: derive message identity from content * fix: prevent batch abandon and preserve poison-message detection * add tests for BusinessRef derivation & helper for bip68 check * fix sequencer-set-publish * fix: reuse genesis UTXOs and preserve retryable RPC errors * feat(store): persist P2P admission, identity and delivery state * feat(p2p): harden admission and isolate message workers * fix: recover graph setup and signing-round delivery * fix: recover exhausted graph setup delivery * fix: bind graph setup ACKs to payload and peers * fix: reject duplicate verifier peers in graph endorsements * fix ci * feat: Add CreateGraph delivery ACK retries --------- Co-authored-by: ethan <kslashhh@gmail.com>
1 parent 1ff9a4a commit d23add0

62 files changed

Lines changed: 13732 additions & 2632 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/ci.yml‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,10 @@ jobs:
4949
mkdir -p ~/.local/share/tlaplus
5050
curl -sL -o ~/.local/share/tlaplus/tla2tools.jar \
5151
https://github.com/tlaplus/tlaplus/releases/latest/download/tla2tools.jar
52-
# This audit pass proves bugs exist in CURRENT code and proves correct
53-
# fix designs for them - the fixes are NOT yet applied to the Rust code
54-
# (see node/README.md's "Known gap" sections). These configs model the
55-
# verified fix designs (or a baseline that was never buggy) and must
56-
# always pass. See root README.md's "Formal verification (TLA+)"
57-
# section for what each spec covers.
58-
- name: Run baseline + proposed-fix specs (must pass)
52+
# These configs model the implemented fixes (or a baseline that was
53+
# never buggy) and must always pass. The separate configs below retain
54+
# the pre-fix counterexamples as historical regression documentation.
55+
- name: Run baseline + fixed specs (must pass)
5956
working-directory: node/tla
6057
run: |
6158
set -e
@@ -152,7 +149,7 @@ jobs:
152149
run: sudo apt update && sudo apt install protobuf-compiler
153150
- run: |
154151
source ~/.zkm-toolchain/env
155-
cargo clippy --all-targets -- -D warnings
152+
cargo clippy --all-targets --features rpc-debug-endpoints -- -D warnings
156153
test:
157154
name: Cargo Test
158155
needs: tla-plus
@@ -173,4 +170,4 @@ jobs:
173170
run: |
174171
set -e
175172
source ~/.zkm-toolchain/env
176-
cargo test -r --all --all-targets
173+
cargo test -r --all --all-targets --features rpc-debug-endpoints

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ proof-builder-rpc/*.ckpt
3838
node/tla/states/
3939

4040
local_docs/
41+
local_scripts/
4142
scripts/testnet/
4243
scripts/devnet/
4344
*.DS_Store

‎Cargo.lock‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ members = [
1010
"crates/cbft-rpc",
1111
"crates/bitvm-gc",
1212
"crates/store",
13+
"crates/node-macros",
1314
"crates/client",
1415
"crates/util",
1516
"crates/header-chain",

‎crates/bitvm-gc/src/timelocks.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ pub fn validate_timelock_config(network: Network, config: &TimelockConfig) -> Re
155155
("operator_commit", config.operator_commit),
156156
("connector_f", config.connector_f),
157157
] {
158+
ensure_bip68_height_based_sequence(name, value)?;
158159
if value < budget.reaction_blocks {
159160
bail!(
160161
"timelock_config.{name} must be at least {} reaction blocks, got {value}",
@@ -210,6 +211,17 @@ pub fn validate_timelock_config(network: Network, config: &TimelockConfig) -> Re
210211
Ok(())
211212
}
212213

214+
const BIP68_BLOCKS_MASK: u32 = 0x0000_ffff;
215+
216+
fn ensure_bip68_height_based_sequence(name: &str, value: u32) -> Result<()> {
217+
if value & !BIP68_BLOCKS_MASK != 0 {
218+
bail!(
219+
"timelock_config.{name} must use a BIP68 height-based sequence value, got {value:#010x}"
220+
);
221+
}
222+
Ok(())
223+
}
224+
213225
fn ensure_gap_at_least(
214226
left_name: &str,
215227
left: u32,
@@ -291,4 +303,14 @@ mod tests {
291303
validate_timelock_config(network, &config).unwrap();
292304
}
293305
}
306+
307+
#[test]
308+
fn rejects_non_height_based_bip68_sequences() {
309+
assert!(ensure_bip68_height_based_sequence("test", BIP68_BLOCKS_MASK).is_ok());
310+
311+
for invalid in [1 << 16, 1 << 22, 1 << 31] {
312+
let error = ensure_bip68_height_based_sequence("test", invalid).unwrap_err();
313+
assert!(error.to_string().contains("BIP68 height-based sequence"));
314+
}
315+
}
294316
}

‎crates/node-macros/Cargo.toml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[package]
2+
name = "node-macros"
3+
version.workspace = true
4+
edition.workspace = true
5+
6+
[lib]
7+
proc-macro = true
8+
9+
[dependencies]
10+
proc-macro2 = "1"
11+
quote = "1"
12+
syn = { version = "2", features = ["full"] }

‎crates/node-macros/src/lib.rs‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
use proc_macro::TokenStream;
2+
use quote::quote;
3+
use syn::spanned::Spanned;
4+
use syn::{Attribute, Data, DeriveInput, Fields, Ident, Meta, Result, Variant, parse_macro_input};
5+
6+
#[proc_macro_derive(MessageBusinessRef, attributes(business_ref))]
7+
pub fn derive_message_business_ref(input: TokenStream) -> TokenStream {
8+
let input = parse_macro_input!(input as DeriveInput);
9+
match expand_message_business_ref(input) {
10+
Ok(tokens) => tokens.into(),
11+
Err(error) => error.into_compile_error().into(),
12+
}
13+
}
14+
15+
fn expand_message_business_ref(input: DeriveInput) -> Result<proc_macro2::TokenStream> {
16+
let enum_name = input.ident;
17+
let Data::Enum(data) = input.data else {
18+
return Err(syn::Error::new(
19+
enum_name.span(),
20+
"MessageBusinessRef can only be derived for enums",
21+
));
22+
};
23+
24+
let match_arms = data.variants.iter().map(expand_variant).collect::<Result<Vec<_>>>()?;
25+
26+
Ok(quote! {
27+
impl HasBusinessRef for #enum_name {
28+
fn business_ref(&self) -> BusinessRef {
29+
match self {
30+
#(#match_arms),*
31+
}
32+
}
33+
}
34+
})
35+
}
36+
37+
fn expand_variant(variant: &Variant) -> Result<proc_macro2::TokenStream> {
38+
let scope = business_ref_scope(variant)?;
39+
let variant_name = &variant.ident;
40+
41+
match scope.as_str() {
42+
"graph" => {
43+
let binding = tuple_payload_binding(variant)?;
44+
Ok(quote! {
45+
Self::#variant_name(#binding) => BusinessRef::Graph {
46+
instance_id: #binding.instance_id,
47+
graph_id: #binding.graph_id,
48+
}
49+
})
50+
}
51+
"instance" => {
52+
let binding = tuple_payload_binding(variant)?;
53+
Ok(quote! {
54+
Self::#variant_name(#binding) => BusinessRef::Instance {
55+
instance_id: #binding.instance_id,
56+
}
57+
})
58+
}
59+
"unscoped" => match &variant.fields {
60+
Fields::Unit => Ok(quote! {
61+
Self::#variant_name => BusinessRef::Unscoped
62+
}),
63+
Fields::Unnamed(_) => Ok(quote! {
64+
Self::#variant_name(..) => BusinessRef::Unscoped
65+
}),
66+
Fields::Named(_) => Ok(quote! {
67+
Self::#variant_name { .. } => BusinessRef::Unscoped
68+
}),
69+
},
70+
_ => unreachable!("business_ref_scope validates accepted values"),
71+
}
72+
}
73+
74+
fn tuple_payload_binding(variant: &Variant) -> Result<Ident> {
75+
match &variant.fields {
76+
Fields::Unnamed(fields) if fields.unnamed.len() == 1 => {
77+
Ok(Ident::new("message", variant.span()))
78+
}
79+
_ => Err(syn::Error::new(
80+
variant.span(),
81+
"graph and instance business references require exactly one payload field",
82+
)),
83+
}
84+
}
85+
86+
fn business_ref_scope(variant: &Variant) -> Result<String> {
87+
let mut matching = variant
88+
.attrs
89+
.iter()
90+
.filter(|attribute: &&Attribute| attribute.path().is_ident("business_ref"));
91+
let Some(attribute) = matching.next() else {
92+
// Point at the offending variant rather than the derive site, so the
93+
// compiler error names the variant that lacks an attribute.
94+
return Err(syn::Error::new(
95+
variant.ident.span(),
96+
"each message variant must declare #[business_ref(graph)], #[business_ref(instance)], or #[business_ref(unscoped)]",
97+
));
98+
};
99+
if matching.next().is_some() {
100+
return Err(syn::Error::new(attribute.span(), "duplicate business_ref attribute"));
101+
}
102+
103+
let Meta::List(list) = &attribute.meta else {
104+
return Err(syn::Error::new(
105+
attribute.span(),
106+
"business_ref must be written as #[business_ref(graph)], #[business_ref(instance)], or #[business_ref(unscoped)]",
107+
));
108+
};
109+
let scope: Ident = list.parse_args()?;
110+
let scope = scope.to_string();
111+
if matches!(scope.as_str(), "graph" | "instance" | "unscoped") {
112+
Ok(scope)
113+
} else {
114+
Err(syn::Error::new(attribute.span(), "business_ref must be graph, instance, or unscoped"))
115+
}
116+
}
117+
118+
#[cfg(test)]
119+
mod tests {
120+
use super::*;
121+
use syn::parse_quote;
122+
123+
fn expand_error(input: DeriveInput) -> String {
124+
expand_message_business_ref(input).unwrap_err().to_string()
125+
}
126+
127+
#[test]
128+
fn rejects_non_enum_input() {
129+
let input = parse_quote! {
130+
struct Message;
131+
};
132+
133+
assert_eq!(expand_error(input), "MessageBusinessRef can only be derived for enums");
134+
}
135+
136+
#[test]
137+
fn requires_a_business_ref_scope_for_every_variant() {
138+
let input = parse_quote! {
139+
enum Message {
140+
Missing(Payload),
141+
}
142+
};
143+
144+
assert_eq!(
145+
expand_error(input),
146+
"each message variant must declare #[business_ref(graph)], #[business_ref(instance)], or #[business_ref(unscoped)]"
147+
);
148+
}
149+
150+
#[test]
151+
fn rejects_invalid_or_duplicate_scopes() {
152+
let invalid = parse_quote! {
153+
enum Message {
154+
#[business_ref(other)]
155+
Invalid(Payload),
156+
}
157+
};
158+
assert_eq!(expand_error(invalid), "business_ref must be graph, instance, or unscoped");
159+
160+
let duplicate = parse_quote! {
161+
enum Message {
162+
#[business_ref(graph)]
163+
#[business_ref(instance)]
164+
Duplicate(Payload),
165+
}
166+
};
167+
assert_eq!(expand_error(duplicate), "duplicate business_ref attribute");
168+
}
169+
170+
#[test]
171+
fn graph_and_instance_scopes_require_one_tuple_payload() {
172+
let named = parse_quote! {
173+
enum Message {
174+
#[business_ref(graph)]
175+
Graph { payload: Payload },
176+
}
177+
};
178+
assert_eq!(
179+
expand_error(named),
180+
"graph and instance business references require exactly one payload field"
181+
);
182+
183+
let multiple = parse_quote! {
184+
enum Message {
185+
#[business_ref(instance)]
186+
Instance(Payload, Payload),
187+
}
188+
};
189+
assert_eq!(
190+
expand_error(multiple),
191+
"graph and instance business references require exactly one payload field"
192+
);
193+
}
194+
195+
#[test]
196+
fn accepts_all_unscoped_variant_shapes() {
197+
let input = parse_quote! {
198+
enum Message {
199+
#[business_ref(unscoped)]
200+
Unit,
201+
#[business_ref(unscoped)]
202+
Tuple(u8, u16),
203+
#[business_ref(unscoped)]
204+
Named { value: u8 },
205+
}
206+
};
207+
208+
assert!(expand_message_business_ref(input).is_ok());
209+
}
210+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
use node_macros::MessageBusinessRef;
2+
3+
#[derive(Debug, Eq, PartialEq)]
4+
enum BusinessRef {
5+
Instance { instance_id: u64 },
6+
Graph { instance_id: u64, graph_id: u64 },
7+
Unscoped,
8+
}
9+
10+
trait HasBusinessRef {
11+
fn business_ref(&self) -> BusinessRef;
12+
}
13+
14+
struct Payload {
15+
instance_id: u64,
16+
graph_id: u64,
17+
}
18+
19+
#[allow(dead_code)]
20+
#[derive(MessageBusinessRef)]
21+
enum Message {
22+
#[business_ref(graph)]
23+
Graph(Payload),
24+
#[business_ref(instance)]
25+
Instance(Payload),
26+
#[business_ref(unscoped)]
27+
Unit,
28+
#[business_ref(unscoped)]
29+
Tuple(u8, u16),
30+
#[business_ref(unscoped)]
31+
Named { value: u8 },
32+
}
33+
34+
#[test]
35+
fn derives_the_expected_business_reference_for_every_scope() {
36+
assert_eq!(
37+
Message::Graph(Payload { instance_id: 7, graph_id: 11 }).business_ref(),
38+
BusinessRef::Graph { instance_id: 7, graph_id: 11 }
39+
);
40+
assert_eq!(
41+
Message::Instance(Payload { instance_id: 7, graph_id: 11 }).business_ref(),
42+
BusinessRef::Instance { instance_id: 7 }
43+
);
44+
assert_eq!(Message::Unit.business_ref(), BusinessRef::Unscoped);
45+
assert_eq!(Message::Tuple(1, 2).business_ref(), BusinessRef::Unscoped);
46+
assert_eq!(Message::Named { value: 1 }.business_ref(), BusinessRef::Unscoped);
47+
}

‎crates/store/.sqlx/query-0d089d8a580c440ac9dda18b624a9244b2039b01d37b4f44177c6509a47ca762.json‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

0 commit comments

Comments
 (0)