Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Adding mask_bits function #525

Closed
wants to merge 2 commits into from
Closed

Adding mask_bits function #525

wants to merge 2 commits into from

Conversation

rodoufu
Copy link

@rodoufu rodoufu commented Dec 13, 2023

No description provided.

@rodoufu rodoufu marked this pull request as ready for review December 13, 2023 11:12
@xJonathanLEI
Copy link
Owner

Hi @rodoufu. Thanks for the PR. Could you please explain what the use case of this function is? Would be helpful to get more context on this.

@rodoufu
Copy link
Author

rodoufu commented Dec 15, 2023

Hi @rodoufu. Thanks for the PR. Could you please explain what the use case of this function is? Would be helpful to get more context on this.

Hi @xJonathanLEI , one needs it when calculating keccack256_hash.

fn keccack256_hash(msg: String) -> DriverResult<FieldElement> {
    let mut msg_hash = keccak256(msg.as_bytes());
    mask_bits(250, 8, &mut msg_hash[..]);
    FieldElement::from_byte_slice_be(&msg_hash).map_err(|err| {
        DriverError::generic(format!(
            "problem converting keccack256 result to feld {err}"
        ))
    })
}

One would need that to generate the account address for Paradex, see this example in Go https://github.com/tradeparadex/code-samples/blob/main/examples/go/utils.go#L40 it uses types.GetSelectorFromName which uses a similar function https://github.com/NethermindEth/starknet.go/blob/main/utils/keccak.go#L119 defined here https://github.com/NethermindEth/starknet.go/blob/main/utils/keccak.go#L177

@rodoufu
Copy link
Author

rodoufu commented Dec 15, 2023

@xJonathanLEI I see some tests failed with "Too many requests"

---- can_execute_tst_mint_with_jsonrpc stdout ----
thread 'can_execute_tst_mint_with_jsonrpc' panicked at starknet-accounts\tests\single_owner_account.rs:310:10:
called `Result::unwrap()` on an `Err` value: Provider(Other(JsonRpcError(JsonRpcError { code: -32603, message: "Internal Error", data: Some(String("<!doctype html><meta charset=\"utf-8\"><meta name=viewport content=\"width=device-width, initial-scale=1\"><title>429</title>429 Too Many Requests")) })))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- can_declare_cairo1_contract_with_jsonrpc stdout ----
thread 'can_declare_cairo1_contract_with_jsonrpc' panicked at starknet-accounts\tests\single_owner_account.rs:366:10:
called `Result::unwrap()` on an `Err` value: Provider(Other(JsonRpcError(JsonRpcError { code: -32603, message: "Internal Error", data: Some(String("<!doctype html><meta charset=\"utf-8\"><meta name=viewport content=\"width=device-width, initial-scale=1\"><title>429</title>429 Too Many Requests")) })))

---- can_declare_cairo0_contract_with_jsonrpc stdout ----
thread 'can_declare_cairo0_contract_with_jsonrpc' panicked at starknet-accounts\tests\single_owner_account.rs:419:10:
called `Result::unwrap()` on an `Err` value: Provider(Other(JsonRpcError(JsonRpcError { code: -32603, message: "Internal Error", data: Some(String("<!doctype html><meta charset=\"utf-8\"><meta name=viewport content=\"width=device-width, initial-scale=1\"><title>429</title>429 Too Many Requests")) })))

It does not seem to be related to the MR, but I don't think I can run again the failed tests.

@xJonathanLEI
Copy link
Owner

@rodoufu If this is just for Keccak, there's already starknet_core::utils::starknet_keccak().

@xJonathanLEI
Copy link
Owner

Since there's no response, judging by the conversation I'm assuming that it's just for performing Starknet Keccak, which is already supported in the library, which the masking performed internally inside the relevant function. I'm closing this optimistically. Feel free to reopen if you have other use cases not already covered. Thanks!

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.

2 participants