Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): implement document transfers in WASM DPP #2406

Open
wants to merge 18 commits into
base: v1.8-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/js-dash-sdk/src/SDK/Client/Platform/Platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import createAssetLockTransaction from './createAssetLockTransaction';

import broadcastDocument from './methods/documents/broadcast';
import createDocument from './methods/documents/create';
import transferDocument from './methods/documents/transfer';
import getDocument from './methods/documents/get';

import publishContract from './methods/contracts/publish';
Expand Down Expand Up @@ -58,6 +59,7 @@ export interface PlatformOpts {
interface Records {
broadcast: Function,
create: Function,
transfer: Function,
get: Function,
}

Expand Down Expand Up @@ -165,6 +167,7 @@ export class Platform {
this.documents = {
broadcast: broadcastDocument.bind(this),
create: createDocument.bind(this),
transfer: transferDocument.bind(this),
get: getDocument.bind(this),
};
this.contracts = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default async function broadcast(
documents: {
create?: ExtendedDocument[],
replace?: ExtendedDocument[],
delete?: ExtendedDocument[]
delete?: ExtendedDocument[],
},
identity: any,
): Promise<any> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { Identity, ExtendedDocument } from '@dashevo/wasm-dpp';
import { Platform } from '../../Platform';
import broadcastStateTransition from '../../broadcastStateTransition';
import { signStateTransition } from '../../signStateTransition';
/**
* Transfer document in the platform
*
* @param {Platform} this - bound instance class
* @param {string} typeLocator - type locator
* @param identity - identity
* @param {Object} [data] - options
* @returns {StateTransition}
*/
export async function transfer(
this: Platform,
document: ExtendedDocument,
receiver: Identity,
sender: Identity,
): Promise<any> {
this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString} to {${receiver.getId().toString()}`);
await this.initialize();

const identityContractNonce = await this.nonceManager
.bumpIdentityContractNonce(sender.getId(), document.getDataContractId());

const documentsBatchTransition = document
.createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));

await signStateTransition(this, documentsBatchTransition, sender, 1);

await broadcastStateTransition(this, documentsBatchTransition);
Comment on lines +20 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling for external calls

The function makes several external calls without proper error handling, which could lead to unhandled rejections.

+  try {
     await this.initialize();
 
     const identityContractNonce = await this.nonceManager
       .bumpIdentityContractNonce(sender.getId(), document.getDataContractId());
 
     const documentsBatchTransition = document
       .createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));
 
     await signStateTransition(this, documentsBatchTransition, sender, 1);
 
     await broadcastStateTransition(this, documentsBatchTransition);
+    return documentsBatchTransition;
+  } catch (error) {
+    this.logger.error('[Document#transfer] Failed to transfer document:', error);
+    throw error;
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await this.initialize();
const identityContractNonce = await this.nonceManager
.bumpIdentityContractNonce(sender.getId(), document.getDataContractId());
const documentsBatchTransition = document
.createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));
await signStateTransition(this, documentsBatchTransition, sender, 1);
await broadcastStateTransition(this, documentsBatchTransition);
try {
await this.initialize();
const identityContractNonce = await this.nonceManager
.bumpIdentityContractNonce(sender.getId(), document.getDataContractId());
const documentsBatchTransition = document
.createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));
await signStateTransition(this, documentsBatchTransition, sender, 1);
await broadcastStateTransition(this, documentsBatchTransition);
return documentsBatchTransition;
} catch (error) {
this.logger.error('[Document#transfer] Failed to transfer document:', error);
throw error;
}

Comment on lines +13 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation for required parameters

The function should validate its input parameters before proceeding with the transfer operation.

 export async function transfer(
   this: Platform,
   document: ExtendedDocument,
   receiver: Identity,
   sender: Identity,
 ): Promise<any> {
+  if (!document || !receiver || !sender) {
+    throw new Error('Document, receiver, and sender are required parameters');
+  }
+
+  if (sender.getId().equals(receiver.getId())) {
+    throw new Error('Sender and receiver cannot be the same identity');
+  }
+
   this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()}...
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function transfer(
this: Platform,
document: ExtendedDocument,
receiver: Identity,
sender: Identity,
): Promise<any> {
this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString} to {${receiver.getId().toString()}`);
await this.initialize();
const identityContractNonce = await this.nonceManager
.bumpIdentityContractNonce(sender.getId(), document.getDataContractId());
const documentsBatchTransition = document
.createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));
await signStateTransition(this, documentsBatchTransition, sender, 1);
await broadcastStateTransition(this, documentsBatchTransition);
export async function transfer(
this: Platform,
document: ExtendedDocument,
receiver: Identity,
sender: Identity,
): Promise<any> {
if (!document || !receiver || !sender) {
throw new Error('Document, receiver, and sender are required parameters');
}
if (sender.getId().equals(receiver.getId())) {
throw new Error('Sender and receiver cannot be the same identity');
}
this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString} to {${receiver.getId().toString()}`);
await this.initialize();
const identityContractNonce = await this.nonceManager
.bumpIdentityContractNonce(sender.getId(), document.getDataContractId());
const documentsBatchTransition = document
.createTransferStateTransition(receiver.getId(), BigInt(identityContractNonce));
await signStateTransition(this, documentsBatchTransition, sender, 1);
await broadcastStateTransition(this, documentsBatchTransition);

}

export default transfer;
1 change: 1 addition & 0 deletions packages/rs-dpp/src/document/document_factory/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
DocumentsBatchTransition, DocumentsBatchTransitionV0,
};
use itertools::Itertools;
use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition;

Check warning on line 35 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (dpp) / Linting

unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition`

warning: unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition` --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:35:5 | 35 | use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

Check warning on line 35 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (drive) / Linting

unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition`

warning: unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition` --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:35:5 | 35 | use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

Check warning on line 35 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (dash-sdk) / Linting

unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition`

warning: unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition` --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:35:5 | 35 | use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

Check warning on line 35 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (wasm-dpp) / Linting

unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition`

warning: unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition` --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:35:5 | 35 | use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

Check warning on line 35 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (drive-abci) / Linting

unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition`

warning: unused import: `crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition` --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:35:5 | 35 | use crate::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

const PROPERTY_FEATURE_VERSION: &str = "$version";
const PROPERTY_ENTROPY: &str = "$entropy";
Expand Down Expand Up @@ -212,10 +213,10 @@
nonce_counter: &mut BTreeMap<(Identifier, Identifier), u64>, //IdentityID/ContractID -> nonce
) -> Result<DocumentsBatchTransition, ProtocolError> {
let platform_version = PlatformVersion::get(self.protocol_version)?;
let documents: Vec<(
DocumentTransitionActionType,
Vec<(Document, DocumentTypeRef, Bytes32)>,
)> = documents_iter.into_iter().collect();

Check warning on line 219 in packages/rs-dpp/src/document/document_factory/v0/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (dpp) / Linting

very complex type used. Consider factoring parts into `type` definitions

warning: very complex type used. Consider factoring parts into `type` definitions --> packages/rs-dpp/src/document/document_factory/v0/mod.rs:216:24 | 216 | let documents: Vec<( | ________________________^ 217 | | DocumentTransitionActionType, 218 | | Vec<(Document, DocumentTypeRef, Bytes32)>, 219 | | )> = documents_iter.into_iter().collect(); | |__________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity = note: `#[warn(clippy::type_complexity)]` on by default
let mut flattened_documents_iter = documents.iter().flat_map(|(_, v)| v).peekable();

let Some((first_document, _, _)) = flattened_documents_iter.peek() else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ impl DocumentsBatchTransitionInternalTransformerV0 for DocumentsBatchTransition
StateError::InvalidDocumentRevisionError(InvalidDocumentRevisionError::new(
document_id,
Some(previous_revision),
transition_revision,
expected_revision,
)),
))
}
Expand Down
36 changes: 32 additions & 4 deletions packages/wasm-dpp/src/document/extended_document.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use dpp::document::{
DocumentV0Getters, DocumentV0Setters, ExtendedDocument, EXTENDED_DOCUMENT_IDENTIFIER_FIELDS,
};
use dpp::document::{DocumentV0Getters, DocumentV0Setters, ExtendedDocument, EXTENDED_DOCUMENT_IDENTIFIER_FIELDS};
use serde_json::Value as JsonValue;

