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

disco/choreo: parsing, generating vote transactions; echoing gossip vote transactions #1925

Merged
merged 1 commit into from
May 30, 2024

Conversation

yhzhangjump
Copy link
Contributor

No description provided.

@yhzhangjump yhzhangjump force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from 13b31f3 to 18d06c3 Compare May 22, 2024 17:27
@yhzhangjump yhzhangjump requested a review from lidatong May 22, 2024 17:28
@yhzhangjump yhzhangjump force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from 18d06c3 to a1c17e3 Compare May 22, 2024 18:31
@yhzhangjump
Copy link
Contributor Author

The program for echoing vote transactions works. This PR is still in progress -- I will add a library in choreo for vote txn parsing/generating in this PR.

@yhzhangjump yhzhangjump changed the title disco: add a test program that echos vote transactions from gossip back to solana validator disco/choreo: parsing, generating vote transactions; echoing gossip vote transactions May 22, 2024
src/flamenco/gossip/fd_gossip.c Outdated Show resolved Hide resolved
src/choreo/vote/fd_vote.c Outdated Show resolved Hide resolved
+ FD_TXN_ACCT_ADDR_SZ * program_id );

if ( memcmp( program_account_addr, fd_solana_vote_program_id.key, sizeof( fd_pubkey_t ) ) ) {
FD_LOG_WARNING( ("fd_vote_txn_parse: txn targets program %32J instead of %32J",
Copy link
Contributor

Choose a reason for hiding this comment

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

%32J doesn't work anymore

Copy link
Contributor Author

@yhzhangjump yhzhangjump May 23, 2024

Choose a reason for hiding this comment

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

Ah, good to know. Are you suggesting that I should remove 32J here? Do we have an alternative for printing hash values then?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you really need it, use

#pragma GCC diagnostic ignored "-Wformat"
#pragma GCC diagnostic ignored "-Wformat-extra-args"

for debugging

Copy link
Contributor Author

@yhzhangjump yhzhangjump May 29, 2024

Choose a reason for hiding this comment

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

Right, I have them here: https://github.com/firedancer-io/firedancer/pull/1925/files#diff-c2df46d55326a4c2437845547d03a6cedd76bbbf8b568f6de7a0416efa60dd2fR3

Is this fine? Do you mean that I should avoid using "%32J" in real production (as opposed to debug)?

@yhzhangjump yhzhangjump force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from 05096b1 to 6374b8f Compare May 28, 2024 20:55
+ FD_TXN_ACCT_ADDR_SZ * program_id );

if ( memcmp( program_account_addr, fd_solana_vote_program_id.key, sizeof( fd_pubkey_t ) ) ) {
FD_LOG_WARNING( ("fd_vote_txn_parse: txn targets program %32J instead of %32J",
Copy link
Contributor

Choose a reason for hiding this comment

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

If you really need it, use

#pragma GCC diagnostic ignored "-Wformat"
#pragma GCC diagnostic ignored "-Wformat-extra-args"

for debugging

src/choreo/vote/fd_vote.c Outdated Show resolved Hide resolved
ushort vote_instr_size = (ushort)fd_vote_instruction_size( &vote_instr );

uchar instr_accounts[2];
instr_accounts[0] = 1; /* vote account */
Copy link
Contributor

Choose a reason for hiding this comment

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

both instr accounts are the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the case for local agave validator. Testnet does have 2 different accounts and I will work on adapting to testnet validator next.

Copy link
Contributor Author

@yhzhangjump yhzhangjump Jun 10, 2024

Choose a reason for hiding this comment

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

I know this PR is old, but I wish to let you know that your concern here has been addressed in my latest PR #2059. It turns out that there is a bug in solana-test-validator which causes their vote txn to have this issue.

lidatong
lidatong previously approved these changes May 29, 2024
arjain4
arjain4 previously approved these changes May 30, 2024
Copy link
Contributor

@arjain4 arjain4 left a comment

Choose a reason for hiding this comment

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

let's get this merged

lheeger-jump
lheeger-jump previously approved these changes May 30, 2024
@yhzhangjump yhzhangjump added this pull request to the merge queue May 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 30, 2024
@yhzhangjump yhzhangjump added this pull request to the merge queue May 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 30, 2024
@arjain4 arjain4 force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from c8cd498 to 82c67fa Compare May 30, 2024 15:57
@yhzhangjump yhzhangjump dismissed stale reviews from lheeger-jump and arjain4 via 0d9969c May 30, 2024 16:00
@yhzhangjump yhzhangjump force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from 82c67fa to 0d9969c Compare May 30, 2024 16:00
@arjain4 arjain4 force-pushed the yunzhang/feat/gossip-echo-vote-txns branch from 0d9969c to 5ef9bdd Compare May 30, 2024 16:00
@arjain4 arjain4 added this pull request to the merge queue May 30, 2024
Merged via the queue into main with commit 998a6f3 May 30, 2024
9 checks passed
@arjain4 arjain4 deleted the yunzhang/feat/gossip-echo-vote-txns branch May 30, 2024 16:42
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.

5 participants