Support `create` and `update` token metadata in stake-pool python bindings #3978

gnapoli23 · 2023-01-17T17:37:18Z

Problem
#3335 added new instructions (create_metadata and update_metadata) to the stake-pool, while these are not supported in python.

Solution
Add python bindings create_token_metadata and update_token_metadata in stake-pool, with their relative tests.

Fixes #3370

joncinque

This is on the right track! You'll need to be careful on the instruction encoding, but other than that, all good

joncinque · 2023-01-17T18:19:39Z

stake-pool/py/stake_pool/constants.py

@@ -17,6 +17,9 @@
 MINIMUM_ACTIVE_STAKE: int = MINIMUM_DELEGATION
 """Minimum active delegated staked required in a stake account"""

+METADATA_PROGRAM_ID: PublicKey = PublicKey("metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s")
+"""Public key that identifies the SPL Token Metadata program."""


nit: this is the metaplex metadata program

Suggested change

"""Public key that identifies the SPL Token Metadata program."""

"""Public key that identifies the Metaplex Token Metadata program."""

Yes sure, C/P mistake 😅

stake-pool/py/stake_pool/instructions.py

joncinque · 2023-01-17T18:34:39Z

stake-pool/py/stake_pool/instructions.py

+    (withdraw_authority, _seed) = find_withdraw_authority_program_address(params.program_id, params.stake_pool)
+    (token_metadata, _seed) = find_metadata_account(params.pool_mint)
+
+    keys = [
+        AccountMeta(pubkey=params.stake_pool, is_signer=False, is_writable=False),
+        AccountMeta(pubkey=params.manager, is_signer=True, is_writable=False),
+        AccountMeta(pubkey=withdraw_authority, is_signer=False, is_writable=False),
+        AccountMeta(pubkey=params.pool_mint, is_signer=False, is_writable=False),
+        AccountMeta(pubkey=params.payer, is_signer=True, is_writable=True),
+        AccountMeta(pubkey=token_metadata, is_signer=False, is_writable=True),
+        AccountMeta(pubkey=METADATA_PROGRAM_ID, is_signer=False, is_writable=False),
+        AccountMeta(pubkey=SYS_PROGRAM_ID, is_signer=False, is_writable=False),
+        AccountMeta(pubkey=SYSVAR_RENT_PUBKEY, is_signer=False, is_writable=False),
+    ]


nit: I know it's not the easiest usage pattern, but to make this consistent with the other instruction creators, can you provide the ability to specify all accounts?

gnapoli23 · 2023-01-20T22:07:29Z

@joncinque
Updated as discussed. About tests, can you give me more details? I took a look at the one related to deposit_sol as you suggested, but unfortunately i didn't got what is the logic flow to follow regarding new create and update functions.
Is there any Rust test about this fns that has to be considered as reference test?
More over, I also have to implement stake-pool's py async actions for these new functions, right? Seems there's nothing present yet about.

joncinque · 2023-01-24T18:35:51Z

The idea is to test that the new instruction bindings work, and that you can create and update token metadata through the stake pool program. The deposit_sol test just showed the overall flow of what one of those python tests looks like.

You can see the metadata rust tests at https://github.com/solana-labs/solana-program-library/blob/master/stake-pool/program/tests/create_pool_token_metadata.rs and https://github.com/solana-labs/solana-program-library/blob/master/stake-pool/program/tests/update_pool_token_metadata.rs.

Feel free to add actions for these too, it'll make testing easier.

And finally, you'll need to add the token metadata program to the test validator when it's booted for testing, so be sure to change this in conftest.py:

@pytest.fixture(scope="session")
def solana_test_validator():
    old_cwd = os.getcwd()
    newpath = tempfile.mkdtemp()
    os.chdir(newpath)
    validator = Popen([
        "solana-test-validator",
        "--reset", "--quiet",
        "--bpf-program", "SPoo1Ku8WFXoNDMHPsrGSTSG1Y47rzgn41SLUNakuHy",
        f"{old_cwd}/../../target/deploy/spl_stake_pool.so",
        "--bpf-program", "metaqbxxUerdq28cj1RbAWkYQm3ybzjb6a8bt518x1s",
        f"{old_cwd}/../program/tests/fixtures/mpl_token_metadata.so",
        "--slots-per-epoch", str(NUM_SLOTS_PER_EPOCH),
    ],)
    yield
    validator.kill()
    os.chdir(old_cwd)
    shutil.rmtree(newpath)

