Adding Token Metadata Support to @solana/spl-token

[mistersimon]

Addresses #5229

With the addition of token metadata to Token2022 and the SPL Token Metadata interface itself to the SPL library, we need to support this cool feature & interface in JavaScript!

The goal here is to lay down a JavaScript library for SPL Token Metadata and then import it into @solana/spl-token for Token2022 metadata support.

This issue is importing and implementing @solana/spl-token-metadata.

[mistersimon]

Hey @buffalojoec, can't get the test case setup correct with the token metadata.

Thought I'd ask if you got any guidance

[buffalojoec]

What exactly is the issue with the test that you're having?

[mistersimon]

Hey @buffalojoec, cleaned up the PR and added the instructions to init, update and emit the token metadata

Wanted to check on the decoding of the Metadata before I continue to add the rest. It works, but I don't understand why and not sure if its correct. Please take a look at let me know

[buffalojoec]

This is moving along nicely! Let's iron out the MetadataPointer PR first and then rebase this one on top, then I can give this another look.

Thanks for the continued effort!

[mistersimon]

This is moving along nicely! Let's iron out the MetadataPointer PR first and then rebase this one on top, then I can give this another look.

Thanks for the continued effort!

Sounds like a good idea. Once the MetadataPointer PR is merged, I'll come back to this - rebase and address your comments. Then if you can take a look then before i go add the other instructions, would be really helpful.

[mistersimon]

Hey @buffalojoec - hit a bit of blocker when trying to implement the tokenMetadataUpdateFieldWithRentTransfer.

Wanted to see if there is some elegant way of calculating how much lamports is needed doing it before adding a bunch of code/logic to get it.

Take a look at the comment above

[mistersimon]

Hey @buffalojoec - think we should be close to wrapping this up.

Would appreciate your input on the ...WithRentTransfer actions. I tried to keep it as simple / understandable as possible, but its still quite a bit of logic

[buffalojoec]

I think I see some of your concerns around the rent calculation stuff.

If you take a look at the Rust implementation, there's two separate functions for calculating rent due.

solana-program-library/token/client/src/token.rs

Lines 3515 to 3530 in b49a7ce

	async fn get_additional_rent_for_new_metadata(
	&self,
	token_metadata: &TokenMetadata,
	) -> TokenResult<u64> {
	let account = self.get_account(self.pubkey).await?;
	let account_lamports = account.lamports;
	let mint_state = self.unpack_mint_info(account)?;
	let new_account_len = mint_state
	.try_get_new_account_len_for_variable_len_extension::<TokenMetadata>(token_metadata)?;
	let new_rent_exempt_minimum = self
	.client
	.get_minimum_balance_for_rent_exemption(new_account_len)
	.await
	.map_err(TokenError::Client)?;
	Ok(new_rent_exempt_minimum.saturating_sub(account_lamports))
	}

solana-program-library/token/client/src/token.rs

Lines 3595 to 3613 in b49a7ce

	async fn get_additional_rent_for_updated_metadata(
	&self,
	field: Field,
	value: String,
	) -> TokenResult<u64> {
	let account = self.get_account(self.pubkey).await?;
	let account_lamports = account.lamports;
	let mint_state = self.unpack_mint_info(account)?;
	let mut token_metadata = mint_state.get_variable_len_extension::<TokenMetadata>()?;
	token_metadata.update(field, value);
	let new_account_len = mint_state
	.try_get_new_account_len_for_variable_len_extension::<TokenMetadata>(&token_metadata)?;
	let new_rent_exempt_minimum = self
	.client
	.get_minimum_balance_for_rent_exemption(new_account_len)
	.await
	.map_err(TokenError::Client)?;
	Ok(new_rent_exempt_minimum.saturating_sub(account_lamports))
	}

They may not have to be fully separate functions (because they do share some functionality), but the key difference is that the "new" one is just creating a new TokenMetadata object, and asking for the rent calculation for that, while the "update" one is unpacking the existing, updating it, then asking for rent calculation.

I think we just have to mimic this behavior here. We should isolate these two cases, roll a function to calculate length of a new variable length extension, and follow what we're doing in Rust.

[mistersimon]

Thanks for the suggestion to split new/update - I think its much easier to follow now.

Still need that generic variable extension length handling. Would like to get your thoughts what you had in mind as i'm not sure if it can be so generic as to handle anything, or calls to separate functions per extension

[buffalojoec]

I tried to suggest some light refactoring to simplify the additional rent approach and align more with the Rust client.

Let's tighten this stuff down, and then I'll give it a good once-over and we can hopefully get this thing stamped!

I appreciate you keeping on it!

[buffalojoec]

Hey @mistersimon as usually I really appreciate the continued effort!

I wanted to reach out and see if you were interested in continuing this PR, or if you'd like me to take it over. I only ask because we may need this feature soon.

If I don't hear from you by the weekend I'll probably build off what you've got and wrap this thing up, not a problem! Again, thanks!

[mistersimon]

Hey @mistersimon as usually I really appreciate the continued effort!

I wanted to reach out and see if you were interested in continuing this PR, or if you'd like me to take it over. I only ask because we may need this feature soon.

If I don't hear from you by the weekend I'll probably build off what you've got and wrap this thing up, not a problem! Again, thanks!

Hey @buffalojoec - thanks for reaching out.

Yeah have been a bit busy with other stuff and haven't had as much time to work on this as I had hope. I have carved out some time this weekend though to finish up the outstanding issues.

But completely understand you all have your own schedule, and i have taken a while to finish this. No worries if you feel the need to take over - just let me know!

[buffalojoec]

Yeah have been a bit busy with other stuff and haven't had as much time to work on this as I had hope. I have carved out some time this weekend though to finish up the outstanding issues.

But completely understand you all have your own schedule, and i have taken a while to finish this. No worries if you feel the need to take over - just let me know!

If you've got the time and you want to give it another go this weekend I'm totally cool with that!

If at any point you decide you're too busy or otherwise don't want to keep hammering on this, just throw a flare up.

[mistersimon]

Hey @buffalojoec - think i've picked up all your edits. Feel free to take a look and let me know if theres anything still left to do.

I'll keep some time free next week to pick them up and get this closed out!

[buffalojoec]

Hey @mistersimon thanks for the awesome work!

This PR was just about ready to go, but in the interest of time I did the little bit of cleanup and also refactored the tests a bit.

[mistersimon]

Thanks @buffalojoec for getting this across the line and with all the help in reviews!

[rholang]

@mistersimon
Hi,
I am still getting "insufficient lamports" error, when executing the "create metadata" proposal on devnet (from params created). What could be the reason (there is more then enough SOL in the account)?
https://explorer.solana.com/tx/5nRBvH6zooKGZKWwxwYTab4ZEw12jh2y84VHUXQHXwvowf2MgPMUJWiC5GBRttwoqZJTpzXuG6tuULejTVv75Mzi?cluster=devnet