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

Update epsilon paths, use ChainId #196

Merged
merged 12 commits into from
Feb 12, 2025
Merged

Conversation

volovyks
Copy link
Contributor

@volovyks volovyks commented Feb 11, 2025

Our last chance to update these strings.
@Pessina @esaminu , we are changing root derivation path, you may want to update it in tooling,

@volovyks volovyks changed the title update epsilon paths Update epsilon paths, use ChainId Feb 11, 2025
@@ -42,7 +42,7 @@ pub struct ContractSignatureRequest {

impl SignatureRequest {
pub fn new(payload_hash: Scalar, predecessor_id: &AccountId, path: &str) -> Self {
let epsilon = derive_epsilon(predecessor_id, path);
let epsilon = derive_epsilon_near(predecessor_id, path);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After this renaming, I've realized that we are using this struct for all chains, but deriving the key using the NEAR path. new() is only used in Near flows, so it's ok, but we need to fix this. @ChaoticTempest, this is related to your latest PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

the issue with this one is that making it consistent with my refactoring is a breaking serialization change, which is not ideal if eth relies on it and I'm not sure if it disrupts your guys work. We can change the near one pretty easily since we haven't deployed mainnet yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I see it is used only in NEAR flows. And if we want to change something - now is the time. We are not released anywhere.

Copy link
Contributor

@ppca ppca left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +303 to +308
let big_r = "029b1b94bf4511b1a25986ba858cfa0fbdd5e4077c02e1d1102a194389b1f72df7";
let s = "25f3494bb7e7b3349a4b4d939d3e5ae1787a0863e4f698fb8ed2d3e11c195035";
let mpc_key = "045b4fa179e005361fd858f8a6f896d7afc23a53d3f95d6566a88cde954e7b2f1cb77c554705c35d4ffced67aeafbcda46d9d89d6f200c3a3d109f92872863b3dc";
let account_id = "dev-20250212213501-93636560094065.test.near";
let payload_hash: [u8; 32] =
hex::decode("49f32740939bfdcbd8d1786075df7aca384381ec203975c3a6c1fd80acddcd4c")
hex::decode("835b9f469b36126284df2e06ecab9482cf495413ab9275faaafb2d40d79cf7bb")
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we changing these values? These should not change at all unless our crypto got changed somehow

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait, you added the near chain id in this so this does change the signature

@volovyks volovyks merged commit 44b2be7 into develop Feb 12, 2025
2 of 3 checks passed
@volovyks volovyks deleted the serhii/update-epsilon-paths branch February 12, 2025 23:15
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.

3 participants