gnapoli23 · 2023-01-31T21:36:09Z

@joncinque I added actions for create_token_metadata and update_token_metdata, can you give a look? I'm not so sure about the signers in send_transaction, ' cause i saw that in Rust code there's also the stake pool manager as signer, but in python it is actually a PublicKey and not a Keypair (as expected by the function itself).
I'll add tests in the maintime :)

Btw sorry for rebases, I added ref to the issue in some commits that were missing it.

joncinque

I'm not so sure about the signers in send_transaction, ' cause i saw that in Rust code there's also the stake pool manager as signer, but in python it is actually a PublicKey and not a Keypair (as expected by the function itself).

The action is a simplified wrapper that assumes the payer is actually the manager, and there is no analogue to the Rust side, since it's also signing and sending the transaction, whereas the Rust version is just creating the instruction.

joncinque · 2023-02-01T11:21:45Z

stake-pool/py/stake_pool/actions.py

+            symbol=symbol,
+            uri=uri,
+            withdraw_authority=withdraw_authority,
+            token_metadata=METADATA_PROGRAM_ID,


This should be the actual metadata account, and not the metadata program id.

joncinque · 2023-02-01T11:22:35Z

stake-pool/py/stake_pool/actions.py

+
+    txn = Transaction()
+    txn.add(
+        sp.CreateTokenMetadataParams(


Maybe I'm missing something, but this just gives a tuple, and not an instruction. You'll need to call create_metadata_account with sp.CreateTokenMetadataParams as a parameter.

Yep sorry, missed it

joncinque · 2023-02-01T11:24:28Z

stake-pool/py/stake_pool/instructions.py

+    if params.withdraw_authority:
+        withdraw_authority = params.withdraw_authority
+    else:
+        (withdraw_authority, _seed) = find_withdraw_authority_program_address(params.program_id, params.stake_pool)
+
+    if params.token_metadata:
+        token_metadata = params.token_metadata
+    else:
+        (token_metadata, _seed) = find_metadata_account(params.pool_mint)


To avoid the earlier footgun, let's keep this logic as dumb as possible and put all parameters in CreateTokenMetadataParams, including the rent sysvar, system program id, and metadata program id, and not make any of them optional.

Essentially, let's make this the same as all of the other instruction creators, and then the action can fill in the right parameters

joncinque · 2023-02-01T11:58:29Z

Btw, looking through your PR made me realize that we're behind in the stake pool program, so I've updated to the newest bindings at #4012 -- you'll just need to remove the sysvar rent account during "create metadata".

gnapoli23 · 2023-02-01T16:25:49Z

@joncinque Thanks, I did some updates following your guidelines, hope they're correct. I also added a simple test, can you check if it's ok?

joncinque

It's looking close! Once the create test works, then you just need an update test, then this should be good to go!

joncinque · 2023-02-01T23:06:37Z

stake-pool/py/stake_pool/actions.py

+async def create_token_metadata(client: AsyncClient, payer: Keypair, stake_pool_program_id: PublicKey,
+                                stake_pool_address: PublicKey, token_program_id: PublicKey,
+                                withdraw_authority: PublicKey, name: str, symbol: str, uri: str):


nit: For the actions, let's keep it simpler, same as you're using it in the test, and in this function you can derive the missing parameters:

Suggested change

async def create_token_metadata(client: AsyncClient, payer: Keypair, stake_pool_program_id: PublicKey,

stake_pool_address: PublicKey, token_program_id: PublicKey,

withdraw_authority: PublicKey, name: str, symbol: str, uri: str):

async def create_token_metadata(client: AsyncClient, payer: Keypair,

stake_pool_address: PublicKey,

name: str, symbol: str, uri: str):

joncinque · 2023-02-01T23:15:21Z

stake-pool/py/tests/test_create_token_metadata.py

+    data = resp['result']['value']['data']
+    assert data[0] == name
+    assert data[1] == symbol
+    assert data[2] == uri


You'll need a way to deserialize the data in the metadata account, which will be a bit annoying. Looking at the struct space definitions at https://github.com/metaplex-foundation/metaplex-program-library/blob/0e999e2507118a7e930c3a176709d4ab559e060b/token-metadata/program/src/state/metadata.rs#L17, looks like the name should be between indexes 69 and 101, the symbol between 105 and 115, and the uri between 119 and 319, and I think you can interpret those bytes as utf8, so just directly pull out those ranges

Yes I saw that data in Rust code are deserialized in the Metadata struct, I felt something was definitely missing. I did some change at the test and reverted the last commit on actions.

gnapoli23 · 2023-02-16T11:49:45Z

Hi @joncinque , sorry to bother...did you read my last comment? If you can, please check the latest commit so I can move forward following you direction. Thanks a lot.

joncinque

Sorry, I thought you were still working on it. Once you fix the issues, get the create test working, and add an update test, this should be good to go!

joncinque · 2023-02-16T13:55:02Z

stake-pool/py/stake_pool/actions.py

+                symbol=symbol,
+                uri=uri,
+                withdraw_authority=withdraw_authority,
+                token_metadata=TOKEN_PROGRAM_ID,


token_metadata should be the result of find_metadata_account(stake_pool.pool_mint), and then you still need to add the system program id and token metadata program id

joncinque · 2023-02-16T13:55:28Z

stake-pool/py/stake_pool/actions.py

+                symbol=symbol,
+                uri=uri,
+                withdraw_authority=withdraw_authority,
+                token_metadata=TOKEN_PROGRAM_ID,


Same here, you'll need find_metadata_account(stake_pool.pool_mint) and the metadata program id

gnapoli23 · 2023-02-23T14:53:32Z

@joncinque Hi, I updated actions.py and the solana_test_validator setup function as suggested, but unfortunately I'm not able to run test somehow, it keeps on telling me that:
ERROR solanaweb3.rpc.httprpc.HTTPClient:async_http.py:35 Health check failed with error: All connection attempts failed

Giving you more info, I'm working in a VirtualBox Ubuntu machine, don' t know if it's related to this.
I also tried to run the setup-test-validato.sh script to see if something changes, it creates the validators pool (indeed creates also all related files under keys folder, but still gives the same error).

Hope this can be relevant info, sorry to bother you :)

joncinque · 2023-03-08T15:57:14Z

It looks like there was a bug in your test, since it's trying to decode the wrong part of the response. Be sure to check the errors reported when you run python3 -m pytest!

Here's a patch that will fix your test and make everything work:

-    raw_data = resp['result']
-    assert name == str(raw_data[69:101], "utf-8")
-    assert symbol == str(raw_data[105:115], "utf-8")
-    assert uri == str(raw_data[119:319], "utf-8")
+    data = resp['result']['value']['data']
+    raw_data = decode_byte_string(data[0], data[1])
+    assert name == str(raw_data[69:101], "utf-8")[:len(name)]
+    assert symbol == str(raw_data[105:115], "utf-8")[:len(symbol)]
+    assert uri == str(raw_data[119:319], "utf-8")[:len(uri)]

After that, you just need an update test, then this will be good to go!

)

