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

feat: take chain_id mod 64 #1048

Merged
merged 1 commit into from
Mar 27, 2024
Merged

feat: take chain_id mod 64 #1048

merged 1 commit into from
Mar 27, 2024

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Mar 27, 2024

Time spent on this PR: 0.1d

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Resolves #

What is the new behavior?

  • Take chain_id mod 64, as EVM tools don't accept chain_id with values > 64 bits. For example, "SN_SEPOLIA" takes 10 bytes, thus doesn't fit in 64 bits, and cannot be used as a valid CHAIN_ID in wallets. Both the wallets and Kakarot will take CHAIN_ID modulo 64

This change is Reviewable

@enitrat enitrat force-pushed the feat/chain-id-mod-64 branch from 2287294 to 955e418 Compare March 27, 2024 17:02
"version": 1,
"account_contract_address": 0xABDE1,
"max_fee": int(1e17),
# Signature len will be set later based on the signature.
Copy link
Member

Choose a reason for hiding this comment

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

can remove this comment

@@ -6,6 +6,7 @@
BLOCK_GAS_LIMIT = BLOCK_GAS_LIMIT

CHAIN_ID = int.from_bytes(b"KKRT", "big") # KKRT (0x4b4b5254) in ASCII
BIG_CHAIN_ID = int.from_bytes(b"SN_SEPOLIA", "big")
Copy link
Member

Choose a reason for hiding this comment

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

👍 always a good habit to actually use in the test the observed failing case

@ClementWalter ClementWalter merged commit 127b7ac into main Mar 27, 2024
7 checks passed
@ClementWalter ClementWalter deleted the feat/chain-id-mod-64 branch March 27, 2024 17:58
@enitrat enitrat mentioned this pull request Mar 27, 2024
7 tasks
enitrat added a commit that referenced this pull request Mar 27, 2024
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR:

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- remove dead code introduced by #1048 
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/1050)
<!-- Reviewable:end -->
@ClementWalter ClementWalter mentioned this pull request Apr 12, 2024
matthieuauger pushed a commit to matthieuauger/kakarot that referenced this pull request Nov 9, 2024
* add docs

* fix block number padding + remove "magic" numbers

* add test

* fix comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants