Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Greeter Contract Documentation to use local signing instead of transact #2414

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lauwsj-alpha
Copy link

@lauwsj-alpha lauwsj-alpha commented Apr 5, 2022

What was wrong?

Fix #2386, Currently not all Ethereum nodes support signing transaction and it would be better to sign the transactions locally.

How was it fixed?

Replace all the transact() method with send_raw_transaction and also add example using construct_sign_and_send_raw_middleware for Greeter programme

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@lauwsj-alpha lauwsj-alpha changed the title Documentation fix/simon 05042022 contracts Fix Greeter Contract Documentation to use local signing instead of transact Apr 5, 2022
@lauwsj-alpha lauwsj-alpha force-pushed the documentation_fix/simon_05042022_contracts branch from 1986cd0 to 00c2042 Compare April 5, 2022 16:19
Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for how long it took to review this... there are a lot of comments here but it's mostly about using snake_case over camelCase in a few places so don't worry 😅.

I also posed a question on the EthereumTesterProvider() examples that I'm curious to hear thoughts on.

Thanks for taking this on... this is a great addition to keep our docs up-to-date 👍



Using we can use ``send_raw_transaction`` to create the contract :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?

Suggested change
Using we can use ``send_raw_transaction`` to create the contract :
We can use ``send_raw_transaction`` to create the contract:

>>> tx_receipt = w3.eth.wait_for_transaction_receipt(tx_hash)
>>> greeter.functions.greet().call()
'Nihao'

or we can set up ``construct_and_sign_middleware`` and let the middleware signs the transaction:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or we can set up ``construct_and_sign_middleware`` and let the middleware signs the transaction:
or we can set up ``construct_and_sign_middleware`` and let the middleware sign the transaction:

@@ -35,7 +35,7 @@ After ``py-solc-x`` is installed, you will need to install a version of ``solc``
>>> from solcx import install_solc
>>> install_solc(version='latest')

You should now be set up to run the contract deployment example below:
You should now be set up to run the contract deployment example below :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, this space isn't necessary and keeps consistency across docs

Suggested change
You should now be set up to run the contract deployment example below :
You should now be set up to run the contract deployment example below:

# Build the transaction parameters that deploys the contract
>>> tx_params = Greeter.constructor().buildTransaction()

# Send the transaction and let the middleware signs it
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Send the transaction and let the middleware signs it
# Send the transaction and let the middleware sign it

>>> tx_params = token_contract.constructor(web3.eth.coinbase, 12345).buildTransaction({
'nonce': nonce
})
>>> signed_transaction = my_account.signTransaction(tx_params)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

signTransaction (camelCase) has been deprecated over the snake_case

Suggested change
>>> signed_transaction = my_account.signTransaction(tx_params)
>>> signed_transaction = my_account.sign_transaction(tx_params)

>>> tx_hash = contract.functions.transfer(alice, 10).transact({'gas': 899000, 'gasPrice': 674302241})
>>> private_key = provider.ethereum_tester.backend.account_keys[0]
>>> alice_nonce = w3.eth.get_transaction_count(alice_account.address)
>>> tx_params = contract.functions.transfer(alice, 10).buildTransaction({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> tx_params = contract.functions.transfer(alice, 10).buildTransaction({
>>> tx_params = contract.functions.transfer(alice, 10).build_transaction({

... 'gasPrice': 674302241,
... 'nonce': alice_nonce,
... })
>>> signed_transaction = alice_account.signTransaction(tx_params)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> signed_transaction = alice_account.signTransaction(tx_params)
>>> signed_transaction = alice_account.sign_transaction(tx_params)


>>> my_account = w3.eth.account.from_key(PRIVATE_KEY)
>>> nonce = w3.eth.get_transaction_count(my_account.address)
>>> tx_params = deployed_contract.functions.update({'s1': ['0x0000000000000000000000000000000000000001', '0x0000000000000000000000000000000000000002'], 's2': [b'0'*32, b'1'*32], 'users': []}).buildTransaction({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> tx_params = deployed_contract.functions.update({'s1': ['0x0000000000000000000000000000000000000001', '0x0000000000000000000000000000000000000002'], 's2': [b'0'*32, b'1'*32], 'users': []}).buildTransaction({
>>> tx_params = deployed_contract.functions.update({'s1': ['0x0000000000000000000000000000000000000001', '0x0000000000000000000000000000000000000002'], 's2': [b'0'*32, b'1'*32], 'users': []}).build_transaction({

>>> tx_params = deployed_contract.functions.update({'s1': ['0x0000000000000000000000000000000000000001', '0x0000000000000000000000000000000000000002'], 's2': [b'0'*32, b'1'*32], 'users': []}).buildTransaction({
'nonce': nonce
})
>>> signed_transaction = my_account.signTransaction(tx_params)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> signed_transaction = my_account.signTransaction(tx_params)
>>> signed_transaction = my_account.sign_transaction(tx_params)

@@ -1072,75 +1179,106 @@ Event Log Object

from web3 import Web3
from hexbytes import HexBytes
w3 = Web3(Web3.EthereumTesterProvider())
provider = Web3.EthereumTesterProvider()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thought as above on the EthereumTesterProvider()... do we just keep this simpler? This isn't a node and we don't need to tiptoe around anything by signing before sending to eth-tester. So should we keep the api simpler here? I can go either way but I think I lean towards the simpler setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert Greeter contract example to use local signing middleware
4 participants