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

Return parsed token accounts in simulateBundle #600

Open
wants to merge 120 commits into
base: master
Choose a base branch
from

Conversation

mcintyre94
Copy link
Contributor

@mcintyre94 mcintyre94 commented Mar 5, 2024

Rewrite of #592 on latest master

Problem

If you call simulateBundle and pass a token account in pre/postExecutionAccountsConfigs, then the token accounts are never returned in JSON. You will always get the base64 encoded data.

This is caused by attempting to encode the data without fetching the number of decimals for the mint, which causes a fallback to base64 data.

This was fixed for simulateTransaction upstream, but simulateBundle has the same issue.

Upstream issue (with more details of the code path causing this): solana-labs/solana#34693
Upstream fix to simulateTransaction: solana-labs/solana#34619

Summary of Changes

The changes are based on the upstream changes in simulateTransaction.

Instead of directly encoding the post-account data, we call get_encoded_account (added in the upstream PR), which will correctly parse a token account. As this function was written for transaction post-accounts it also takes overwrite_accounts: Option<&HashMap<Pubkey, AccountSharedData>>, so that account state can be overridden if a transaction has changed it.

In order to provide this, we now maintain a AccountOverrides object in rpc_bundle_result_from_bank_result. This is the same data structure, but has some useful helper functions and I think makes sense to use here. After each transaction, this is updated (using upsert_account_overrides) with the post_accounts from the transaction. This is then passed when we encode the post-accounts for that transaction.

The result is that rpc_bundle_result_from_bank_result (and the simulateBundle RPC call) now correctly JSON encodes token accounts, using data from the transactions, or the bank if required.

End-to-end test on localhost

  • Build this branch: cargo build

  • Create a directory /Users/username/scratch/solana-validator-tokens with the following files:

wallet.json
{
    "pubkey": "2hSzAdfBYSx3q9duv2VwxVoapcbHy14qaTfkPmHn4gbY",
    "account": {
      "data": [
        "",
        "base64"
      ],
      "executable": false,
      "lamports": 100000000,
      "owner": "11111111111111111111111111111111",
      "rentEpoch": 0
    }
  }
token-mint.json
{
    "pubkey": "Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr",
    "account": {
        "data": [
            "AQAAAOkoOVUJZf/U1krKr0bUXfcxjltPV8kMSH1gYl2Cm4N7iOvPHkm44BgGAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==",
            "base64"
        ],
        "executable": false,
        "lamports": 58203456600,
        "owner": "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA",
        "rentEpoch": 18446744073709551615,
        "space": 82
    }
}
receive-token.json
{
    "pubkey": "D9EYGf9etBGpPLHwiKMpxzn8gcHZapP6uvakAiCkdapK",
    "account": {
        "data": [
            "6Sg5VQll/9TWSsqvRtRd9zGOW09XyQxIfWBiXYKbg3vbLzrNfN5YZjAai4m3oURtvycuDlQ8wTPq3YIlZUPQ9KiqxS0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
            "base64"
        ],
        "executable": false,
        "lamports": 2039280,
        "owner": "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA",
        "rentEpoch": 18446744073709551615,
        "space": 165
    }
}
send-token.json
{
    "pubkey": "Dg5iatJR92aPAZ6E9kBCrdb2DDeaPoQdexjC48MrvNGf",
    "account": {
        "data": [
            "6Sg5VQll/9TWSsqvRtRd9zGOW09XyQxIfWBiXYKbg3sZOE0jrk6GS84ZX4OU7iFB1zRTqOg8ThrPzCdGFT/NlwDh9QUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
            "base64"
        ],
        "executable": false,
        "lamports": 2039280,
        "owner": "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA",
        "rentEpoch": 18446744073709551615,
        "space": 165
    }
}
  • Run the local validator: ./target/debug/solana-test-validator --account-dir /Users/username/scratch/solana-validator-tokens

  • Run the following curl command:

