Use Nile's Signer #283
Use Nile's Signer #283
Conversation
tests/utils.py
Outdated
| @@ -128,7 +125,7 @@ def cached_contract(state, definition, deployed): | |||
| return contract | |||
|
|
|||
|
|
|||
| class Signer(): | |||
| class ActivatedSigner(): | |||
There was a problem hiding this comment.
It's not clear to me what the right name for this is. I was considering moving the transaction functionality upstream to Nile, but the reason we had this split in the first time is that Nile's should be generic enough while this is tied to StarkNet's testing framework -- which is not good to send transactions to public chains.
We could either abstract the provider (testing fw / starknet.py / public net?) on Nile's side and just provision the testing fw from each test and not have this intermediate abstraction, or maybe rename this one to TestSigner or something like that.
There was a problem hiding this comment.
Hmmm yeah, I struggled a little bit with naming this (SignerWithAccount, ExtendedSigner, and this). I think renaming to TestSigner is perfectly fine. We're just using it here to test the contracts anyway.
tox.ini
Outdated
| @@ -16,6 +16,7 @@ passenv = | |||
| PYTHONPATH | |||
| deps = | |||
| cairo-lang==0.8.1 | |||
| cairo-nile | |||
There was a problem hiding this comment.
should we fix versions in here to avoid breaking changes impacting this repo's development?
There was a problem hiding this comment.
Yes, good idea
| @@ -1,11 +1,12 @@ | |||
| import pytest | |||
| from starkware.starknet.testing.starknet import Starknet | |||
| from utils import ( | |||
| Signer, str_to_felt, TRUE, FALSE, get_contract_def, cached_contract, assert_revert, to_uint | |||
| ActivatedSigner, str_to_felt, TRUE, FALSE, get_contract_def, cached_contract, | |||
There was a problem hiding this comment.
I know this PR is old, but would that TRUE and FALSE end up in main if we merge this?
There was a problem hiding this comment.
Would that TRUE and FALSE end up in main as a duplicate you mean? If that's the question, I'll rebase first and make sure there are no duplicates
docs/Account.md
Outdated
|
|
||
| To use Signer, pass a private key when instantiating the class: | ||
| * reformats callarray |
There was a problem hiding this comment.
should "callarray" be formatted somehow? has been defined before? reformat how?
There was a problem hiding this comment.
Good questions. It's really just converting callarray[0] to hex so "reformat" isn't correct. I'll better explain this and include where it's first defined as well.
docs/Account.md
Outdated
|
|
||
| To use Signer, pass a private key when instantiating the class: | ||
| * reformats callarray | ||
| * a necessary process to convert the `to` contract address to hexadecimal format |
There was a problem hiding this comment.
Because Nile converts to to base-16 which requires the to address to be a string. Converting the address with str() creates an integer that exceeds Cairo's FIELD_PRIME...I'll add this to the docs :)
Co-authored-by: Martín Triay <[email protected]>
This PR removes the local
Signerin utils.py and instead imports/uses Nile'sSignerclass. This PR proposes to include theActivatedSignerclass.ActivatedSignerperforms the following:Signer__execute__with the tx signature returned from Nile's SignerFurther, this PR includes updated documentation regarding how
ActivatedSignerworks with Nile'sSigner. This resolves #221.