Skip to content

Conversation

@melotik
Copy link
Contributor

@melotik melotik commented Jan 19, 2023

New substream to get SPL token balances on solana.

Deliverables:

  • Substream module following sol_token.proto to get the balances of all spl tokens on solana

Model off: https://github.com/streamingfast/substreams-playground/tree/master/modules/sol-spl-tokens

@melotik melotik requested a review from robinbernon February 3, 2023 19:42
@@ -0,0 +1 @@
// DO NOT EDIT - the file is generated by build script
Copy link
Contributor

Choose a reason for hiding this comment

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

rm

@@ -0,0 +1,4 @@
[toolchain]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a fan of local toolchains for each project. Would be best to have these settings configured globally for all projects by default if possible

@@ -0,0 +1,19 @@
#[rustfmt::skip]
#[path = "../target/pb/messari.common.v1.rs"]
pub(in crate::pb) mod common_v1;
Copy link
Contributor

Choose a reason for hiding this comment

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

proto files aren't being used so we could remove them from the substream for now?

@@ -1,7 +1,8 @@
use substreams::{log, store};
use substreams::pb;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same deal here


protobuf:
files:
- test.proto
Copy link
Contributor

Choose a reason for hiding this comment

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

rm?

@@ -0,0 +1 @@
// DO NOT EDIT - the file is generated by build script
Copy link
Contributor

Choose a reason for hiding this comment

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

rm

@@ -0,0 +1,4 @@
[toolchain]
Copy link
Contributor

Choose a reason for hiding this comment

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

rm if possible

use substreams::store::{StoreGet, StoreGetProto, StoreNew, StoreSet, StoreSetProto};
use substreams_solana::pb::sol as solana;

#[substreams::handlers::map]
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you could move this to a modules folder like the other substreams are now doing?

type: wasm/rust-v1
file: "../target/wasm32-unknown-unknown/release/spl_balance.wasm"

modules:
Copy link
Contributor

Choose a reason for hiding this comment

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

Think these modules are back to front?

output.set(
0,
format!(
"address:{}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use a storekey struct or a small fn that you can reuse between modules

/// A C representation of Rust's `std::option::Option`
#[repr(C)]
#[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
pub enum COption<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole file can be removed and you can switch back to using std::Option instead

/// Instructions supported by the token program.
#[repr(C)]
#[derive(Clone, Debug, PartialEq)]
pub enum TokenInstruction<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shrink this enum down to having just three fields:

  • InitializeMint
  • InitializeMint2
  • NonMintInstruction

You will also be able to remove all C api code and a bunch of the map code

/// Specifies the authority type for SetAuthority instructions
#[repr(u8)]
#[derive(Clone, Debug, PartialEq)]
pub enum AuthorityType {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this also

name: "".to_string(),
symbol: "".to_string(),
decimals: decimal.into(),
freeze_authority: "".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

You could have this all on one line using .map() and then .unwrap_or_default(). This would then make it simple enough to doing the instantiation back inside the main module code rather than this fn here.

) -> proto::TokenAccount {
let mut token = proto::TokenAccount {
address: bs58::encode(&token_address).into_string(),
name: "".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Think all protos inherit the default trait automatically so you might be able to get away with ..default at the bottom rather than manually spefying these fields during the instantiation

return token;
}

// pull the TokenAccount using the key provided
Copy link
Contributor

Choose a reason for hiding this comment

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

What are your plans for this code block?

@@ -0,0 +1,9 @@
#[rustfmt::skip]
#[path = "../target/pb/messari.sol_token.v1.rs"]
pub(in crate::pb) mod sol_token_v1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting this here for the future but what we should do here is filter out all protos which are from substream-helpers in the build.rs script and not build bindings for them. Then we can just import the bindings from the substreams-helper crate instead which will avoid the unnecessary duplication of code here

version = "0.1.0"
description = "Messari's standardized substream for solana SPL tokens"
edition = "2021"
repository = "https://github.com/messari/substreams"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really that important but it would be nice to have all these links be consistant. (If you use the CLI to create the substream projects it will do this for you :) )

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