…olana-labs#3370)

This reverts commit 3cb0f8b.

This reverts commit f653311.

gnapoli23 · 2023-03-14T15:40:23Z

@joncinque Hi! Rebased, fixed the create test and added the update one. Thanks for your support!

joncinque · 2023-03-14T20:22:45Z

Thanks for all of your patience and for integrating all of my suggestions -- this is finally good to go!

…dings (solana-labs#3978)

mergify bot added the community Community contribution label Jan 17, 2023

gnapoli23 mentioned this pull request Jan 17, 2023

stake-pool-py: Support add / update metadata for pool token #3370

Closed

joncinque reviewed Jan 17, 2023

View reviewed changes

gnapoli23 force-pushed the py-add-update-token-metadata branch from f5d6e61 to 760639d Compare January 31, 2023 21:17

gnapoli23 force-pushed the py-add-update-token-metadata branch 4 times, most recently from 0a9bb5a to a231551 Compare January 31, 2023 21:46

joncinque reviewed Feb 1, 2023

View reviewed changes

gnapoli23 force-pushed the py-add-update-token-metadata branch from a45222a to 9ea6458 Compare February 1, 2023 15:31

joncinque reviewed Feb 1, 2023

View reviewed changes

joncinque reviewed Feb 16, 2023

View reviewed changes

gnapoli23 force-pushed the py-add-update-token-metadata branch from 4571a8e to 6e73e00 Compare February 23, 2023 11:18

gnapoli23 added 9 commits March 14, 2023 15:38

implemented create_token_metadata (solana-labs#3370)

4d16e2d

fix missing create_token_metadata instruction layout (solana-labs#3370

fbe20cf

)

add update_token_metadata (solana-labs#3370)

9f4cbf0

add optional withdraw_authority and token_metadata specification (s…

8e29817

…olana-labs#3370)

fix token metadata struct layout (solana-labs#3370)

55da6a0

add create and update token metadata actions (solana-labs#3370)

a5867e4

fix send_transaction (solana-labs#3370)

8088c7b

fix payer (solana-labs#3370)

0dadaac

fix metadata program id (solana-labs#3370)

17624ac

gnapoli23 added 10 commits March 14, 2023 15:38

Revert "fix metadata program id (solana-labs#3370)"

2e22ec8

This reverts commit 3cb0f8b.

fix actions, update Create and Update params and instructions

4885be1

add simple test for create

b1bfa90

update actions (solana-labs#3370)

4d0ff9b

Revert "update actions (solana-labs#3370)"

345d55b

This reverts commit f653311.

update test

01508ab

raw data from result

89ee10c

update actions

57777f5

add mpl token metadata to test validator setup

9f11892

rename test and add update test

dd1046e

gnapoli23 force-pushed the py-add-update-token-metadata branch from 04decaf to dd1046e Compare March 14, 2023 15:37

gnapoli23 marked this pull request as ready for review March 14, 2023 15:39

flake8 fix

91eab25

joncinque added this pull request to the merge queue Mar 14, 2023

Merged via the queue into solana-labs:master with commit 325f3d2 Mar 14, 2023

HaoranYi pushed a commit to HaoranYi/solana-program-library that referenced this pull request Jul 19, 2023

Support create and update token metadata in stake-pool python bin…

2d73e9a

…dings (solana-labs#3978)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `create` and `update` token metadata in stake-pool python bindings #3978

Support `create` and `update` token metadata in stake-pool python bindings #3978

gnapoli23 commented Jan 17, 2023 •

edited

Loading

joncinque left a comment

joncinque Jan 17, 2023

gnapoli23 Jan 17, 2023

joncinque Jan 17, 2023

gnapoli23 commented Jan 20, 2023 •

edited

Loading

joncinque commented Jan 24, 2023

gnapoli23 commented Jan 31, 2023 •

edited

Loading

joncinque left a comment

joncinque Feb 1, 2023

joncinque Feb 1, 2023

gnapoli23 Feb 1, 2023

joncinque Feb 1, 2023

joncinque commented Feb 1, 2023

gnapoli23 commented Feb 1, 2023 •

edited

Loading

joncinque left a comment

joncinque Feb 1, 2023

joncinque Feb 1, 2023

gnapoli23 Feb 2, 2023

gnapoli23 commented Feb 16, 2023 •

edited

Loading

joncinque left a comment

joncinque Feb 16, 2023

joncinque Feb 16, 2023

gnapoli23 commented Feb 23, 2023

joncinque commented Mar 8, 2023

gnapoli23 commented Mar 14, 2023 •

edited

Loading

joncinque commented Mar 14, 2023

	"""Public key that identifies the SPL Token Metadata program."""
	"""Public key that identifies the Metaplex Token Metadata program."""

Support create and update token metadata in stake-pool python bindings #3978

Support create and update token metadata in stake-pool python bindings #3978

Conversation

gnapoli23 commented Jan 17, 2023 • edited Loading

joncinque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnapoli23 commented Jan 20, 2023 • edited Loading

joncinque commented Jan 24, 2023

gnapoli23 commented Jan 31, 2023 • edited Loading

joncinque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joncinque commented Feb 1, 2023

gnapoli23 commented Feb 1, 2023 • edited Loading

joncinque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnapoli23 commented Feb 16, 2023 • edited Loading

joncinque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnapoli23 commented Feb 23, 2023

joncinque commented Mar 8, 2023

gnapoli23 commented Mar 14, 2023 • edited Loading

joncinque commented Mar 14, 2023

Support `create` and `update` token metadata in stake-pool python bindings #3978

Support `create` and `update` token metadata in stake-pool python bindings #3978

gnapoli23 commented Jan 17, 2023 •

edited

Loading

gnapoli23 commented Jan 20, 2023 •

edited

Loading

gnapoli23 commented Jan 31, 2023 •

edited

Loading

gnapoli23 commented Feb 1, 2023 •

edited

Loading

gnapoli23 commented Feb 16, 2023 •

edited

Loading

gnapoli23 commented Mar 14, 2023 •

edited

Loading