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

Upgrade Gummyroll SDK to Solita #164

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

Conversation

samwise2
Copy link
Collaborator

@samwise2 samwise2 commented Aug 2, 2022

This PR upgrades the Gummyroll SDK to use Solita. Creating instructions is now facilitated by Solita ix generators, with some wrappers for convenience. The main "manual" piece of code which remains is a deserializer for merkle roll accounts, since Solita will not generate a beet which we can use for that purpose (since its mainly free-form bytes other than the header).

The latest SDK changes are published here: https://www.npmjs.com/package/@sorend-solana/gummyroll.

All services in this repo have been upgraded to work with the new release.

Follow up work:

  • Upgrade our use of @solana/spl-token to 0.2.0
  • Remove our use of types from contracts/target/types
  • Minified browser build for gummyroll/utils SDKs (or maybe all SDKs but metaplex will do their own thing once they take over Bubblegum, GBall, SS)

Copy link
Collaborator

@ngundotra ngundotra left a comment

Choose a reason for hiding this comment

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

Looking great Sam! Keep up the momentum.

Can you attempt to replace the .getProvider() call with a manually created provider in the gummyroll test? Would hate to accidentally run something on testnet given our current infra setup.

Otherwise this is great. Good design naming proof convenience ix names. Getting this right really helps out in the long run.

Once you make the changes I requested, go ahead and merge.

@@ -24,7 +24,7 @@
"@project-serum/anchor": "0.24.2",
"@solana/spl-token": "^0.1.8",
"@solana/web3.js": "^1.50.1",
"@sorend-solana/gummyroll": "^0.0.5",
"@sorend-solana/gummyroll": "^0.0.6",
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah yes, the beginning of package administration h#ll

const generatedSDKDir = path.join(__dirname, 'src', 'generated');

async function main() {
const anchorExecutable = realpathSync("../../../deps/anchor/target/debug/anchor");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add to list of todos: switch to normal anchor since it builds to solana 1.10 now

Comment on lines +17 to +20
export function createInitGummyrollWithRootWithProofInstruction(
accts: InitGummyrollWithRootInstructionAccounts,
args: InitGummyrollWithRootInstructionArgs,
proof: Buffer[]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -3,19 +3,24 @@
# plz update this script so all devs can easily get setup
Copy link
Collaborator

Choose a reason for hiding this comment

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

add
set -e
to the top which will exit the script if any line throws an error

);
}
let txId = await execute(Gummyroll.provider, ixs, [
let txId = await execute(anchor.getProvider(), ixs, [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer using a provider object created at the top of the file, alongside connection, if possible

Comment on lines -35 to -37
// @ts-ignore
let Gummyroll;

Copy link
Collaborator

Choose a reason for hiding this comment

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

💪 one step at a time, we will remove these types

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