Support SNIP-9 #1530

baitcode · 2024-12-02T12:36:04Z

Introduced changes

[x] Added outside execution model, defined hashing
[x] Extended base account interface
[x] Added all utilities to deal with SNIP-9 nonce, interface support queries and generation OutsideExecution call
[x] Cleaned up as much as I could to keep changes minimal
[x] Added a bit of docs
[x] Added one test for positive scenario
[x] Pinned hash calculation for OutsideExecution
[x] Added one test for wrong caller scenario

Small things I had to rework on the way

[x] Moved broadcast transaction generation method closer to usage to get rid of Cyclic dependency
[x] Bugfix type data generation
[x] Allowed for incomplete definition of ParameterDict as contains fields is often missing and linter is.

While implementing the feature I realised better what are the problems of current library implementation. I'd love to discuss those and if it proves that my vision is similar to maintainers I'd love to help fixing those.

baitcode · 2024-12-05T02:15:53Z

@franciszekjob Please, take a look.

There are failing checks. I am unsure if I can figure those ones out quickly. Some dependecy does not install in docker. Let me know If you want me investigate.

From the top of stackoverflow:

https://stackoverflow.com/questions/67297760/rust-compilation-error-failed-to-run-custom-build-command-for-freetype-sys-v0

- Added new Transaction models: InvokeOutsideV1, InvokeOutsideV2 - Added method for SNIP9 nonce verification - Added Account methods for population and broadcasting(execution) of InvokeOutside transactions Small fixes: - Allowed for incomplete definition of ParameterDict as `contains` fiels is often missing and linter is complaining. WIP: miss docs and tests

- Extended base account interface - Added all utilities to deal with SNIP-9 nonce and generating OutsideExecution call - Bugfix type data generation - Cleaned up as much as I could to keep changes minimal - Added docs - Added test for positive scenario - Added one test for wrong caller scenario

* test fixes

…unt balance

…est account balance" This reverts commit 5851f12.

franciszekjob · 2024-12-10T17:11:05Z

Hi @baitcode 👋
I see you already did a lots of work! I fixed failing build of Docker container. Let me know if you need any help and ping request review once you're ready 😄

baitcode · 2024-12-10T18:42:25Z

@franciszekjob Hey hey!

I fixed all the required steps. With sad heart I got to admit, that some tests in the project are really bad.
Those to are not reproducible, as they depend on the account that is global and is shared among all other tests (and not immutable):

e2e/client/client_test.py:test_get_storage_at 
e2e/client/client_test.py:test_call_contract

It's better to remove those. Or create new account for every run.

And yes, I need help with test_on_network. I can't reproduce those, neither can I understand what is wrong. You can start the review.

franciszekjob · 2024-12-10T22:52:19Z

@baitcode

I fixed all the required steps. With sad heart I got to admit, that some tests in the project are really bad.
Those to are not reproducible, as they depend on the account that is global and is shared among all other tests (and not immutable)

I see, we can think about refactoring these tests in separate PR. I understand they may have space for improvement.

And yes, I need help with test_on_network. I can't reproduce those, neither can I understand what is wrong. You can start the review.

Tests on networks are failing because you haven't added secrets to your forked repository
You need to add:

SEPOLIA_RPC_URL
SEPOLIA_ACCOUNT_ADDRESS
SEPOLIA_ACCOUNT_PRIVATE_KEY

If you don't have testnet account, you can create one using sncast tool.

Let me know if it works!

baitcode · 2024-12-10T23:45:08Z

@franciszekjob Added repository secrets on my repo. Still not working.

starknet_py/net/account/account.py

starknet_py/net/client_models.py

starknet_py/tests/e2e/account/outside_execution_test.py

starknet_py/tests/e2e/docs/guide/test_account_sign_outside_transaction.py

starknet_py/tests/e2e/account/outside_execution_test.py

franciszekjob · 2024-12-16T00:56:22Z

@baitcode just a couple more comments regarding tests, lmk if you need anything 👍

baitcode · 2024-12-16T12:20:25Z

@franciszekjob I need help. For some reason argent account doesn't execute v3 transactions. Reverts with error:

'argent/invalid-tx-version'

For now I've reverted execute_v1 method, as it seems that v3 are actually not supported by argent according to that report.

Just to provide a bit more context. Non argent account that is being set up in tests does not provide external execution support.

franciszekjob · 2024-12-16T13:36:23Z

For now I've reverted execute_v1 method, as it seems that v3 are actually not supported by argent according to that report.

Fact, right.

Just to provide a bit more context. Non argent account that is being set up in tests does not provide external execution support.

Ok, I see. But that's not a problem, as we're using it to execute, not create outside transaction.

starknet_py/tests/e2e/docs/guide/test_account_sign_outside_transaction.py

starknet_py/net/account/account.py

starknet_py/net/account/base_account.py

docs/guide/account_and_client.rst

franciszekjob · 2024-12-16T13:43:12Z

@baitcode please check out my comments 👍

Co-authored-by: Franciszek Job <[email protected]>

…nsaction.py Co-authored-by: Franciszek Job <[email protected]>

franciszekjob

Could you also resolve error while building docs 🙏 ?

baitcode · 2024-12-16T15:01:48Z

Sure. I'm on it atm.

baitcode · 2024-12-16T16:03:42Z

@franciszekjob Is there anything left?

franciszekjob

@baitcode
Just these values in tests + could you please mention the change in migration_guide.rst 🙏 ?

starknet_py/tests/e2e/account/outside_execution_test.py

Co-authored-by: Franciszek Job <[email protected]>

franciszekjob

Please apply suggested change and we're good to go 🚀

docs/migration_guide.rst

Co-authored-by: Franciszek Job <[email protected]>

franciszekjob · 2024-12-16T19:00:29Z

Thanks for contribution @baitcode 🤝 tomorrow I will reward you on Only Dust for completing this issue

baitcode · 2024-12-16T19:37:49Z

WOOOOHOOOO. Thanks a lot! @franciszekjob

baitcode · 2024-12-16T22:35:43Z

OMG. I should've squashed commits myself.

franciszekjob · 2024-12-16T23:09:27Z

I squashed and merged so it's fine 🤔

franciszekjob · 2024-12-17T09:38:01Z

@baitcode you should see the funds on Only Dust 😄

baitcode marked this pull request as draft December 2, 2024 12:36

baitcode force-pushed the feat-snip-9 branch 2 times, most recently from 282d593 to 7f4c71a Compare December 5, 2024 01:43

baitcode marked this pull request as ready for review December 5, 2024 01:44

baitcode added 2 commits December 8, 2024 22:43

baitcode force-pushed the feat-snip-9 branch from 6d4f5f2 to d5de3b1 Compare December 8, 2024 19:44

* lint

9137b40

* test fixes

baitcode force-pushed the feat-snip-9 branch from f265059 to 9137b40 Compare December 8, 2024 22:31

baitcode added 11 commits December 9, 2024 01:37

ok. so I am not allowed to change ci configs

ac7b2cc

Merge branch 'development' into feat-snip-9

6a9fdc9

fixing tests

dcb044b

lint

0487bd6

switched back to increase balance

a79ab75

tiny revert

aceded7

comment documentation test to check if that is what affects test acco…

5851f12

…unt balance

Revert "comment documentation test to check if that is what affects t…

e54e510

…est account balance" This reverts commit 5851f12.

change for another call in documentation. removed account deployment.

7ea8760

fix

14531b1

fix doctest

37f24df

baitcode added 4 commits December 10, 2024 20:23

more fixes

fb9829e

remove balance change calls

4f72882

Fixed naming

1881d84

fix documentation

d369165

added secrets

2c2be9f

franciszekjob requested changes Dec 16, 2024

View reviewed changes

a bit more review comment fixes

c97df0f

baitcode added 2 commits December 16, 2024 15:20

revert execute_v1

86e8f84

remove auto fee estimation

4b0b0a2

franciszekjob reviewed Dec 16, 2024

View reviewed changes

baitcode and others added 3 commits December 16, 2024 14:47

Update docs/guide/account_and_client.rst

ee9a064

Co-authored-by: Franciszek Job <[email protected]>

Update starknet_py/tests/e2e/docs/guide/test_account_sign_outside_tra…

9776344

…nsaction.py Co-authored-by: Franciszek Job <[email protected]>

Merge branch 'development' into feat-snip-9

13d8fe0

franciszekjob reviewed Dec 16, 2024

View reviewed changes

baitcode added 2 commits December 16, 2024 18:23

import fix

ceaa8ca

fix doctest

a9aa417

franciszekjob reviewed Dec 16, 2024

View reviewed changes

starknet_py/tests/e2e/account/outside_execution_test.py Outdated Show resolved Hide resolved

starknet_py/tests/e2e/account/outside_execution_test.py Outdated Show resolved Hide resolved

baitcode and others added 3 commits December 16, 2024 16:32

Update starknet_py/tests/e2e/account/outside_execution_test.py

df4079d

Co-authored-by: Franciszek Job <[email protected]>

Update starknet_py/tests/e2e/account/outside_execution_test.py

817d679

Co-authored-by: Franciszek Job <[email protected]>

changelog

b931c5c

franciszekjob approved these changes Dec 16, 2024

View reviewed changes

docs/migration_guide.rst Outdated Show resolved Hide resolved

baitcode and others added 2 commits December 16, 2024 17:49

Update docs/migration_guide.rst

e5e76e4

Co-authored-by: Franciszek Job <[email protected]>

fixing fixes

7434a25

franciszekjob merged commit a8d7353 into software-mansion:development Dec 16, 2024
9 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support SNIP-9 #1530

Support SNIP-9 #1530

baitcode commented Dec 2, 2024 •

edited

Loading

baitcode commented Dec 5, 2024 •

edited

Loading

franciszekjob commented Dec 10, 2024

baitcode commented Dec 10, 2024 •

edited

Loading

franciszekjob commented Dec 10, 2024 •

edited

Loading

baitcode commented Dec 10, 2024

franciszekjob commented Dec 16, 2024

baitcode commented Dec 16, 2024 •

edited

Loading

franciszekjob commented Dec 16, 2024

franciszekjob commented Dec 16, 2024 •

edited

Loading

franciszekjob left a comment

baitcode commented Dec 16, 2024

baitcode commented Dec 16, 2024

franciszekjob left a comment •

edited

Loading

franciszekjob left a comment

franciszekjob commented Dec 16, 2024

baitcode commented Dec 16, 2024

baitcode commented Dec 16, 2024

franciszekjob commented Dec 16, 2024

franciszekjob commented Dec 17, 2024

Support SNIP-9 #1530

Support SNIP-9 #1530

Conversation

baitcode commented Dec 2, 2024 • edited Loading

Introduced changes

Small things I had to rework on the way

baitcode commented Dec 5, 2024 • edited Loading

franciszekjob commented Dec 10, 2024

baitcode commented Dec 10, 2024 • edited Loading

franciszekjob commented Dec 10, 2024 • edited Loading

baitcode commented Dec 10, 2024

franciszekjob commented Dec 16, 2024

baitcode commented Dec 16, 2024 • edited Loading

franciszekjob commented Dec 16, 2024

franciszekjob commented Dec 16, 2024 • edited Loading

franciszekjob left a comment

Choose a reason for hiding this comment

baitcode commented Dec 16, 2024

baitcode commented Dec 16, 2024

franciszekjob left a comment • edited Loading

Choose a reason for hiding this comment

franciszekjob left a comment

Choose a reason for hiding this comment

franciszekjob commented Dec 16, 2024

baitcode commented Dec 16, 2024

baitcode commented Dec 16, 2024

franciszekjob commented Dec 16, 2024

franciszekjob commented Dec 17, 2024

baitcode commented Dec 2, 2024 •

edited

Loading

baitcode commented Dec 5, 2024 •

edited

Loading

baitcode commented Dec 10, 2024 •

edited

Loading

franciszekjob commented Dec 10, 2024 •

edited

Loading

baitcode commented Dec 16, 2024 •

edited

Loading

franciszekjob commented Dec 16, 2024 •

edited

Loading

franciszekjob left a comment •

edited

Loading