Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

RPC schema felt validation #401

Closed
2 tasks done
penovicp opened this issue Feb 17, 2023 · 5 comments · Fixed by #467
Closed
2 tasks done

RPC schema felt validation #401

penovicp opened this issue Feb 17, 2023 · 5 comments · Fixed by #467

Comments

@penovicp
Copy link

Describe the bug (observed vs expected behavior)

For the RPC schema validation of the latest devnet version the regular expression used for validating felt fields seems to be too strict. It necessitates that the corresponding values conform to the 0x0{digits} format, while for the earlier versions and for testnet the 0x{digits} format is accepted.

For the payload:

{
    "jsonrpc": "2.0",
    "method": "starknet_getClass",
    "params": {
        "block_id": "latest",
        "class_hash": "0x25ec026985a3bf9d0cc1fe17326b245dfdc3ff89b8fde106542a3ea56c5a918"
    },
    "id": 0
}
  • testnet replies with a successful response
  • devnet 0.4.4 replies with the expected UNDECLARED_CLASS error
  • devnet 0.4.5 replies with the unexpected "Got invalid value for parameter: \"'0x25ec026985a3bf9d0cc1fe17326b245dfdc3ff89b8fde106542a3ea56c5a918' does not match '^0x0[a-fA-F0-9]{1,63}$'\"" error message

Not reproducible on alpha-goerli

  • This issue is only present on Devnet and cannot be reproduced on alpha-goerli (check the box if true).

Devnet version

  • I am using Devnet version: 0.4.5
  • This happens with a dockerized Devnet (check the box if true).
  • This does not appear on the following Devnet version: 0.4.4
@fracek
Copy link

fracek commented Feb 27, 2023

I'm also hitting this bug by requesting the transaction receipt of the seed accounts deploy transactions.

@ClementWalter
Copy link

@fracek just proposed another updated regex with a simple ?, see corresponding PR

@THenry14
Copy link
Contributor

I believe the devnet should stick with the "official" regexp from starknet's rpc-spec repo. This change will probably soon be released, and my understanding is that regexp used there was deliberate (as the leading 0 does not change the value - 0x0123 == 0x123).

I'd rather the devnet complies to the rpc spec (especially that it is supposed to validate payloads/responses against it) than introduce some other regexp that technically works, but is only specific to it. This can lead to all kinds of problems in the future.

It probably means some additional work is necessary on the devnet part to make it work (I guess changing/removing rpc_felts or checking felt values and converting them by removing leading 0 if needed). It shouldn't be too complicated though.

@ClementWalter
Copy link

I believe the devnet should stick with the "official" regexp from starknet's rpc-spec repo. This change will probably soon be released, and my understanding is that regexp used there was deliberate (as the leading 0 does not change the value - 0x0123 == 0x123).

I'd rather the devnet complies to the rpc spec (especially that it is supposed to validate payloads/responses against it) than introduce some other regexp that technically works, but is only specific to it. This can lead to all kinds of problems in the future.

It probably means some additional work is necessary on the devnet part to make it work (I guess changing/removing rpc_felts or checking felt values and converting them by removing leading 0 if needed). It shouldn't be too complicated though.

I don't really have an opinion, I don't understand why people care so much about a leading 0 which doesn't really make any sense to me (is it to have a 32 long bytes string? though felt is not a bytes32), but what I'm sure about is that it makes the current devnet not working with starknet-rs

@FabijanC
Copy link
Collaborator

FabijanC commented Mar 1, 2023

Does it help if starknet-devnet is started with --disable-rpc-request-validation or --disable-rpc-response-validation?

EDIT: Yes, running Devnet with:

starknet-devnet --disable-rpc-request-validation

resolves the issue and can be used at least as a temporary solution.

ClementWalter added a commit to kkrt-labs/kakarot that referenced this issue Mar 9, 2023
Time spent on this PR: 0.5

## 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)
- [x] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

The JSON RPC spec does not allow for address missing the leading 0 in
hex string

## What is the new behavior?

Devnet is started with the `--disable-rpc-request-validation` flag to
skip this validation?

## Other information

See corresponding discussion in
0xSpaceShard/starknet-devnet#401
@ivpavici ivpavici moved this to 🆕 New in starknet-devnet Apr 17, 2023
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in starknet-devnet May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done
5 participants