[Improve Existing SDK: Python] - Milestone 1 & 2 #80

Cuttlas90 · 2021-10-26T19:38:06Z

[Improve Existing SDK: Python] - Milestone 1 & 2

Description

This PR is for issue #46.

All user stories are completed, examples and usage documentations are available at: https://github.com/janezpodhostnik/flow-py-sdk

Features:

Blocks:

retrieve a block by ID
retrieve a block by height
retrieve the latest block

Collections:

retrieve a collection by ID

Events:

retrieve events by name in the block height range

Scripts:

submit a script and parse the response
submit a script with arguments and parse the response

Accounts:

retrieve an account by address
create a new account
deploy a new contract to the account
remove a contract from the account
update an existing contract on the account

Transactions:

retrieve a transaction by ID
sign a transaction (single payer, proposer, authorizer or combination of multiple)
submit a signed transaction
sign a transaction with arguments and submit it

Milestones

1: Implement examples covering all user stories. (done)
2: Create tests, documentation, and optimize for performance and usability. (done)
examples, test and documents files are send as PR to flow python sdk

Other Details

Authers

@amhossaini
@barekati

kerrywei · 2021-10-26T20:35:53Z

update:
provided comments in the flow-py-sdk repo

kerrywei · 2021-10-28T20:05:24Z

update:
comments left in the PR. currently blocked by an emulator bug; will continue review once the flow-cli is updated tomorrow so that the emulator no longer panics on certain test cases

kerrywei · 2021-10-29T19:53:52Z

@barekati great progress!
Two suggestions from me:

