Skip to content

Conversation

@aregng
Copy link

@aregng aregng commented Dec 19, 2025

No description provided.

@aregng aregng requested a review from sjoshisupra December 19, 2025 06:30
@aregng aregng marked this pull request as ready for review December 29, 2025 12:06
- If needed it should be enabled and generated manually
Comment on lines +47 to +49
pub fn is_supra_reserved(address: &Address) -> bool {
SUPRA_RESERVED_ADDRESSES.contains(address)
}

Choose a reason for hiding this comment

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

I am guessing this would be changed based on our discussion that prefix comparision would be faster.

Comment on lines +58 to +60
VM_SIGNER = SUPRA_RESERVED_ADDRESSES[0],
"Supra Reserved address Precompile address to retrieve transaction hash",
TX_HASH_ADDRESS = SUPRA_RESERVED_ADDRESSES[1],

Choose a reason for hiding this comment

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

Since we are not going to generate SUPRA_RESERVED_ADDRESSES for now, may be this can be directly be defned as corresponding const value?

Comment on lines +907 to +911
interface SupraContractsBindings {
function blockPrologue() external;
function getAllActiveTaskIds() external view returns (uint256[] memory);
function getCycleInfo() external view returns (uint64, uint64, uint64, CommonUtils.CycleState);
function getTaskDetails(uint64 _taskIndex) external view returns (CommonUtils.TaskDetails memory);

Choose a reason for hiding this comment

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

just wondering, are you able to call registry methods from here or not? Does alloy provide necessary API to be able to do so?

Copy link
Author

Choose a reason for hiding this comment

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

That is my hope so, just need to investigate the apis more thorough to have the definite answer.

Comment on lines +1501 to +1502
const SIGNATURE: &'static str = "getAllActiveTaskIds()";
const SELECTOR: [u8; 4] = [197u8, 220u8, 246u8, 172u8];

Choose a reason for hiding this comment

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

does this mean you are indeed able to make a call from Rust layer to solidity methods?

Comment on lines +197 to +203
pub struct AutomatedTransactionDetails {
/// Transaction details
pub txn: AutomatedTransaction,
/// Priority of the automated transaction to be scheduled.
/// The low value indicates higher priority.
pub priority: u64,
}

Choose a reason for hiding this comment

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

In Move, since the contract was already deployed, we had to define another struct to be able to add new fields. But in Solidity we don't hve to do that. It means that Solidity can perhaps have a single struct encompassing all required fields, including priority (cc: @udityadav-supraoracles @aregng ) and therefore the Rust interface also may have a single big struct.

Copy link
Author

Choose a reason for hiding this comment

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

Dr @sjoshisupra, this is already AutomatedTransaction which is created based on the automation TaskDetails. When TaskDetails is updated to have priority as separate property, the rust counterpart representation will be updated accordingly/automatically.
Here priority is separate for AutomatedTransaction because it will be only used to schedule the transaction based on its priority, during the execution it will not play any role, so it is not included in AutomatedTransaction

Comment on lines +68 to +74
#[inline]
fn gas_price(&self) -> Option<u128> {
None
}

#[inline]
fn max_fee_per_gas(&self) -> u128 {

Choose a reason for hiding this comment

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

what is the difference between gas_price and max_fee_per_gas?

Copy link
Author

@aregng aregng Jan 5, 2026

Choose a reason for hiding this comment

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

The meaning of them is the same but based on the transaction eip type only one of them is present TxEip1559&TxEip7702 have max_fee_per_gas, Legacy&TxEip2930 have gas-price

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants