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

[Python] Simulation and other trivial improvements #5770

Merged
merged 4 commits into from
Dec 6, 2022

Conversation

davidiw
Copy link
Contributor

@davidiw davidiw commented Dec 4, 2022

  • Cleaned up some typing
  • Some aesthetic changes
  • Support for simulating txns within python
  • Basic support for a client config

tested locally and now with ci/cd!

I verified that the API guarantees to return data and that it won't be
optional if it is a successful response
…ansaction

This is the default transaction type, this is where folks will look
first
verified with an example
this allows folks to customize the internal values for each function
without adding bloat. I'll write up a longer task on this so it can be
used at a more granular level.
Copy link
Contributor

@gregnazario gregnazario left a comment

Choose a reason for hiding this comment

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

It looks fine to me, except we really need a test harness around this code

Comment on lines +37 to +48
transaction = rest_client.create_bcs_transaction(alice, TransactionPayload(payload))

print("\n=== Simulate before creatng Bob's Account ===")
output = rest_client.simulate_transaction(transaction, alice)
assert output[0]["vm_status"] != "Executed successfully", "This shouldn't succeed"
print(json.dumps(output, indent=4, sort_keys=True))

print("\n=== Simulate after creatng Bob's Account ===")
faucet_client.fund_account(bob.address(), 0)
output = rest_client.simulate_transaction(transaction, alice)
assert output[0]["vm_status"] == "Executed successfully", "This should succeed"
print(json.dumps(output, indent=4, sort_keys=True))
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes...

@davidiw
Copy link
Contributor Author

davidiw commented Dec 5, 2022

It looks fine to me, except we really need a test harness around this code

I'm working on a set of new platform tasks. Some good stuff for some eager folks in the ecosystem :)

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.

2 participants