Fix account signature #387
Conversation
|
Why is the ecdsa_ptr implicit argument required in |
| # bump nonce | ||
| await signer.send_transactions(account, [(initializable.contract_address, 'initialized', [])]) | ||
|
|
There was a problem hiding this comment.
was this passing before? if so, why? if not, why is it needed?
There was a problem hiding this comment.
This was passing before, but with the proposed changes, the nonce's 0 - 1 is out of range for a valid signature, which in turn, returns a different error. This is similar to what happened here:
#361 (comment)
| return _unsafe_execute(call_array_len, call_array, calldata_len, calldata, nonce) | ||
| end | ||
|
|
||
| func _unsafe_execute{ | ||
| syscall_ptr : felt*, | ||
| pedersen_ptr : HashBuiltin*, | ||
| range_check_ptr, | ||
| ecdsa_ptr: SignatureBuiltin*, |
There was a problem hiding this comment.
following Fran's comment, this shouldn't be needed right?
There was a problem hiding this comment.
Why is the ecdsa_ptr implicit argument required in unsafe_execute ?
Compilation without the ecdsa_ptr in _unsafe_execute fails:
E starkware.cairo.lang.compiler.preprocessor.preprocessor_error.PreprocessorError: /Users/andrewfleming/Documents/starknet/cairo-contracts/cairo-contracts/.tox/default/lib/python3.7/site-packages/openzeppelin/account/library.cairo:211:9: Cannot convert the implicit arguments of _unsafe_execute to the implicit arguments of execute.
E return _unsafe_execute(call_array_len, call_array, calldata_len, calldata, nonce)
E ^*******************************************************************************^
E The implicit arguments of '_unsafe_execute' were defined here:
E /Users/andrewfleming/Documents/starknet/cairo-contracts/cairo-contracts/.tox/default/lib/python3.7/site-packages/openzeppelin/account/library.cairo:241:26
E func _unsafe_execute{
E ^
E The implicit arguments of 'execute' were defined here:
E /Users/andrewfleming/Documents/starknet/cairo-contracts/cairo-contracts/.tox/default/lib/python3.7/site-packages/openzeppelin/account/library.cairo:187:18
E func execute{
E ^
_unsafe_execute makes the actual contract call and seems to require passing the sig builtin for verification:
I'm still trying to understand this issue.
There was a problem hiding this comment.
And what if we explicitly pass the implicit arguments without including ecdsa_ptr? It looks like it's trying to pass along all the available implicit arguments including those that aren't needed.
There was a problem hiding this comment.
If I understand you question correctly, I don't think this works. Taking out the implicit ecdsa_ptr means we'll still need to create some reference to it to validate the signature...which brings us back to the same initial issue: Signature hint must point to the signature builtin segment, not {addr}.
There was a problem hiding this comment.
My suggestion was to write the call as
_unsafe_execute{syscall_ptr, pedersen_ptr, etc...}(call_array_len, call_array, calldata_len, calldata, nonce)
(I'm not sure if this works)
There was a problem hiding this comment.
Afaict the implicit arguments are not only received by _unsafe_execute but also passed to the next function call, doing so could revoke ecdsa_ptr but other than that it might work.
|
Just to ask about this, accounts created on goerli with 0.2.0 and have funds in them are currently irrecoverable? |
I'm afraid so. Mainnet ones shouldn't since they weren't whitelisted. Are you aware of any relevant one that need a solution? |
|
Nothing of notice. I was just playing around with |
|
BTW, trying to compile the latest Could it be related? |
Do you see this in the stdout? |
|
Indeed Should I compile it like this: |
|
Yes. This is fixed in Nile 0.7.0 (fixed in OpenZeppelin/nile#112, about to be integrated in #381). |
Resolves #386.
This proposed Account change will yield an artifact upon compilation that should be added to Nile to address issue 143.