-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat: forester service #774
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great that the forester works!
Couple comments here and there.
photon-api/src/models/token_data.rs
Outdated
#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] | ||
pub struct TokenData { | ||
#[serde(rename = "amount")] | ||
pub amount: i32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amounts are ususally u64s, Nick told me they prefer i64s because those are natively supported in postgres, but i32 seems small and risky to me. Absolute amounts with many decimals can easily be in the billions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same with delegate amount, and is native
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah this is just copied from photon right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's autogenerated, but it should still be u64, I'll have a look https://github.com/helius-labs/photon/blob/main/src/common/typedefs/unsigned_integer.rs#L10
#[serde(rename = "dataHash")] | ||
pub data_hash: String, | ||
#[serde(rename = "discriminator")] | ||
pub discriminator: i32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discriminator for us is u64
} | ||
} | ||
|
||
fn account_nullified(&mut self, _merkle_tree_pubkey: Pubkey, _account_hash: &str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would be the use case for this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a readme.md
file to the forester? A brief high-level summary of the key functions and how to run it should be fine.
@@ -37,3 +39,4 @@ solana-rpc-client = { git = "https://github.com/Lightprotocol/agave", branch = " | |||
solana-rpc-client-api = { git = "https://github.com/Lightprotocol/agave", branch = "v1.18.11-enforce-cpi-tracking" } | |||
solana-runtime = { git = "https://github.com/Lightprotocol/agave", branch = "v1.18.11-enforce-cpi-tracking" } | |||
solana-sdk = { git = "https://github.com/Lightprotocol/agave", branch = "v1.18.11-enforce-cpi-tracking" } | |||
solana-transaction-status = { git = "https://github.com/Lightprotocol/agave", branch = "v1.18.11-enforce-cpi-tracking" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is switching back to the main agave repo contingent on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On this commit being released:
Without it, our program tests can't track inner instructions, so we wouldn't be able to test the event emission.
forester/forester.toml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do these signatures do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merkle_tree: address of the state merkle tree
nullifier_queue: address of the nullifier queue
payer: payer's keypair
(I added this info to the README.md)
54dd6bc
to
984cf31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lfg! looks good to me
054e2f1
to
014fc98
Compare
014fc98
to
dc59b33
Compare
e624d00
to
5b8e2e5
Compare
…ure enabled via programs/package.json
5b8e2e5
to
6cd33e8
Compare
6cd33e8
to
f8e3173
Compare
The timeout for the 'forester-tests' workflow in GitHub Actions has been extended from 30 minutes to 60 minutes, improving the chance of test completion for longer running tests.
No description provided.