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

Replace Deprecated Account type with Keypair #92

Merged
merged 9 commits into from
Nov 4, 2021
Merged

Replace Deprecated Account type with Keypair #92

merged 9 commits into from
Nov 4, 2021

Conversation

Imaclean74
Copy link
Contributor

@Imaclean74 Imaclean74 commented Oct 13, 2021

The Account class was recently deprecated in favor of Keypair in the upstream solana python package as well as the js web3 library. This PR brings pyserum in line with that and resolves #88 and #89

Summary of changes:

  • Replace all usage of Account with Keypair
  • update all relevant tests as well

solana-py change :
michaelhly/solana-py#62

web3 :
Deprecate Account for Keypair: https://solana-labs.github.io/solana-web3.js/classes/account.html

@Imaclean74 Imaclean74 changed the title Replace Deprecated Account type with Keypair #88 Replace Deprecated Account type with Keypair Oct 13, 2021
@Imaclean74 Imaclean74 changed the title Replace Deprecated Account type with Keypair [WIP] Replace Deprecated Account type with Keypair Oct 13, 2021
@vikulikov
Copy link

successfully placed an order with this PR

@Imaclean74
Copy link
Contributor Author

thanks for verifying @vikulikov. I'm working to get the integration tests all working. Had some issues bootstrapping serum-dex locally.

@leofisG
Copy link
Contributor

leofisG commented Oct 25, 2021

Hey @Imaclean74 , anything that I can help with?

@Imaclean74
Copy link
Contributor Author

thanks @leonardgee. I've had some issues running the integration tests locally - older PC not running the release builds of solana. So have been pushing fixes and waiting for the CI to run :(
if the last commit still triggers errors - it would be great if you could run the integration tests locally - and flag any required fixes.

@Imaclean74
Copy link
Contributor Author

oh - and there is also some weirdness with stubbed_base_mint() in conftest.py. It was returning an Account ( changed to Keypair ), but it should almost certainly be a public_key - which is what test_bootstrapped_market() has in it's type hints. But then it was doing a public_key() call on the input which would only work if passed an Account. if you have some insight into how the bootstrapping should work in conftest.py that would be great. Maybe just as simple as returning something like return PublicKey(__bs_params["coin_mint"]) from stubbed_base_mint() ?

@Imaclean74
Copy link
Contributor Author

@leonardgee I have the integration tests passing locally now - against Solana v1.5.8. With the latest ( 1.7.15 ) I'm getting a couple of failures that will need digging in to. However that looks unrelated to my Account -> Keypair change. It might be tidier to address that in a separate PR ?

@codecov
Copy link

codecov bot commented Nov 3, 2021

Codecov Report

Merging #92 (02afda8) into alpha (4ab65b0) will not change coverage.
The diff coverage is 76.19%.

@@           Coverage Diff           @@
##            alpha      #92   +/-   ##
=======================================
  Coverage   87.17%   87.17%           
=======================================
  Files          24       24           
  Lines        1053     1053           
=======================================
  Hits          918      918           
  Misses        135      135           

@michaelhly michaelhly changed the title [WIP] Replace Deprecated Account type with Keypair Replace Deprecated Account type with Keypair Nov 4, 2021
@michaelhly michaelhly merged commit 7d10ecc into serum-community:alpha Nov 4, 2021
@Imaclean74
Copy link
Contributor Author

thanks for merging @michaelhly. Would this now be ready to publish to Pypi ?

@leofisG
Copy link
Contributor

leofisG commented Nov 5, 2021

Thanks for this! @Imaclean74

@leofisG
Copy link
Contributor

leofisG commented Nov 5, 2021

Why is the doc failure after merge. 🤔 Is this caused by the dependency upgrade?

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.

Account type has been deprecated
4 participants