shall we merge all documentation into one? i.e. following the proposed SDK document template https://github.com/onflow/sdks/blob/main/templates/documentation/template.md . I think you have all use cases covered (you actually have extra ones than the SDK template, which is great & please include all of them), just a matter to use the same template to make documentation format for all SDKs consistent
usability improvement
One thing to check the milestone 2 is to "optimize for performance and usability" (Improve Existing SDK: Python #46). One usability improvement I recommend to tackle is to allow flexible

current sdk does not allow reversing the order of calling .with_payload_signature() and .with_envelope_signature(). For example:

tx = (
                create_account_template(
                    keys = [account_key],
                    reference_block_id = latest_block.id,
                    payer = payer_addr,
                    proposal_key = ProposalKey(
                        key_address = ctx.service_account_address,
                        key_id = ctx.service_account_key_id,
                        key_sequence_number = proposer.keys[
                            ctx.service_account_key_id
                        ].sequence_number,
                    ),
                )
                .add_authorizers(ctx.service_account_address)
                .with_payload_signature(
                    ctx.service_account_address, 0, ctx.service_account_signer
                )
                .with_envelope_signature(
                    payer_addr, 0, gen_signer(payer_private_key.hex()),
                )
            )
            result = await client.execute_transaction(tx)

works, but the following does not:

tx = (
                create_account_template(
                    keys = [account_key],
                    reference_block_id = latest_block.id,
                    payer = payer_addr,
                    proposal_key = ProposalKey(
                        key_address = ctx.service_account_address,
                        key_id = ctx.service_account_key_id,
                        key_sequence_number = proposer.keys[
                            ctx.service_account_key_id
                        ].sequence_number,
                    ),
                )
                .add_authorizers(ctx.service_account_address)
                .with_envelope_signature(
                    payer_addr, 0, gen_signer(payer_private_key.hex()),
                )
                .with_payload_signature(
                    ctx.service_account_address, 0, ctx.service_account_signer
                )
            )
            result = await client.execute_transaction(tx)

^ this will result in error:
"execution error code 1006: [Error Code: 1006] invalid proposal key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature"
To make it easier to use our SDK, we should allow SDK users to be able to freely change the order of setting .with_envelope_signature(), .with_payload_signature(), and/or .add_authorizer().
Does this make sense to you? happy to explain more if it's not clear

Other than these, it's super promising to nail it 👍

In case you did not know: we extended the deadline by one week to Nov 7 to give participants and us enough time to continue tweaking the submissions! See more details in the announcement in our Discord channel (you are super welcome to join there in case you haven't!) -- https://discord.com/channels/613813861610684416/885732226086633483/903516288796721193

Cuttlas90 · 2021-11-01T14:58:36Z

@barekati great progress! Two suggestions from me:

shall we merge all documentation into one? i.e. following the proposed SDK document template https://github.com/onflow/sdks/blob/main/templates/documentation/template.md . I think you have all use cases covered (you actually have extra ones than the SDK template, which is great & please include all of them), just a matter to use the same template to make documentation format for all SDKs consistent

usability improvement
One thing to check the milestone 2 is to "optimize for performance and usability" (Improve Existing SDK: Python #46). One usability improvement I recommend to tackle is to allow flexible

current sdk does not allow reversing the order of calling .with_payload_signature() and .with_envelope_signature(). For example:
tx = (
                create_account_template(
                    keys = [account_key],
                    reference_block_id = latest_block.id,
                    payer = payer_addr,
                    proposal_key = ProposalKey(
                        key_address = ctx.service_account_address,
                        key_id = ctx.service_account_key_id,
                        key_sequence_number = proposer.keys[
                            ctx.service_account_key_id
                        ].sequence_number,
                    ),
                )
                .add_authorizers(ctx.service_account_address)
                .with_payload_signature(
                    ctx.service_account_address, 0, ctx.service_account_signer
                )
                .with_envelope_signature(
                    payer_addr, 0, gen_signer(payer_private_key.hex()),
                )
            )
            result = await client.execute_transaction(tx)
works, but the following does not:
tx = (
                create_account_template(
                    keys = [account_key],
                    reference_block_id = latest_block.id,
                    payer = payer_addr,
                    proposal_key = ProposalKey(
                        key_address = ctx.service_account_address,
                        key_id = ctx.service_account_key_id,
                        key_sequence_number = proposer.keys[
                            ctx.service_account_key_id
                        ].sequence_number,
                    ),
                )
                .add_authorizers(ctx.service_account_address)
                .with_envelope_signature(
                    payer_addr, 0, gen_signer(payer_private_key.hex()),
                )
                .with_payload_signature(
                    ctx.service_account_address, 0, ctx.service_account_signer
                )
            )
            result = await client.execute_transaction(tx) 
^ this will result in error: "execution error code 1006: [Error Code: 1006] invalid proposal key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature" To make it easier to use our SDK, we should allow SDK users to be able to freely change the order of setting .with_envelope_signature(), .with_payload_signature(), and/or .add_authorizer(). Does this make sense to you? happy to explain more if it's not clear

Other than these, it's super promising to nail it 👍

In case you did not know: we extended the deadline by one week to Nov 7 to give participants and us enough time to continue tweaking the submissions! See more details in the announcement in our Discord channel (you are super welcome to join there in case you haven't!) -- https://discord.com/channels/613813861610684416/885732226086633483/903516288796721193

@kerrywei thanks

is done. I just don't delete the other examples yet, in case I need them.
About improvement
I have a point
I think what you propose is not in a same way with flow logic. if I want explain it in a example, its like that I have a cheque, I write payment amount on it, put it in envelop and seal it. if there is a way to take out cheque and sign the payload, it shows I don't seal envelop in a appropriate way.
also I read flow docs ()
in graph it shows payload signature is inside envelop signature. also in authorization it say: The transaction authorization envelope contains both the "transaction payload" and the payload signatures.
so I don't think its possible based on flow workflow. Does this make sense to you? I will be glad if feedback me.

janezpodhostnik · 2021-11-01T15:41:50Z

current sdk does not allow reversing the order of calling .with_payload_signature() and .with_envelope_signature()

@barekati
The way this would function is that the methods in tx.py methods with_payload_signature and with_envelope_signature would get renamed to with_payload_signer and with_envelope_signer. They would also change so that instead on signing the transaction when you call them they would just store the signer. The transaction would only be signed in client.py execute_transaction right before this line: https://github.com/janezpodhostnik/flow-py-sdk/blob/716c1690f38eeb78f479d1cf860b974cc6a53b04/flow_py_sdk/client/client.py#L200.

So basically you give the Tx object the stamps (signers) and once everything is ready the Tx object gets everything in place and wacks it with the stamps just before sending it away.

Does that make sense?

Cuttlas90 · 2021-11-01T15:50:12Z

Yes, i get what you say

Cuttlas90 · 2021-11-01T19:35:01Z

current sdk does not allow reversing the order of calling .with_payload_signature() and .with_envelope_signature()

@barekati The way this would function is that the methods in tx.py methods with_payload_signature and with_envelope_signature would get renamed to with_payload_signer and with_envelope_signer. They would also change so that instead on signing the transaction when you call them they would just store the signer. The transaction would only be signed in client.py execute_transaction right before this line: https://github.com/janezpodhostnik/flow-py-sdk/blob/716c1690f38eeb78f479d1cf860b974cc6a53b04/flow_py_sdk/client/client.py#L200.

So basically you give the Tx object the stamps (signers) and once everything is ready the Tx object gets everything in place and wacks it with the stamps just before sending it away.

Does that make sense?

I fixed the issue. now payload ,envelope and authorizer order is not important.

janezpodhostnik · 2021-11-01T21:57:40Z

I left some minor comments, but the solution looks good.

kerrywei · 2021-11-04T17:57:03Z

update: I'll give another look soooon today.

kerrywei · 2021-11-04T20:13:07Z

@barekati I don't have further comments on that PR. My teammate @janezpodhostnik will merge in the Python SDK PR tmr, then merge in this PR.

janezpodhostnik · 2021-11-05T14:32:37Z

Nice work! Thank you very much! 🎉 🚀

Cuttlas90 added 6 commits October 14, 2021 20:13

Improve Existing SDK: Python - Milestone 1 & 2

d4f584d

Create milestone-1&2.md

160da64

milestone1&2

322d997

Merge branch 'main' of https://github.com/barekati/flip-fest

17dc9f6

Update mohsen23_1423.md

3c6645b

Delete milestone-1&2.md

2364154

janezpodhostnik requested review from janezpodhostnik and kerrywei October 26, 2021 19:46

kerrywei changed the title ~~This PR is for issue #46.~~ [Improve Existing SDK: Python] - Milestone 1 & 2 Oct 27, 2021

janezpodhostnik merged commit 0c55f6a into onflow:main Nov 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improve Existing SDK: Python] - Milestone 1 & 2 #80

[Improve Existing SDK: Python] - Milestone 1 & 2 #80

Cuttlas90 commented Oct 26, 2021 •

edited

Loading

kerrywei commented Oct 26, 2021

kerrywei commented Oct 28, 2021

kerrywei commented Oct 29, 2021

Cuttlas90 commented Nov 1, 2021 •

edited

Loading

janezpodhostnik commented Nov 1, 2021

Cuttlas90 commented Nov 1, 2021

Cuttlas90 commented Nov 1, 2021

janezpodhostnik commented Nov 1, 2021

kerrywei commented Nov 4, 2021

kerrywei commented Nov 4, 2021

janezpodhostnik commented Nov 5, 2021

[Improve Existing SDK: Python] - Milestone 1 & 2 #80

[Improve Existing SDK: Python] - Milestone 1 & 2 #80

Conversation

Cuttlas90 commented Oct 26, 2021 • edited Loading