Skip to content

Commit

Permalink
fix(platform)!: document types should not have a contested unique ind…
Browse files Browse the repository at this point in the history 
…ex with a unique index (#1984)
  • Loading branch information
@QuantumExplorer
QuantumExplorer authored Jul 23, 2024
1 parent d3170b9 commit 319c597
Show file tree
Hide file tree
Showing 328 changed files with 3,073 additions and 410 deletions.
33 changes: 6 additions & 27 deletions packages/dpns-contract/schema/v1/dpns-contract-documents.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,11 @@
}
},
{
"name": "dashIdentityId",
"name": "identityId",
"nullSearchable": false,
"properties": [
{
"records.dashUniqueIdentityId": "asc"
}
],
"unique": true
},
{
"name": "dashAlias",
"properties": [
{
"records.dashAliasIdentityId": "asc"
"records.identity": "asc"
}
]
}
Expand Down Expand Up @@ -91,32 +83,19 @@
"records": {
"type": "object",
"properties": {
"dashUniqueIdentityId": {
"type": "array",
"byteArray": true,
"minItems": 32,
"maxItems": 32,
"position": 0,
"contentMediaType": "application/x.dash.dpp.identifier",
"description": "Identity ID to be used to create the primary name the Identity",
"$comment": "Must be equal to the document owner"
},
"dashAliasIdentityId": {
"identity": {
"type": "array",
"byteArray": true,
"minItems": 32,
"maxItems": 32,
"position": 1,
"contentMediaType": "application/x.dash.dpp.identifier",
"description": "Identity ID to be used to create alias names for the Identity",
"$comment": "Must be equal to the document owner"
"description": "Identifier name record that refers to an Identity"
}
},
"minProperties": 1,
"maxProperties": 1,
"position": 5,
"additionalProperties": false,
"$comment": "Constraint with max and min properties ensure that only one identity record is used - either a `dashUniqueIdentityId` or a `dashAliasIdentityId`"
"additionalProperties": false
},
"subdomainRules": {
"type": "object",
Expand Down
52 changes: 4 additions & 48 deletions packages/dpns-contract/test/unit/dpnsContract.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('DPNS Contract', () => {
normalizedParentDomainName: 'dash',
preorderSalt: crypto.randomBytes(32),
records: {
dashUniqueIdentityId: await generateRandomIdentifier(),
identity: await generateRandomIdentifier(),
},
subdomainRules: {
allowSubdomains: false,
Expand Down Expand Up @@ -468,54 +468,10 @@ describe('DPNS Contract', () => {
});

describe('Dash Identity', () => {
it('should have either `dashUniqueIdentityId` or `dashAliasIdentityId`', async () => {
rawDomainDocument.records = {
dashUniqueIdentityId: identityId,
dashAliasIdentityId: identityId,
};

const document = dpp.document.create(dataContract, identityId, 'domain', rawDomainDocument);
const validationResult = document.validate(dpp.protocolVersion);
const error = expectJsonSchemaError(validationResult);

expect(error.keyword)
.to
.equal('maxProperties');
expect(error.instancePath)
.to
.equal('/records');
});

describe('dashUniqueIdentityId', () => {
it('should no less than 32 bytes', async () => {
rawDomainDocument.records = {
dashUniqueIdentityId: crypto.randomBytes(30),
};

expect(() => {
dpp.document.create(dataContract, identityId, 'domain', rawDomainDocument);
})
.to
.throw();
});

it('should no more than 32 bytes', async () => {
rawDomainDocument.records = {
dashUniqueIdentityId: crypto.randomBytes(64),
};

expect(() => {
dpp.document.create(dataContract, identityId, 'domain', rawDomainDocument);
})
.to
.throw();
});
});

describe('dashAliasIdentityId', () => {
describe('identity record', () => {
it('should no less than 32 bytes', async () => {
rawDomainDocument.records = {
dashAliasIdentityId: crypto.randomBytes(30),
identity: crypto.randomBytes(30),
};

expect(() => {
Expand All @@ -527,7 +483,7 @@ describe('DPNS Contract', () => {

it('should no more than 32 bytes', async () => {
rawDomainDocument.records = {
dashAliasIdentityId: crypto.randomBytes(64),
identity: crypto.randomBytes(64),
};

expect(() => {
Expand Down
18 changes: 9 additions & 9 deletions packages/platform-test-suite/test/e2e/dpns.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('DPNS', () => {
// skip if DPNS owner private key is not passed and use `dash` in tests above
it('should be able to register a TLD', async () => {
createdTLD = await ownerClient.platform.names.register(newTopLevelDomain, {
dashAliasIdentityId: identity.getId(),
identity: identity.getId(),
}, identity);

// Additional wait time to mitigate testnet latency
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('DPNS', () => {

try {
await client.platform.names.register(getRandomDomain(), {
dashAliasIdentityId: identity.getId(),
identity: identity.getId(),
}, identity);
} catch (e) {
broadcastError = e;
Expand All @@ -161,7 +161,7 @@ describe('DPNS', () => {

it('should be able to register a second level domain', async () => {
registeredDomain = await client.platform.names.register(`${secondLevelDomain}0.${topLevelDomain}`, {
dashUniqueIdentityId: identity.getId(),
identity: identity.getId(),
}, identity);

// Additional wait time to mitigate testnet latency
Expand All @@ -179,7 +179,7 @@ describe('DPNS', () => {
const domain = `${secondLevelDomain}O.${topLevelDomain}`;

await client.platform.names.register(domain, {
dashAliasIdentityId: identity.getId(),
identity: identity.getId(),
}, identity);

expect.fail('should throw error');
Expand All @@ -199,7 +199,7 @@ describe('DPNS', () => {
const domain = `${getRandomDomain()}.${getRandomDomain()}`;

await client.platform.names.register(domain, {
dashAliasIdentityId: identity.getId(),
identity: identity.getId(),
}, identity);

expect.fail('should throw error');
Expand Down Expand Up @@ -252,8 +252,8 @@ describe('DPNS', () => {

it('should be able to resolve domain by it\'s record', async () => {
const [document] = await client.platform.names.resolveByRecord(
'dashUniqueIdentityId',
registeredDomain.getData().records.dashUniqueIdentityId,
'identity',
registeredDomain.getData().records.identity,
);

const rawDocument = document.toObject();
Expand Down Expand Up @@ -310,9 +310,9 @@ describe('DPNS', () => {
expect(broadcastError.code).to.equal(40500);
});

it('should not be able to register two domains with same `dashAliasIdentityId` record');
it('should not be able to register two domains with same `identity` record');

it('should be able to register many domains with same `dashAliasIdentityId` record');
it('should be able to register many domains with same `identity` record');

it('should not be able to update preorder');

Expand Down
3 changes: 3 additions & 0 deletions packages/rs-dpp/schema/meta_schemas/document/v0/document-meta.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@
"unique": {
"type": "boolean"
},
"nullSearchable": {
"type": "boolean"
},
"contested": {
"type": "object",
"properties": {
Expand Down
41 changes: 41 additions & 0 deletions packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ use std::convert::TryInto;

#[cfg(feature = "validation")]
use crate::consensus::basic::data_contract::ContestedUniqueIndexOnMutableDocumentTypeError;
#[cfg(feature = "validation")]
use crate::consensus::basic::data_contract::ContestedUniqueIndexWithUniqueIndexError;
#[cfg(any(test, feature = "validation"))]
use crate::consensus::basic::data_contract::InvalidDocumentTypeNameError;
#[cfg(feature = "validation")]
Expand Down Expand Up @@ -50,6 +52,7 @@ use crate::version::PlatformVersion;
use crate::ProtocolError;
use platform_value::btreemap_extensions::BTreeValueMapHelper;
use platform_value::{Identifier, Value};

const NOT_ALLOWED_SYSTEM_PROPERTIES: [&str; 1] = ["$id"];

const SYSTEM_PROPERTIES: [&str; 11] = [
Expand Down Expand Up @@ -282,6 +285,12 @@ impl DocumentTypeV0 {
#[cfg(feature = "validation")]
let mut unique_indices_count = 0;

#[cfg(feature = "validation")]
let mut last_non_contested_unique_index_name: Option<String> = None;

#[cfg(feature = "validation")]
let mut last_contested_unique_index_name: Option<String> = None;

#[cfg(feature = "validation")]
let mut contested_indices_count = 0;

Expand Down Expand Up @@ -330,6 +339,23 @@ impl DocumentTypeV0 {
.into(),
)));
}

if let Some(last_contested_unique_index_name) =
last_contested_unique_index_name.as_ref()
{
return Err(ProtocolError::ConsensusError(Box::new(
ContestedUniqueIndexWithUniqueIndexError::new(
name.to_string(),
last_contested_unique_index_name.clone(),
index.name,
)
.into(),
)));
}

if index.contested_index.is_none() {
last_non_contested_unique_index_name = Some(index.name.clone());
}
}

if index.contested_index.is_some() {
Expand All @@ -355,6 +381,19 @@ impl DocumentTypeV0 {
)));
}

if let Some(last_unique_index_name) =
last_non_contested_unique_index_name.as_ref()
{
return Err(ProtocolError::ConsensusError(Box::new(
ContestedUniqueIndexWithUniqueIndexError::new(
name.to_string(),
index.name,
last_unique_index_name.clone(),
)
.into(),
)));
}

if documents_mutable {
return Err(ProtocolError::ConsensusError(Box::new(
ContestedUniqueIndexOnMutableDocumentTypeError::new(
Expand All @@ -364,6 +403,8 @@ impl DocumentTypeV0 {
.into(),
)));
}

last_contested_unique_index_name = Some(index.name.clone());
}

// Index names must be unique for the document type
Expand Down
12 changes: 12 additions & 0 deletions packages/rs-dpp/src/data_contract/document_type/index/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ pub struct Index {
pub name: String,
pub properties: Vec<IndexProperty>,
pub unique: bool,
/// Null searchable indicates what to do if all members of the index are null
/// If this is set to false then we do not insert references which makes such items non-searchable
pub null_searchable: bool,
/// Contested indexes are useful when a resource is considered valuable
pub contested_index: Option<ContestedIndexInformation>,
}
Expand Down Expand Up @@ -431,6 +434,9 @@ impl TryFrom<&[(Value, Value)]> for Index {
// For properties, we iterate each and move it to IndexProperty

let mut unique = false;
// The default for null searchable should be true. Do not change this without very
// careful thought and consideration.
let mut null_searchable = true;
let mut name = None;
let mut contested_index = None;
let mut index_properties: Vec<IndexProperty> = Vec::new();
Expand All @@ -454,6 +460,11 @@ impl TryFrom<&[(Value, Value)]> for Index {
unique = value_value.as_bool().expect("confirmed as bool");
}
}
"nullSearchable" => {
if value_value.is_bool() {
null_searchable = value_value.as_bool().expect("confirmed as bool");
}
}
"contested" => {
let contested_properties_value_map = value_value.to_map()?;

Expand Down Expand Up @@ -580,6 +591,7 @@ impl TryFrom<&[(Value, Value)]> for Index {
name,
properties: index_properties,
unique,
null_searchable,
contested_index,
})
}
Expand Down
1 change: 1 addition & 0 deletions packages/rs-dpp/src/data_contract/document_type/index/random_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ impl Index {
name: index_name,
properties,
unique,
null_searchable: true,
contested_index: None,
})
}
Expand Down
Loading

0 comments on commit 319c597

Please sign in to comment.