curl command
curl --location 'http://localhost:8899/' \
--header 'accept: application/json' \
--header 'content-type: application/json; charset=utf-8' \
--data '{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "simulateBundle",
    "params": [
        {
            "encodedTransactions": [
"AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAQHGThNI65OhkvOGV+DlO4hQdc0U6joPE4az8wnRhU/zZe0aDLGa0cy16lBWIf9ij+BU6eobQjtSBAP3ZRNOzSRTLxPOmj5LXZb8EVEBvpSsLG9P9kYCYqlowhFVOefwFCYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADpKDlVCWX/1NZKyq9G1F33MY5bT1fJDEh9YGJdgpuDewVKU1qZKSEGTSTocWDaOHx8NbXdvJK7geQfqEBBBUSNBt324ddloZPZy+FGzut5rBy0he1fWzeROoz1hX7/AKmOYFcfFz0Vu4XMA4jr8WltA1R3YRYSEipAl9FD4qy+FgMFAAY0RkhkVEwDAgAADAIAAAAAAAAAAAAAAAYEAgQBAAoMoIYBAAAAAAAG"
            ]
        },
        {
            "commitment": "confirmed",
            "encoding": "jsonParsed",
            "skipSigVerify": true,
            "replaceRecentBlockhash": true,
            "preExecutionAccountsConfigs": [{
                "addresses": [
                    "D9EYGf9etBGpPLHwiKMpxzn8gcHZapP6uvakAiCkdapK",
                    "Dg5iatJR92aPAZ6E9kBCrdb2DDeaPoQdexjC48MrvNGf"
                ],
                "encoding": "jsonParsed"
            }],
            "postExecutionAccountsConfigs": [{
                "addresses": [
                    "D9EYGf9etBGpPLHwiKMpxzn8gcHZapP6uvakAiCkdapK",
                    "Dg5iatJR92aPAZ6E9kBCrdb2DDeaPoQdexjC48MrvNGf"
                ],
                "encoding": "jsonParsed"
            }]
        }
    ]
}'
Or, just the JSON RPC body
{
    "id": 1,
    "jsonrpc": "2.0",
    "method": "simulateBundle",
    "params": [
        {
            "encodedTransactions": [
"AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABAAQHGThNI65OhkvOGV+DlO4hQdc0U6joPE4az8wnRhU/zZe0aDLGa0cy16lBWIf9ij+BU6eobQjtSBAP3ZRNOzSRTLxPOmj5LXZb8EVEBvpSsLG9P9kYCYqlowhFVOefwFCYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADpKDlVCWX/1NZKyq9G1F33MY5bT1fJDEh9YGJdgpuDewVKU1qZKSEGTSTocWDaOHx8NbXdvJK7geQfqEBBBUSNBt324ddloZPZy+FGzut5rBy0he1fWzeROoz1hX7/AKmOYFcfFz0Vu4XMA4jr8WltA1R3YRYSEipAl9FD4qy+FgMFAAY0RkhkVEwDAgAADAIAAAAAAAAAAAAAAAYEAgQBAAoMoIYBAAAAAAAG"
            ]
        },
        {
            "commitment": "confirmed",
            "encoding": "jsonParsed",
            "skipSigVerify": true,
            "replaceRecentBlockhash": true,
            "preExecutionAccountsConfigs": [{
                "addresses": [
                    "D9EYGf9etBGpPLHwiKMpxzn8gcHZapP6uvakAiCkdapK",
                    "Dg5iatJR92aPAZ6E9kBCrdb2DDeaPoQdexjC48MrvNGf"
                ],
                "encoding": "jsonParsed"
            }],
            "postExecutionAccountsConfigs": [{
                "addresses": [
                    "D9EYGf9etBGpPLHwiKMpxzn8gcHZapP6uvakAiCkdapK",
                    "Dg5iatJR92aPAZ6E9kBCrdb2DDeaPoQdexjC48MrvNGf"
                ],
                "encoding": "jsonParsed"
            }]
        }
    ]
}

You'll get back JSON parsed details for the token accounts, before and after.

If you do the same on the current master branch then you'll get base64 encoded token accounts.

Note that simulateTransaction works correctly because my equivalent upstream fix is already merged into Jito.