use dpp::platform_value::{Bytes32, Value};
use dpp::prelude::{Identifier, Revision, TimestampMillis};
use dpp::prelude::{Identifier, IdentityNonce, Revision, TimestampMillis, UserFeeIncrease};

Check warning on line 5 in packages/wasm-dpp/src/document/extended_document.rs

View workflow job for this annotation

GitHub Actions / Rust packages (wasm-dpp) / Linting

unused import: `UserFeeIncrease`

warning: unused import: `UserFeeIncrease` --> packages/wasm-dpp/src/document/extended_document.rs:5:74 | 5 | use dpp::prelude::{Identifier, IdentityNonce, Revision, TimestampMillis, UserFeeIncrease}; | ^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

use dpp::util::json_value::JsonValueExt;

Expand All @@ -16,12 +14,15 @@
use serde::{Deserialize, Serialize};
use std::convert::TryInto;
use wasm_bindgen::prelude::*;
use dpp::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition;
use dpp::state_transition::documents_batch_transition::{DocumentsBatchTransition, DocumentsBatchTransitionV0};

use crate::buffer::Buffer;
use crate::data_contract::DataContractWasm;
#[allow(deprecated)] // BinaryType is unsed in unused code below
use crate::document::BinaryType;
use crate::document::{ConversionOptions, DocumentWasm};
use crate::document_batch_transition::DocumentsBatchTransitionWasm;
use crate::errors::RustConversionError;
use crate::identifier::{identifier_from_js_value, IdentifierWrapper};
use crate::lodash::lodash_set;
Expand Down Expand Up @@ -235,6 +236,33 @@
.set_created_at(ts.map(|t| t.get_time() as TimestampMillis));
}

