Skip to content
This repository was archived by the owner on Aug 21, 2024. It is now read-only.
This repository was archived by the owner on Aug 21, 2024. It is now read-only.

bad api design for DeclareTransaction creation #1725

@tdelabro

Description

@tdelabro
#[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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions