Skip to content
This repository has been archived by the owner on Jan 8, 2025. It is now read-only.

dev: replace starknet_crypto::FieldElement with starknet-types-core #1233

Closed
tcoratger opened this issue Jun 24, 2024 · 8 comments · Fixed by #1296
Closed

dev: replace starknet_crypto::FieldElement with starknet-types-core #1233

tcoratger opened this issue Jun 24, 2024 · 8 comments · Fixed by #1296
Assignees
Labels
enhancement Enhancement of the code, not introducing new features. time: halfday

Comments

@tcoratger
Copy link
Collaborator

Describe the enhancement request

At the moment, we are using an old version of starknet-rs crypto crate. Following the merge of xJonathanLEI/starknet-rs#562, we should migrate to the new version and replace our current FieldElement around the codebase with the new Felt type.

This update involves replacing all instances of the FieldElement type in our codebase with the new Felt type introduced in the update.

Benefits

  • Improved performance and security with the latest version of the crypto crate.
  • Staying up-to-date with the latest developments and fixes in the starknet-rs library.
@tcoratger tcoratger added enhancement Enhancement of the code, not introducing new features. time: halfday labels Jun 24, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Backlog in Kakarot on Starknet Jun 24, 2024
@greged93
Copy link
Contributor

This will probably lead us to need to bump Katana as well no? Are we blocked there or would that be possible?

@tcoratger tcoratger added the blocked Blocked right now for external reasons label Jun 25, 2024
@tcoratger
Copy link
Collaborator Author

This will probably lead us to need to bump Katana as well no? Are we blocked there or would that be possible?

Yes that's true, just put a blocked tag to remind us that we need to update Katana as well and it's blocked at the moment due to fork issue.

@tcoratger
Copy link
Collaborator Author

blocked by #1245

@greged93
Copy link
Contributor

I think you can start this whenever you want @eugypalu

@tcoratger
Copy link
Collaborator Author

I think you can start this whenever you want @eugypalu

I think this is already work in progress as far as I understood with our week discussions

@greged93 greged93 moved this from 🆕 Backlog to 🏗 In progress in Kakarot on Starknet Jul 15, 2024
@eugypalu
Copy link
Contributor

Current status of the issue:

  • The issue is blocked because it is not possible to run the tests without upgrading this: https://github.com/kkrt-labs/blockifier
  • It is possible to run the cargo build without errors, and all FieldElement have been migrated to Felt.

@tcoratger
Copy link
Collaborator Author

Current status of the issue:

  • The issue is blocked because it is not possible to run the tests without upgrading this: https://github.com/kkrt-labs/blockifier
  • It is possible to run the cargo build without errors, and all FieldElement have been migrated to Felt.

Can you push a draft pr so that we can check what happens with the update of blockifier?

@eugypalu
Copy link
Contributor

Current status of the issue:

  • The issue is blocked because it is not possible to run the tests without upgrading this: https://github.com/kkrt-labs/blockifier
  • It is possible to run the cargo build without errors, and all FieldElement have been migrated to Felt.

Can you push a draft pr so that we can check what happens with the update of blockifier?

Sure, opened: #1296

@greged93 greged93 moved this from 🏗 In progress to 👀 In review in Kakarot on Starknet Jul 17, 2024
@greged93 greged93 removed the blocked Blocked right now for external reasons label Jul 17, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Kakarot on Starknet Jul 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement of the code, not introducing new features. time: halfday
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants