search

xJonathanLEI / starknet-rs

Complete Starknet library in Rust™
https://starknet.rs
Apache License 2.0
286 stars 101 forks source link

Add support for StarkNet v0.8.1 #99

Closed xJonathanLEI closed 2 years ago

xJonathanLEI commented 2 years ago

StarkNet 0.8.1 is about to release. Creating this issue to track integration progress.

See comments below. It looks like we only need to resolve API changes for this version.

milancermak commented 2 years ago

Had a quick look at the diff, here's what might affect us:

xJonathanLEI commented 2 years ago

Thanks for checking out! I took a look and it seems they added the concept of QUERY_VERSION. I haven't seen them talking about this, but I think this has to do with solving the issue of having to sign a transaction to call estimate_fee.

My guess is, the idea would be that account contracts will skip signature verification if version is set to QUERY_VERSION, such that transactions no longer need to be signed twice like it is right now. They're basically adding server-side support for this. I expect to see account contract implementers to add support soon.

If that's the case, I guess no action is needed from our side until it lands in account contract implementations. It's not illegal to not use QUERY_VERSION for queries anyways (and we won't benefit from doing so until account contracts integrate it).

xJonathanLEI commented 2 years ago

The get_state_update endpoint seems to be down... No way to update test data for those.

xJonathanLEI commented 2 years ago

It's inconsistency all over again.

class_hash is added to DeployTransaction, BUT only on get_transaction responses. Transactions in get_block have no such field...

I expect get_block to eventually catch up and also serve class_hash. So instead of splitting the type into two, I simply made the new field optional, as a temporary resolution.

milancermak commented 2 years ago

It looks like get_block does now have class_hash for deploy transaction (see e.g. the sixth one in here), but it should still be marked as optional because old blocks like this one, first TX don't have it 😞

xJonathanLEI commented 2 years ago

Wow that's a whole new level of inconsistency... Maybe we should report to StarkWare as bug? Need to test a bit more first though.