starknet-rs icon indicating copy to clipboard operation
starknet-rs copied to clipboard

Make input for Accout::sign_execution publicly constructable

Open Angel-Petrov opened this issue 1 year ago • 3 comments

Currently the only way to sign a execution is through Account::sign_execution which takes in a RawExecution, however this type has no way to be made by users of the library making it so the only way to use this API is for internal purposes and through the unsafe mem::transmute.

The simplest solution to this would be to make a RawExecution::new associated function or to make the method Execution::to_raw which would return a Option<RawExecution> (though this forces uses to have to add a unwrap if they know the Execution has a nonce and a max_fee)

Angel-Petrov avatar Jan 26 '24 08:01 Angel-Petrov

@xJonathanLEI do you have an opinion on this?

tdelabro avatar Feb 05 '24 14:02 tdelabro

Hi sorry for the delay. I think it makes sense to allow construction of this type. I just want a bit more context on cases where this is useful. Care to elaborate?

xJonathanLEI avatar Feb 24 '24 07:02 xJonathanLEI

@xJonathanLEI Thanks for the reply! The use case that initially caused this PR to be made was for when a load testing framework required that its own http request functions be used to get benchmarking metrics, meaning we have to prepare the requests ourselves as well. This code is currently what we have to do, which is not ideal since any version upgrades might break the code and it is generally not very idiomatic.

It is also very confusing to users of the library to have public methods they could use that have parameters that they cannot construct, ideally it would be a private method if that is the case or the type would be made publicly constructable.

Angel-Petrov avatar Feb 26 '24 09:02 Angel-Petrov