#[wasm_bindgen(js_name=createTransferStateTransition)]
pub fn create_transfer_state_transition(&mut self, recipient: IdentifierWrapper, identity_contract_nonce: IdentityNonce) -> DocumentsBatchTransitionWasm {
let mut cloned_document = self.0.document().clone();

cloned_document.set_revision(Some(cloned_document.revision().unwrap() + 1));

let transfer_transition = DocumentTransferTransition::from_document(
cloned_document,
self.0.document_type().unwrap(),
identity_contract_nonce,
recipient.into(),
PlatformVersion::latest(),
None,
None,
).unwrap();

let documents_batch_transition: DocumentsBatchTransition = DocumentsBatchTransitionV0 {
owner_id: self.0.owner_id(),
transitions: vec![transfer_transition.into()],
user_fee_increase: Default::default(),
signature_public_key_id: Default::default(),
signature: Default::default(),
}.into();

documents_batch_transition.into()
}
Comment on lines +239 to +264
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Check for None revision before incrementing.
The method does not verify cloned_document.revision() is Some before computing unwrap() + 1. If a document is missing a revision, this will panic. To prevent runtime errors, handle the case of None:

     let mut cloned_document = self.0.document().clone();

-    cloned_document.set_revision(Some(cloned_document.revision().unwrap() + 1));
+    if let Some(current_revision) = cloned_document.revision() {
+        cloned_document.set_revision(Some(current_revision + 1));
+    } else {
+        // Decide how to handle the absence of a revision, e.g., return an error or set a default revision
+        return Err(JsValue::from_str("Document is missing a revision"));
+    }

Committable suggestion skipped: line range outside the PR's diff.


#[wasm_bindgen(js_name=setUpdatedAt)]
pub fn set_updated_at(&mut self, ts: Option<js_sys::Date>) {
self.0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use wasm_bindgen::prelude::wasm_bindgen;
use dpp::state_transition::documents_batch_transition::document_transition::DocumentTransferTransition;

#[wasm_bindgen(js_name=DocumentTransferTransition)]
#[derive(Debug, Clone)]
pub struct DocumentTransferTransitionWasm {
inner: DocumentTransferTransition,
}

impl From<DocumentTransferTransition> for DocumentTransferTransitionWasm {
fn from(v: DocumentTransferTransition) -> Self {
Self { inner: v }
}
}

impl From<DocumentTransferTransitionWasm> for DocumentTransferTransition {
fn from(v: DocumentTransferTransitionWasm) -> Self {
v.inner
}
}

#[wasm_bindgen(js_class=DocumentTransferTransition)]

Check warning on line 22 in packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs

View workflow job for this annotation

GitHub Actions / Rust packages (wasm-dpp) / Linting

unused variable: `js_class`

warning: unused variable: `js_class` --> packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs:22:16 | 22 | #[wasm_bindgen(js_class=DocumentTransferTransition)] | ^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_js_class` | = note: `#[warn(unused_variables)]` on by default
impl DocumentTransferTransitionWasm {
}

impl DocumentTransferTransitionWasm {
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod document_create_transition;
mod document_transfer_transition;
// mod document_delete_transition;
// mod document_replace_transition;

Expand Down
Loading