This repository was archived by the owner on Aug 21, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 106
This repository was archived by the owner on Aug 21, 2024. It is now read-only.
bad api design for DeclareTransaction creation #1725
Copy link
Copy link
Open
Description
#[derive(Debug, Clone, Encode, Decode, Eq, PartialEq)]
#[cfg_attr(feature = "scale-info", derive(scale_info::TypeInfo))]
pub struct DeclareTransaction {
pub tx: starknet_api::transaction::DeclareTransaction,
pub tx_hash: TransactionHash,
// Indicates the presence of the only_query bit in the version.
only_query: bool,
pub class_info: ClassInfo,
}
impl DeclareTransaction {
fn create(
declare_tx: starknet_api::transaction::DeclareTransaction,
tx_hash: TransactionHash,
class_info: ClassInfo,
only_query: bool,
) -> TransactionExecutionResult<Self> {
let declare_version = declare_tx.version();
verify_contract_class_version(&class_info.contract_class(), declare_version)?;
Ok(Self { tx: declare_tx, tx_hash, class_info, only_query })
}
pub fn new(
declare_tx: starknet_api::transaction::DeclareTransaction,
tx_hash: TransactionHash,
class_info: ClassInfo,
) -> TransactionExecutionResult<Self> {
Self::create(declare_tx, tx_hash, class_info, false)
}
pub fn new_for_query(
declare_tx: starknet_api::transaction::DeclareTransaction,
tx_hash: TransactionHash,
class_info: ClassInfo,
) -> TransactionExecutionResult<Self> {
Self::create(declare_tx, tx_hash, class_info, true)
}As you can see, both the only_query field and the create method are private. The lib users cannot use DeclareTransaction {} or DeclareTransaction::create in order to instantiate a DeclareTransaction.
The have to use either new or new_for_query resulting in a code looking like this:
let tx = if only_query {
DeclareTransaction::new_for_query(declare_tx, tx_hash, class_info)
} else {
DeclareTransaction::new(declare_tx, tx_hash, class_info)
};Which is super redundant. At least create should be publicbut probably the fieldonly_query` too. I see no reason to keep it out of reach of libs users, when all the others fields are publics.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels