-
Notifications
You must be signed in to change notification settings - Fork 39
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
base: v1.8-dev
Are you sure you want to change the base?
Conversation
…/document-transfer
…/document-transfer
Warning Rate limit exceeded@pshenmic has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a new method, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts (1)
14-34
: Handle potential errors during the broadcast
This function signs and broadcasts the state transition without error handling (e.g., try/catch). If the broadcast fails, the user may not be alerted.Consider wrapping the call to
broadcastStateTransition
in a try/catch block or returning a structured error, for example:try { await broadcastStateTransition(this, documentsBatchTransition); } catch (e) { this.logger.error('[Document#transfer] Broadcast failed:', e); throw e; }packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts (1)
24-24
: Optional usage of keyIndex
The function signature expects anumber
, although the code useskeyIndex ?? 1
as a fallback. If you intend to allow undefined to set a default, consider making the parameter optional in TypeScript.-export default async function broadcast( - this: Platform, - documents: { ... }, - identity: any, - keyIndex : number, +export default async function broadcast( + this: Platform, + documents: { ... }, + identity: any, + keyIndex?: number, ) {packages/js-dash-sdk/src/SDK/Client/Platform/Platform.ts (1)
62-62
: Refine function typing inRecords
interface
UsingFunction
as a type is discouraged. Instead, define the function signature explicitly to reduce ambiguity and potential bugs.interface Records { broadcast: (documents: any, identity: any, keyIndex?: number) => Promise<any>, create: (/* define type signatures */) => Promise<any>, - transfer: Function, + transfer: (document: ExtendedDocument, receiver: Identity, sender: Identity) => Promise<any>, get: (/* define type signatures */) => any, }🧰 Tools
🪛 Biome (1.9.4)
[error] 62-62: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs (1)
22-256
: Consider removing or fully implementing the commented-out methods.
Leaving large code blocks commented out can lead to confusion and code bloat. If they are needed in the future, consider adding aTODO
comment explaining the plan and timeline. Otherwise, removing them reduces code noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/js-dash-sdk/src/SDK/Client/Platform/Platform.ts
(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts
(2 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts
(1 hunks)packages/rs-dpp/src/document/document_factory/v0/mod.rs
(3 hunks)packages/rs-dpp/src/document/specialized_document_factory/v0/mod.rs
(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/transformer/v0/mod.rs
(1 hunks)packages/wasm-dpp/src/document/extended_document.rs
(3 hunks)packages/wasm-dpp/src/document/factory.rs
(3 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs
(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/js-dash-sdk/src/SDK/Client/Platform/Platform.ts
[error] 62-62: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (17)
packages/rs-dpp/src/document/document_factory/v0/mod.rs (2)
35-35
: Import statement looks correct
No issues found with this import.
527-613
:
Potential ownership logic issue in document transfers
Line 552 assigns document.owner_id()
to recipient_owner_id
, suggesting the document remains under the same owner by default. This may not achieve the actual transfer of ownership unless the document's owner is explicitly overwritten somewhere else.
Consider verifying or updating the code to ensure a genuine transfer to the new owner. For instance:
- let recipient_owner_id = document.owner_id();
+ // Retrieve the intended recipient's ID from a separate source or function parameter
+ let recipient_owner_id = new_owner_id;
Likely invalid or redundant comment.
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts (3)
14-14
: New keyIndex parameter recognized
The documentation for keyIndex
is clear and consistent with usage.
21-21
: Additional 'delete' property
Adding this property for deleted documents is consistent.
57-57
: Signing with keyIndex
Using keyIndex ?? 1
ensures the default behavior remains the same.
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs (1)
2-2
: New document_transfer_transition
module imported
No issues found. This aligns with the newly introduced transition logic.
packages/js-dash-sdk/src/SDK/Client/Platform/Platform.ts (2)
12-12
: Import of transferDocument
Import statement is consistent and matches usage in this file.
170-170
: Binding the new transferDocument
method
Binding appears correct and ensures the documents.transfer
method is available.
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs (4)
1-2
: Good use of wasm-bindgen for bridging Rust to JS.
No issues spotted.
4-8
: Well-defined struct for WASM bridging.
The DocumentTransferTransitionWasm
struct straightforwardly wraps DocumentTransferTransition
, enabling seamless integration with JavaScript.
10-14
: Symmetrical type conversion from Rust to WASM.
The From<DocumentTransferTransition>
for DocumentTransferTransitionWasm
is clear and concise.
16-20
: Symmetrical type conversion from WASM to Rust.
This complementary conversion ensures consistency.
packages/wasm-dpp/src/document/factory.rs (3)
35-35
: New field supports Transfer transitions.
Adding transfer: Vec<ExtendedDocumentWasm>
cleanly parallels existing transition vectors (create
, replace
, etc.).
59-63
: Extend transition handling with a dedicated transfer list.
add_transition_transfer
follows the same pattern as other add methods, ensuring consistency.
287-287
: Document extraction for 'transfer' is consistent with other transitions.
The extended logic for extracting transfer documents aligns well with create
, replace
, delete
.
Also applies to: 292-292
packages/wasm-dpp/src/document/extended_document.rs (1)
1-1
: New imports for transfer transitions.
Imports for DocumentTransferTransition
and DocumentsBatchTransitionWasm
ensure the new feature is properly integrated.
Also applies to: 5-5, 17-18, 25-25
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/documents_batch/transformer/v0/mod.rs (1)
723-723
: Looks correct!
This change correctly ensures that the original document’s revision is incremented by one. The logic aligns well with the documented requirement to check that transition_revision == previous_revision + 1
.
DocumentTransitionActionType::Transfer => Self::document_transfer_transitions( | ||
documents | ||
.into_iter() | ||
.map(|(document, document_type, _)| (document, document_type)) | ||
.collect(), | ||
nonce_counter, | ||
platform_version, | ||
), | ||
DocumentTransitionActionType::Purchase => { | ||
Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for Transfer".to_string(), | ||
)) | ||
} | ||
DocumentTransitionActionType::UpdatePrice => Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for UpdatePrice".to_string(), | ||
)), | ||
DocumentTransitionActionType::IgnoreWhileBumpingRevision => Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for IgnoreWhileBumpingRevision".to_string(), | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mismatch in error message for Purchase
action
When DocumentTransitionActionType::Purchase
is encountered, the error message incorrectly states "action type not accounted for Transfer" instead of referring to "Purchase." This might confuse developers or users.
Consider applying the following fix:
- DocumentTransitionActionType::Purchase => {
- Err(ProtocolError::InvalidStateTransitionType(
- "action type not accounted for Transfer".to_string(),
- ))
- }
+ DocumentTransitionActionType::Purchase => {
+ Err(ProtocolError::InvalidStateTransitionType(
+ "action type not accounted for Purchase".to_string(),
+ ))
+ }
📝 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.
DocumentTransitionActionType::Transfer => Self::document_transfer_transitions( | |
documents | |
.into_iter() | |
.map(|(document, document_type, _)| (document, document_type)) | |
.collect(), | |
nonce_counter, | |
platform_version, | |
), | |
DocumentTransitionActionType::Purchase => { | |
Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for Transfer".to_string(), | |
)) | |
} | |
DocumentTransitionActionType::UpdatePrice => Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for UpdatePrice".to_string(), | |
)), | |
DocumentTransitionActionType::IgnoreWhileBumpingRevision => Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for IgnoreWhileBumpingRevision".to_string(), | |
)) | |
DocumentTransitionActionType::Transfer => Self::document_transfer_transitions( | |
documents | |
.into_iter() | |
.map(|(document, document_type, _)| (document, document_type)) | |
.collect(), | |
nonce_counter, | |
platform_version, | |
), | |
DocumentTransitionActionType::Purchase => { | |
Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for Purchase".to_string(), | |
)) | |
} | |
DocumentTransitionActionType::UpdatePrice => Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for UpdatePrice".to_string(), | |
)), | |
DocumentTransitionActionType::IgnoreWhileBumpingRevision => Err(ProtocolError::InvalidStateTransitionType( | |
"action type not accounted for IgnoreWhileBumpingRevision".to_string(), | |
)) |
#[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() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
DocumentTransitionActionType::Transfer => { | ||
Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for Transfer".to_string(), | ||
)) | ||
}, | ||
DocumentTransitionActionType::Purchase => { | ||
Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for Purchase".to_string(), | ||
)) | ||
} | ||
DocumentTransitionActionType::UpdatePrice => Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for UpdatePrice".to_string(), | ||
)), | ||
DocumentTransitionActionType::IgnoreWhileBumpingRevision => Err(ProtocolError::InvalidStateTransitionType( | ||
"action type not accounted for IgnoreWhileBumpingRevision".to_string(), | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing Transfer transition support.
Although transfer transitions are defined elsewhere, here the code returns “action type not accounted for Transfer” as an error. This creates an inconsistency with other segments that do implement transfer logic. Consider implementing the logic or removing the partial references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts (2)
5-13
: Update JSDoc to match the actual parametersThe JSDoc parameters don't match the function's actual parameters. The
typeLocator
anddata
parameters are documented but not used, whiledocument
,receiver
, andsender
parameters are missing from the documentation.Update the JSDoc to:
/** * Transfer document in the platform * * @param {Platform} this - bound instance class - * @param {string} typeLocator - type locator - * @param identity - identity - * @param {Object} [data] - options + * @param {ExtendedDocument} document - document to transfer + * @param {Identity} receiver - receiver identity + * @param {Identity} sender - sender identity * @returns {StateTransition} */
14-19
: Improve type safety by specifying the return typeThe function's return type
Promise<any>
is too broad and could lead to type-safety issues.-): Promise<any> { +): Promise<StateTransition> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts
(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts
(1 hunks)packages/rs-dpp/src/document/document_factory/v0/mod.rs
(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts
- packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_transfer_transition.rs
🔇 Additional comments (1)
packages/rs-dpp/src/document/document_factory/v0/mod.rs (1)
35-35
: LGTM!
The addition of the DocumentTransferTransition
import is necessary for implementing the document transfer functionality.
this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} | ||
from ${sender.getId().toString} to {${receiver.getId().toString()}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix template literal syntax error in debug message
The debug message has a syntax error in the template literal - missing closing curly brace and using a method reference instead of a method call.
- this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()}
- from ${sender.getId().toString} to {${receiver.getId().toString()}`);
+ this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString()} to ${receiver.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.
this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} | |
from ${sender.getId().toString} to {${receiver.getId().toString()}`); | |
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); |
There was a problem hiding this comment.
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.
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; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts (2)
20-20
:⚠️ Potential issueFix template literal syntax error in debug message
The debug message has a syntax error in the template literal - missing closing curly brace and using a method reference instead of a method call.
- this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString} to {${receiver.getId().toString()}`); + this.logger.debug(`[Document#transfer] Transfer document ${document.getId().toString()} from ${sender.getId().toString()} to ${receiver.getId().toString()}`);
21-31
:⚠️ Potential issueAdd 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; + }
🧹 Nitpick comments (2)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts (2)
5-13
: Update JSDoc to match implementationThe JSDoc comments need to be updated to accurately reflect the function signature and return type:
- Parameter list doesn't match implementation (
typeLocator
vs actual parameters)- Return type annotation shows
StateTransition
but function returnsPromise<any>
/** * 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} + * @param {Platform} this - Platform instance + * @param {ExtendedDocument} document - Document to transfer + * @param {Identity} receiver - Receiver's identity + * @param {Identity} sender - Sender's identity + * @returns {Promise<any>} The broadcast state transition */
14-19
: Improve type safety by using specific return typeThe return type
Promise<any>
is too generic. Consider using the specific state transition type from the WASM DPP package.export async function transfer( this: Platform, document: ExtendedDocument, receiver: Identity, sender: Identity, -): Promise<any> { +): Promise<DocumentsBatchTransition> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts
(1 hunks)
🔇 Additional comments (1)
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/transfer.ts (1)
34-34
: LGTM!
The default export is correctly implemented.
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); |
There was a problem hiding this comment.
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.
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); |
Issue being fixed or feature implemented
WASM DPP package is missing few document transitions implementation, one of it is a document transfer. Adding new method binding in the wasm dpp will allow developers to create such transactions from the Web
What was done?
How Has This Been Tested?
In the testnet
https://testnet.platform-explorer.com/transaction/42175A63E25A316B5E0666317A3FF36A407EDDBA01D844982F68A6D10CBBFAF7
Breaking Changes
No
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
transferDocument
method for document management.transfer
function for transferring documents.broadcast
function with an optionalkeyIndex
parameter.create_transfer_state_transition
method for document state transitions.DocumentTransferTransitionWasm
structure for WebAssembly compatibility.Bug Fixes
Documentation
transfer
function andkeyIndex
parameter.