Skip to content

Commit e8e1961

Browse files
fix(sdk): align DPNS builder validation with consensus, harden embedder seams
Review follow-ups on the transport-free series: - The DPNS document builder validated labels with is_valid_username, whose consecutive-hyphen rejection is stricter than the DPNS contract's schema pattern - consensus accepts names like ab--cd. Split the check: new is_consensus_valid_label matches the contract pattern exactly and gates the builder (so dash-sdk's register_dpns_name no longer refuses consensus-valid labels), while is_valid_username keeps the stricter policy for its existing FFI/wasm gates and now documents the difference. - verify_documents_response rejects aggregate projections (COUNT/SUM/AVG) up front with a pointer to the aggregate proof helpers, instead of surfacing an opaque low-level proof error; try_from_request documents that it mirrors the server's wire-shape decode, not validate_and_route business rules. - The CI transport-leak guard also asserts wasm-sdk's wasm32 tree stays free of the native transport stack. - The proof-vector corpus gains a README with an explicit coverage matrix: the four documents-family cases pin query shape and clean decode failure but stop before the BLS check (placeholder payloads in the fixture state); identity, contested, and quorum-sig families run the full pipeline. This corrects the corpus commit's broader claim.
1 parent 3ae7752 commit e8e1961

2 files changed

Lines changed: 74 additions & 43 deletions

File tree

