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

sncast_std appendix in cairo code #2262

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Arcticae
Copy link
Contributor

@Arcticae Arcticae commented Jun 27, 2024

Closes #2015

Introduced changes

  • Moves the docs to the appendix in preparation for scarb doc

Follow up for this: #2263

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added relevant tests
  • Performed self-review of the code
  • Added changes to CHANGELOG.md

Copy link
Contributor

@tomek0123456789 tomek0123456789 left a comment

Choose a reason for hiding this comment

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

good job 👍😎👍

/// - `contract_address` - address of the contract which will be called
/// - `function_selector` - hashed name of the target function (can be obtained with `selector!` macro)
/// - `calldata` - calldata for the function inputs, serialized with `Serde`
/// Returns [`CallResult`] with the returned data from the called function, or [`ScriptCommandError`] in case of failure
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns [`CallResult`] with the returned data from the called function, or [`ScriptCommandError`] in case of failure
/// Returns [`CallResult`] with the returned data from the called function, or [`ScriptCommandError`] in case of failure

pub class_hash: ClassHash,
/// A hash which references the transaction in which the declaration was made
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// A hash which references the transaction in which the declaration was made
/// Hash of the transaction in which it was declared

#[derive(Drop, Clone, Debug, Serde)]
pub struct DeclareResult {
/// Resulting class hash which was declared during the transaction
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Resulting class hash which was declared during the transaction
/// Resulting class hash which was declared by the transaction

@@ -319,10 +452,12 @@ pub impl DisplayExecutionStatus of Display<ExecutionStatus> {
}
}


/// A structure representing status of the transaction execution after its' submission
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// A structure representing status of the transaction execution after its' submission
/// A structure representing status of the transaction execution after its submission

@@ -303,9 +433,12 @@ pub impl DisplayFinalityStatus of Display<FinalityStatus> {
}


/// Result of cairo code execution for the transaction after validation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Result of cairo code execution for the transaction after validation
/// Result of Cairo code execution for the transaction after validation

/// If not provided, max fee will be automatically estimated via estimation endpoint.
/// - `nonce` - Account nonce for declare transaction.
/// If not provided, nonce will be set automatically (by fetching it from the account contract in the background).
/// Returns `InvokeResult` or `ScriptCommandError` if *submitting* the transaction failed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns `InvokeResult` or `ScriptCommandError` if *submitting* the transaction failed
/// Returns `InvokeResult`, or `ScriptCommandError` if *submitting* the transaction failed

@@ -276,17 +387,36 @@ pub fn invoke(
result_data
}

/// Gets nonce of an account for a given block tag
/// - `block_tag` - block tag name in form of shortstring (one of 'pending' or 'latest').
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// - `block_tag` - block tag name in form of shortstring (one of 'pending' or 'latest').
/// - `block_tag` - block tag in form of a shortstring ('pending' or 'latest').

block tag is self-explanatory

#[derive(Drop, Clone, Debug, Serde, PartialEq)]
pub enum FinalityStatus {
/// Transaction was received by the node successfully
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if these sentences shouldn't be in present perfect (has been received) etc.

pub finality_status: FinalityStatus,
/// Result of the cairo code execution. Present when the transaction was actually included in the block.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Result of the cairo code execution. Present when the transaction was actually included in the block.
/// Result of the Cairo code execution. Present when the transaction was actually included in the block.


/// Gets the status of a transaction. Useful for assessment of completion status for the transaction.
/// - `transaction_hash` - A `felt252` representing hash of the transaction (i.e. result of `deploy`/`invoke`/`declare`)
/// Returns `TxStatusResult` or `ScriptCommandError` if retrieving the status failed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns `TxStatusResult` or `ScriptCommandError` if retrieving the status failed
/// Returns `TxStatusResult`, or `ScriptCommandError` if retrieving the status failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move appendix docs to cairo
2 participants