willhickey and others added 19 commits March 2, 2024 22:58
* Add GHA to close new issues with a comment

* Remove trailing whitespace
Add github action to close new pull requests with a comment directing them to agave
* ci: add upload-gcs-artifact

* ci: publish release binaries to GCS

* ci: redirect github repo to anza-xyz

* ci: publish windows binaries to GCS

* replace release.solana.com with release.anza.xyz

* use a explicit name for credential
* Update README.md

* ci: update CodeCov report link

* ci: update github pr link

* ci: rename secondary pipeline

* replace org name in .mergify

* update channel info link

* update dependabot pr link

* use anza docker image

* delete travis

---------

Co-authored-by: Will Hickey <[email protected]>
* ci: fix windows build

* ci: publish sdk docker image with the new name

* update automerge status
* rename geyser-plugin-interface

* rename cargo registry

* rename watchtower

* rename ledger tool

* rename validator

* rename install

* rename geyser plugin interface when patch
jito-foundation#69)

* Make inactive_stake consistent

* Add rpc_deprecated_v1_18 module

* Move get_stake_activation to deprecated list

* Fix typo
yihau and others added 10 commits March 7, 2024 12:18
When viewing in various tools such as gdb and perf, it is not easy to
distinguish which threads are serving which function (TPU or TPU FWD)
- solTvuFetchPmod ==> solTvuPktMod + solTvuRepPktMod
- solSigVerifier ==> solSigVerTpu + solSigVerTpuVot
* Simplify vote-authority display

* Add handling for new vote authority

* Add proper None handling, because unit test (shouldn't happen IRL, though)

* Unwrap->expect
buffalu pushed a commit that referenced this pull request Apr 13, 2024
… of #600) (#614)

* Show staked vs nonstaked packets sent down/throttled (#600)

* Show staked vs nonstaked packets sent down

* add metrics on throttled staked vs non-staked

(cherry picked from commit b443cfb)

# Conflicts:
#	streamer/src/quic.rs

* fix merge conflicts

---------

Co-authored-by: Lijun Wang <[email protected]>
Co-authored-by: HaoranYi <[email protected]>
buffalu pushed a commit that referenced this pull request Apr 13, 2024
… of #600) (#613)

* Show staked vs nonstaked packets sent down/throttled (#600)

* Show staked vs nonstaked packets sent down

* add metrics on throttled staked vs non-staked

(cherry picked from commit b443cfb)

# Conflicts:
#	streamer/src/quic.rs

* fix merge conflicts

---------

Co-authored-by: Lijun Wang <[email protected]>
Co-authored-by: HaoranYi <[email protected]>
@mcintyre94 mcintyre94 force-pushed the cm-bundle-jsonparsed-tokens-2 branch from 299676f to 0c8aca0 Compare May 8, 2024 13:25
rpc/src/rpc.rs Outdated
pre_tx_accounts,
account_encoding,
bank,
Some(&account_overrides),
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need to use account overrides here? you should have access to the accounts in the execution results

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It comes from the post accounts, but needs to be converted to the shape get_encoded_account expects, which is exactly your AccountOverrides type. Upstream doesn't have that type, but it's the same shape.

Are you suggesting writing a version of get_encoded_account that takes the Jito post_accounts: Vec<(Pubkey, AccountSharedData)> directly? I thought it'd be a smaller change to re-use get_encoded_account from upstream by reshaping the post_accounts to match it.

I think I'd also need to write upsert logic to combine the outputs of the transactions in the bundle since we encode the accounts after each transaction and not just at the end. I thought it was better to re-use AccountOverrides::upsert_account_overrides instead of writing custom logic for that.

buffalu pushed a commit that referenced this pull request Jul 18, 2024
* Show staked vs nonstaked packets sent down

* add metrics on throttled staked vs non-staked
@buffalu buffalu force-pushed the master branch 2 times, most recently from 7bc716a to b8605f2 Compare July 23, 2024 23:35
@buffalu buffalu force-pushed the master branch 4 times, most recently from 6a0aa5b to 8d53d7b Compare December 31, 2024 22:52
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.