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

chore: cleanup internals #329

Merged
merged 7 commits into from Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
1 change: 0 additions & 1 deletion workspaces/src/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ impl Transaction {
/// Transfer `deposit` amount from `signer`'s account into `receiver_id`'s account.
pub fn transfer(mut self, deposit: Balance) -> Self {
if let Ok(actions) = &mut self.actions {
let deposit = deposit;
actions.push(TransferAction { deposit }.into());
}
self
Expand Down
29 changes: 8 additions & 21 deletions workspaces/src/rpc/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use tokio::sync::RwLock;
use tokio_retry::strategy::{jitter, ExponentialBackoff};
use tokio_retry::Retry;

use near_crypto::Signer;
use near_jsonrpc_client::errors::{JsonRpcError, JsonRpcServerError};
use near_jsonrpc_client::methods::health::RpcStatusError;
use near_jsonrpc_client::methods::tx::RpcTransactionError;
Expand Down Expand Up @@ -46,7 +45,7 @@ use {
use crate::error::{Error, ErrorKind, RpcErrorCode};
use crate::operations::TransactionStatus;
use crate::result::Result;
use crate::types::{AccountId, InMemorySigner, Nonce, PublicKey};
use crate::types::{signed_transaction, AccountId, InMemorySigner, Nonce, PublicKey};
use crate::{Network, Worker};

pub(crate) const DEFAULT_CALL_FN_GAS: NearGas = NearGas::from_tgas(10);
Expand Down Expand Up @@ -559,22 +558,13 @@ pub(crate) async fn send_batch_tx_and_retry(
receiver_id: &AccountId,
actions: Vec<Action>,
) -> Result<FinalExecutionOutcomeView> {
let signer = signer.inner();
let cache_key = (signer.account_id.clone(), signer.public_key());

let cache_key = (signer.account_id.clone(), signer.secret_key.public_key().0);
retry(|| async {
let (block_hash, nonce) = fetch_tx_nonce(client, &cache_key).await?;
send_tx(
client,
&cache_key,
SignedTransaction::from_actions(
nonce,
signer.account_id.clone(),
receiver_id.clone(),
&signer as &dyn Signer,
actions.clone(),
block_hash,
),
signed_transaction(signer, actions.clone(), nonce, block_hash, receiver_id),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer constructor method (from_actions or maybe other name) on SignedTransaction over a standalone function as it is harder to discover the function. What is exactly the motivation behind this change?

Copy link
Author

Choose a reason for hiding this comment

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

The function is a reimplementation of from_actions without requiring Signer trait as an argument. The hint I took here is that we can condense that implementation and remove the reliance on the trait.
I could name it better.

)
.await
})
Expand All @@ -587,21 +577,18 @@ pub(crate) async fn send_batch_tx_async_and_retry(
receiver_id: &AccountId,
actions: Vec<Action>,
) -> Result<TransactionStatus> {
let signer = signer.inner();
let cache_key = (signer.account_id.clone(), signer.public_key());

let cache_key = (signer.account_id.clone(), signer.secret_key.public_key().0);
retry(|| async {
let (block_hash, nonce) = fetch_tx_nonce(worker.client(), &cache_key).await?;
let hash = worker
.client()
.query(&methods::broadcast_tx_async::RpcBroadcastTxAsyncRequest {
signed_transaction: SignedTransaction::from_actions(
nonce,
signer.account_id.clone(),
receiver_id.clone(),
&signer as &dyn Signer,
signed_transaction: signed_transaction(
signer,
actions.clone(),
nonce,
block_hash,
receiver_id,
),
})
.await
Expand Down
2 changes: 1 addition & 1 deletion workspaces/src/types/gas_meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::Worker;
/// The auto-traits [`Send`], [`Sync`], [`UnwindSafe`] and [`RefUnwindSafe`] are added explicitly because they
/// do not fall under the rules the compiler uses to automatically add them.
/// See here: <https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits>
pub type GasHook = Arc<dyn Fn(Gas) -> Result<()> + Send + Sync + UnwindSafe + RefUnwindSafe>;
pub(crate) type GasHook = Arc<dyn Fn(Gas) -> Result<()> + Send + Sync + UnwindSafe + RefUnwindSafe>;

/// Allows you to meter the amount of gas consumed by transaction(s).
/// Note: This only works with transactions that resolve to [`crate::result::ExecutionFinalResult`]
Expand Down
34 changes: 25 additions & 9 deletions workspaces/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::str::FromStr;

use borsh::{BorshDeserialize, BorshSerialize};
pub use near_account_id::AccountId;
use near_primitives::transaction::{Action, SignedTransaction, Transaction};

use serde::{Deserialize, Serialize};
use sha2::Digest;
Expand All @@ -27,8 +28,7 @@ use crate::result::Result;

pub use self::account::{AccountDetails, AccountDetailsPatch};
pub use self::chunk::{Chunk, ChunkHeader};

pub use self::gas_meter::{GasHook, GasMeter};
pub use self::gas_meter::GasMeter;

/// Nonce is a unit used to determine the order of transactions in the pool.
pub type Nonce = u64;
Expand Down Expand Up @@ -292,13 +292,6 @@ impl InMemorySigner {
SecretKey(signer.secret_key),
))
}

pub(crate) fn inner(&self) -> near_crypto::InMemorySigner {
near_crypto::InMemorySigner::from_secret_key(
self.account_id.clone(),
self.secret_key.0.clone(),
)
}
}

// type taken from near_primitives::hash::CryptoHash.
Expand Down Expand Up @@ -528,3 +521,26 @@ impl From<Finality> for near_primitives::types::BlockReference {
value.into()
}
}

/// A helper function to create a signed transaction with the given actions.
pub(crate) fn signed_transaction(
signer: &InMemorySigner,
actions: Vec<Action>,
nonce: near_primitives::types::Nonce,
block_hash: near_primitives::hash::CryptoHash,
receiver_id: &AccountId,
) -> SignedTransaction {
let tx = Transaction {
signer_id: signer.account_id.clone(),
public_key: signer.secret_key.public_key().0,
nonce,
receiver_id: receiver_id.clone(),
block_hash: near_primitives::hash::CryptoHash(block_hash.0),
actions,
};

SignedTransaction::new(
signer.secret_key.0.sign(tx.get_hash_and_size().0.as_ref()),
tx,
)
Copy link
Member

Choose a reason for hiding this comment

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

you shouldn't need to call into SignedTransaction::new. A tx.sign(&signer) is simpler

}
7 changes: 7 additions & 0 deletions workspaces/src/worker/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,15 @@ impl<T: ?Sized> Worker<T>
where
T: NetworkClient,
{
/// Provides a list of changes in block associated with the given block reference.
pub async fn changes_in_block(
&self,
block_reference: BlockReference,
) -> Result<RpcStateChangesInBlockByTypeResponse> {
self.client().changes_in_block(block_reference).await
}

/// Provides a list of changes in block associated with the given block reference and state changes request.
pub async fn changes(
&self,
block_reference: BlockReference,
Expand All @@ -211,28 +213,33 @@ where
.await
}

/// Provides a genesis config associated with the network being used.
pub async fn genesis_config(&self) -> Result<GenesisConfig> {
self.client().genesis_config().await
}

/// Provides a protocol config associated with the given block reference.
pub async fn protocol_config(
&self,
block_reference: BlockReference,
) -> Result<ProtocolConfigView> {
self.client().protocol_config(block_reference).await
}

/// Provides a receipt associated with the given receipt reference.
pub async fn receipt(&self, receipt_reference: ReceiptReference) -> Result<ReceiptView> {
self.client().receipt(receipt_reference).await
}

/// Returns the transaction status for a given transaction hash or signed transaction.
pub async fn tx_status(
&self,
transaction_info: TransactionInfo,
) -> Result<FinalExecutionOutcomeWithReceiptView> {
self.client().tx_status(transaction_info).await
}

/// Provides a list of validators ordered with respect to their stake.
pub async fn validators_ordered(
&self,
block_id: MaybeBlockId,
Expand Down
2 changes: 1 addition & 1 deletion workspaces/src/worker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::sync::Arc;

use crate::network::builder::NetworkBuilder;
use crate::network::{Betanet, Custom, Mainnet, Sandbox, Testnet};
use crate::types::GasHook;
use crate::types::gas_meter::GasHook;
use crate::{Network, Result};

/// The `Worker` type allows us to interact with any NEAR related networks, such
Expand Down
Loading