Skip to content

Refactor: TransactionConnection::run_contract_call has too many arguments #5943

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
kantai opened this issue Mar 21, 2025 · 1 comment
Open
Assignees

Comments

@kantai
Copy link
Contributor

kantai commented Mar 21, 2025

The function

fn run_contract_call<F>(
        &mut self,
        sender: &PrincipalData,
        sponsor: Option<&PrincipalData>,
        contract: &QualifiedContractIdentifier,
        public_function: &str,
        args: &[Value],
        abort_call_back: F,
        max_execution_time: Option<std::time::Duration>,
    ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), Error>

Trips clippy due to too many arguments, and indeed, it's too many arguments, and its the kind of thing that also triggers large multi-file changesets whenever the signature gets updated. This is annoying.

My suggestion for this (and similarly BlockBuilder::try_mine_tx_with_len) is to make an argument struct:

pub struct RunContractCallArg<'a, F> {
    sender: &'a PrincipalData,
    sponsor: Option<&'a PrincipalData>,
    ..
}

And then we can define constructors and chaining modifiers to simplify our lives:

/// Setup arguments for running a contract call with no abort callback
///  and default options
fn new(sender, sponsor, contract, fn_name, args) -> Self {
   Self { sender, sponsor, contract, fn_name, args, abort_call_back: |_| false, ... }
}
fn with_time_limt(mut self, time_limit: Duration) -> Self {
   self.time_limit = Some(time_limit);
   self
}
@rdeioris
Copy link
Contributor

I did a first round with run_contract_call. I think that if we want to pursue this logic we first need a macro allowing automatic generation of the with_ functions. In addition to this, by looking at the resulting code, it looks like the with_ calls are not inlined. We should probably enforce this (even if the two currently involved functions are not called so often in the run loop)

@rdeioris rdeioris self-assigned this Mar 24, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: Status: 🆕 New
Development

No branches or pull requests

2 participants