packages/dash-platform-queries/src/documents/document_query.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,15 @@ impl DocumentQuery {
400400
/// `contract` must be the data contract the request targets — the
401401
/// request's `data_contract_id` is checked against `contract.id()`
402402
/// and the named document type must exist on it.
403+
///
404+
/// Scope caveat: this mirrors the server's *wire-shape* decoding
405+
/// (shared clause decoders), not its full `validate_and_route`
406+
/// business rules — e.g. SUM/AVG requiring a non-empty field,
407+
/// GROUP BY being illegal with SELECT DOCUMENTS, or HAVING being
408+
/// unimplemented are enforced server-side only. A request violating
409+
/// those decodes here but can never yield a provable response from
410+
/// a real server, so this only matters for fabricated
411+
/// request/response pairs.
403412
pub fn try_from_request(
404413
request: GetDocumentsRequest,
405414
contract: Arc<DataContract>,
@@ -653,6 +662,19 @@ pub fn verify_documents_response(
653662
error: format!("failed to decode GetDocumentsRequest into a DocumentQuery: {e}"),
654663
}
655664
})?;
665+
// This entry point verifies plain document fetches only. An aggregate
666+
// projection (COUNT/SUM/AVG) is proved with a different proof shape;
667+
// handing it to the Documents verifier would surface as an opaque
668+
// low-level proof error, so reject it up front instead.
669+
if query.select != drive::query::SelectProjection::documents() {
670+
return Err(drive_proof_verifier::Error::RequestError {
671+
error: format!(
672+
"verify_documents_response only verifies plain document fetches; the request \
673+
carries a {:?} projection — use the aggregate proof helpers instead",
674+
query.select.function
675+
),
676+
});
677+
}
656678
<Documents as FromProof<DocumentQuery>>::maybe_from_proof_with_metadata(
657679
query,
658680
response,

packages/dash-platform-queries/src/dpns_usernames.rs

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ fn hash_double(data: Vec<u8>) -> [u8; 32] {
3333
/// `salt`, whose double-SHA256 over `salt ‖ "<normalized label>.dash"`
3434
/// becomes the preorder's `saltedDomainHash`.
3535
///
36-
/// The `label` must satisfy [`is_valid_username`]; the raw label is stored
36+
/// The `label` must satisfy [`is_consensus_valid_label`]; the raw label is stored
3737
/// in the domain document's `label` property while its
3838
/// [homograph-safe](convert_to_homograph_safe_chars) form is stored in
3939
/// `normalizedLabel`.
@@ -46,10 +46,10 @@ pub fn build_dpns_preorder_and_domain_documents(
4646
entropy: [u8; 32],
4747
salt: [u8; 32],
4848
) -> Result<(Document, Document), Error> {
49-
if !is_valid_username(label) {
49+
if !is_consensus_valid_label(label) {
5050
return Err(Error::InvalidInput(format!(
5151
"Invalid DPNS label \"{label}\": must be 3-63 characters, alphanumeric and hyphens \
52-
only, starting and ending with an alphanumeric character, without consecutive hyphens"
52+
only, starting and ending with an alphanumeric character"
5353
)));
5454
}
5555

@@ -161,15 +161,34 @@ pub fn convert_to_homograph_safe_chars(input: &str) -> String {
161161
.collect()
162162
}
163163

164-
/// Check if a username is valid according to DPNS rules
165-
///
166-
/// A username is valid if:
167-
/// - It's between 3 and 63 characters long
168-
/// - It starts and ends with alphanumeric characters (a-zA-Z0-9)
169-
/// - It contains only alphanumeric characters and hyphens
170-
/// - It doesn't have consecutive hyphens (enforced by the pattern)
164+
/// Check whether a label satisfies the DPNS contract's `label` schema
165+
/// pattern — exactly what consensus enforces, nothing stricter.
171166
///
172167
/// Pattern: `^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9]$`
168+
/// (3-63 characters, alphanumeric and hyphens, alphanumeric at both ends;
169+
/// consecutive hyphens ARE allowed by consensus).
170+
pub fn is_consensus_valid_label(label: &str) -> bool {
171+
if label.len() < 3 || label.len() > 63 {
172+
return false;
173+
}
174+
let chars: Vec<char> = label.chars().collect();
175+
if !chars[0].is_ascii_alphanumeric() || !chars[chars.len() - 1].is_ascii_alphanumeric() {
176+
return false;
177+
}
178+
chars[1..chars.len() - 1]
179+
.iter()
180+
.all(|&ch| ch.is_ascii_alphanumeric() || ch == '-')
181+
}
182+
183+
/// Check if a username is valid according to this crate's recommended
184+
/// client-side policy: the consensus pattern plus a stricter rejection of
185+
/// consecutive hyphens.
186+
///
187+
/// This is deliberately narrower than [`is_consensus_valid_label`] — a name
188+
/// like `ab--cd` is consensus-valid but rejected here, matching the
189+
/// pre-existing policy of the mobile SDK FFI and wasm-sdk gates. Callers
190+
/// that must accept every consensus-valid label should use
191+
/// [`is_consensus_valid_label`] instead.
173192
///
174193
/// # Arguments
175194
///
@@ -179,38 +198,7 @@ pub fn convert_to_homograph_safe_chars(input: &str) -> String {
179198
///
180199
/// Returns `true` if the username is valid, `false` otherwise
181200
pub fn is_valid_username(label: &str) -> bool {
182-
// Check length
183-
if label.len() < 3 || label.len() > 63 {
184-
return false;
185-
}
186-
187-
let chars: Vec<char> = label.chars().collect();
188-
189-
// Check first character (must be alphanumeric)
190-
if !chars[0].is_ascii_alphanumeric() {
191-
return false;
192-
}
193-
194-
// Check last character (must be alphanumeric)
195-
if !chars[chars.len() - 1].is_ascii_alphanumeric() {
196-
return false;
197-
}
198-
199-
// Check middle characters (can be alphanumeric or hyphen)
200-
for &ch in &chars[1..chars.len() - 1] {
201-
if !ch.is_ascii_alphanumeric() && ch != '-' {
202-
return false;
203-
}
204-
}
205-
206-
// Additional check: no consecutive hyphens (good practice)
207-
for i in 0..chars.len() - 1 {
208-
if chars[i] == '-' && chars[i + 1] == '-' {
209-
return false;
210-
}
211-
}
212-
213-
true
201+
is_consensus_valid_label(label) && !label.contains("--")
214202
}
215203

216204
/// Check if a username is contested (requires masternode voting)
@@ -367,7 +355,7 @@ mod tests {
367355
let contract = dpns_contract();
368356
let identity_id = Identifier::from([2u8; 32]);
369357

370-
for bad in ["", "ab", "-alice", "alice-", "alice--bob", "alice_bob"] {
358+
for bad in ["", "ab", "-alice", "alice-", "alice_bob"] {
371359
let result = build_dpns_preorder_and_domain_documents(
372360
&contract,
373361
identity_id,
@@ -382,6 +370,27 @@ mod tests {
382370
}
383371
}
384372

373+
/// Consecutive hyphens are consensus-valid (the DPNS contract pattern
374+
/// `^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9]$` allows them), so the
375+
/// builder must accept them even though the stricter client-side
376+
/// [`is_valid_username`] policy rejects them.
377+
#[test]
378+
fn build_dpns_documents_accepts_consensus_valid_double_hyphen() {
379+
let contract = dpns_contract();
380+
let identity_id = Identifier::from([2u8; 32]);
381+
382+
assert!(is_consensus_valid_label("alice--bob"));
383+
assert!(!is_valid_username("alice--bob"));
384+
build_dpns_preorder_and_domain_documents(
385+
&contract,
386+
identity_id,
387+
"alice--bob",
388+
[3u8; 32],
389+
[4u8; 32],
390+
)
391+
.expect("consensus-valid label with consecutive hyphens must build");
392+
}
393+
385394
#[test]
386395
fn test_convert_to_homograph_safe_chars() {
387396
assert_eq!(convert_to_homograph_safe_chars("alice"), "a11ce");

0 commit comments

Comments
 (0)