Feat: add RIP-7212 to the list of precompiles Cairo1Helpers module

kkrt-labs / kakarot-ssj

Kakarot zkEVM - rewrite in the latest version of Cairo

https://www.kakarot.org

MIT License

114 stars 53 forks source link

Feat: add RIP-7212 to the list of precompiles Cairo1Helpers module #779

Closed Quentash closed 2 months ago

Quentash commented 2 months ago

Add the precompiled contracts p256verify to the list of precompiled contracts at address 0x0b following this implementation : https://github.com/ethereum/go-ethereum/pull/27540 .

I have chosen the 0x0b instead of 0x100 because of the amount of lines that the second one would have added to the precompiles.cairo and I am currently unaware of a precompiled contract at address 0x0b.

Pull Request type

Please check the type of change your PR introduces:

[ ] Bugfix
[X] 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: #755

What is the new behavior?

p256verify is now added to the list of precompiled contract at address 0x0b

Does this introduce a breaking change?

[ ] Yes
[X] No

This change is

enitrat commented 2 months ago

I have chosen the 0x0b instead of 0x100 because of the amount of lines that the second one would have added to the precompiles.cairo and I am currently unaware of a precompiled contract at address 0x0b.

You can just add a specific if / else before the match table handling this specific case for 0x100. This should not be an issue when deciding a precompile address. Given that it seems a consensus was found on using 0x100 - 0x1ff for rollup precompiles, let's go with 0x100

Quentash commented 2 months ago

Chose 0x100 as precompiled address
Added length check ( nice catch )
Added tests concerning length check
Removed setup_contracts for unit tests

Quentash commented 2 months ago

Corrected precompiled contracts address range

enitrat commented 2 months ago

some tests are failing, can you verify is 256 or 0x100 is an address used in tests?

Quentash commented 2 months ago

Alright, my bad, I missed those since I struggle running every tests and didn't notice it on github. But indeed, some tests were deploying on the 0x100 address which created conflicts. I changed them to deploy their test contract on 0x101.

enitrat commented 2 months ago

Alright, my bad, I missed those since I struggle running every tests and didn't notice it on github. But indeed, some tests were deploying on the 0x100 address which created conflicts. I changed them to deploy their test contract on 0x101.

0x101 is still too "close" we should ideally have a random value like 0xabfa740ccd

Quentash commented 2 months ago

You're right ! 0xabfa740ccd it is then !