get_signatures_for_address does not correctly account for result sets that span local and Bigtable sources by omarkilani · Pull Request #22115 · solana-labs/solana

omarkilani · 2021-12-25T22:26:36Z

Problem

When an account has tx history both in the Blockstore and Bigtable, but the tx hasn't been uploaded to Bigtable yet, there is no way for Bigtable to know which txs precede the tx passed in as before.

This causes Bigtable to return RowNotFound until the new tx is uploaded.

Proposed Solution

Check that before exists in Bigtable, and if not, set it to None to return the full data set.

References #21442
Fixes #22110

codecov · 2021-12-26T00:23:54Z

Codecov Report

Merging #22115 (e924368) into master (57986f9) will decrease coverage by 0.0%.
The diff coverage is 57.7%.

@@            Coverage Diff            @@
##           master   #22115     +/-   ##
=========================================
- Coverage    81.1%    81.1%   -0.1%     
=========================================
  Files         521      521             
  Lines      146196   146233     +37     
=========================================
+ Hits       118673   118685     +12     
- Misses      27523    27548     +25

omarkilani · 2021-12-26T23:32:32Z

Improved the commit and tested all 3 scenarios.

In this case, I generated a new tx 5qrr8KSCZ7Ro9TABv7nsFKyjDsrboTKfoDZyBrCzs3D8SABQGUfFj1pFvCsQxgz8cpdfXMhb8jxMm8JUkJb1J8Fs.

Bigtable had an older tx, 5Wkc3CD41maw6qytgKAbgJcjrA9ZdWVLb5pe63dpXjE9mH9JBLTVEPBbn2XCE8Q5fvfK62RAF8KtcjKYYrzWHHPP.

When the new tx was generated, Solscan and Explorer both "lost" the old tx for around 30 minutes.

This RPC output is from the patched validator.

Case where only Bigtable contains tx history:

{"jsonrpc":"2.0","result":[{"blockTime":1633010555,"confirmationStatus":"finalized","err":null,"memo":null,"signature":"5Wkc3CD41maw6qytgKAbgJcjrA9ZdWVLb5pe63dpXjE9mH9JBLTVEPBbn2XCE8Q5fvfK62RAF8KtcjKYYrzWHHPP","slot":99095027}],"id":1}

Case where both local storage and Bigtable contain non-overlapping sets of tx history:

{"jsonrpc":"2.0","result":[{"blockTime":1640558167,"confirmationStatus":"finalized","err":null,"memo":null,"signature":"5qrr8KSCZ7Ro9TABv7nsFKyjDsrboTKfoDZyBrCzs3D8SABQGUfFj1pFvCsQxgz8cpdfXMhb8jxMm8JUkJb1J8Fs","slot":113464571},{"blockTime":1633010555,"confirmationStatus":"finalized","err":null,"memo":null,"signature":"5Wkc3CD41maw6qytgKAbgJcjrA9ZdWVLb5pe63dpXjE9mH9JBLTVEPBbn2XCE8Q5fvfK62RAF8KtcjKYYrzWHHPP","slot":99095027}],"id":1}

Case where local storage and Bigtable have overlapping sets of tx history:

{"jsonrpc":"2.0","result":[{"blockTime":1640558167,"confirmationStatus":"finalized","err":null,"memo":null,"signature":"5qrr8KSCZ7Ro9TABv7nsFKyjDsrboTKfoDZyBrCzs3D8SABQGUfFj1pFvCsQxgz8cpdfXMhb8jxMm8JUkJb1J8Fs","slot":113464571},{"blockTime":1633010555,"confirmationStatus":"finalized","err":null,"memo":null,"signature":"5Wkc3CD41maw6qytgKAbgJcjrA9ZdWVLb5pe63dpXjE9mH9JBLTVEPBbn2XCE8Q5fvfK62RAF8KtcjKYYrzWHHPP","slot":99095027}],"id":1}

CC @CriesofCarrots

CriesofCarrots

Aha, nice. Thank you for tracking this down!

However, I don't think this is quite right.
The current fix could still hit the RowNotFound error when the getSignaturesForAddress RPC is called with a before parameter that is found in Blockstore but is not yet written to BigTable.

Also, when handling a getSignaturesForAddress RPC request without a specified before, this fix could cause increased (and unnecessary) latency when there are a bunch of signatures in Blockstore that have already been written to BigTable. This could be painful for a very active account.

See my code suggestion to reset before to None only when some last signature is found in Blockstore but not BigTable.

omarkilani · 2021-12-28T00:32:59Z

@CriesofCarrots okay, will test your patch soon.

Please feel free to just do your own commit / PR -- I mainly just wanted to highlight the issue and have a hot-fix I can use.

(Sorry about the newb-ness of my patch... first time looking at the validator.😅)

omarkilani · 2021-12-28T12:40:47Z

@CriesofCarrots updated the patch. I'm not sure it's working as expected with before.

Patch:

diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs
index 2e2becd03..e9cc0f655 100644
--- a/rpc/src/rpc.rs
+++ b/rpc/src/rpc.rs
@@ -1452,6 +1452,16 @@ impl JsonRpcRequestProcessor {
                     if !results.is_empty() {
                         limit -= results.len();
                         before = results.last().map(|x| x.signature);
+
+                        if bigtable_ledger_storage
+                            .get_confirmed_transaction(&before.unwrap())
+                            .await
+                            .ok()
+                            .flatten()
+                            .is_none()
+                        {
+                            before = None
+                        }
                     }
 
                     let bigtable_results = bigtable_ledger_storage

Before creating the new tx:

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "2zBmbRikCMiweJiYB8Rc56VVpPBoZqcAxQ9GjeQYk6jy"
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1633010563,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K",
      "slot": 99095043
    }
  ],
  "id": 1
}

After creating the new tx, now y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W is in Blockstore and 5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K is in Bigtable:

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "2zBmbRikCMiweJiYB8Rc56VVpPBoZqcAxQ9GjeQYk6jy"
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1640694557,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W",
      "slot": 113708452
    },
    {
      "blockTime": 1633010563,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K",
      "slot": 99095043
    }
  ],
  "id": 1
}

So far so good, but:

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
   {       
     "jsonrpc": "2.0",
     "id": 1,
     "method": "getSignaturesForAddress",
     "params": [                         
       "2zBmbRikCMiweJiYB8Rc56VVpPBoZqcAxQ9GjeQYk6jy",
       {"before":"y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W"}
     ]                                               
   }
 '

{"jsonrpc":"2.0","result":[],"id":1}

Once y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W is in Bigtable:

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "2zBmbRikCMiweJiYB8Rc56VVpPBoZqcAxQ9GjeQYk6jy",
      {"before":"y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W"}
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1633010563,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K",
      "slot": 99095043
    }
  ],
  "id": 1
}

Hmmm.

omarkilani · 2021-12-28T13:53:23Z

In the case where the result set looks like:

[
    {
      "blockTime": 1640694557,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W",
      "slot": 113708452
    },
    {
      "blockTime": 1633010563,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K",
      "slot": 99095043
    }
  ]

Where y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W is in Blockstore, and 5LpuTUS5PY5C1mynuMz1VJdqSNcPDUXBFNUytVVSweXSqgDkyrC1MWqrPLHssYsHj1TcdiWUVaiWrMhAsc3pDf9K is in Bigtable.

A get_signatures_for_address(XXX, before="y6F6mpb9UG131aKnpj12G2hnYq5ibUxXuSBJgsUL7aGANN9oA9ddjccJASBzjBKyhxmprejaLoixLwPzDD6pY5W") will return an empty result set because Bigtable doesn't know that the tx is associated with the account until the tx is uploaded to Bigtable.

omarkilani · 2021-12-28T14:43:10Z

Updated the patch to restrict the before passed to Bigtable to the input before instead of None.

It works for the cases where:

Data is only in Blockstore.
Data is only in Bigtable.
Data spans both with no overlaps.
Data spans both with overlaps.

However before filtering across stores is broken as described in #22115 (comment)

CriesofCarrots · 2021-12-28T17:17:28Z

@CriesofCarrots okay, will test your patch soon.

Please feel free to just do your own commit / PR -- I mainly just wanted to highlight the issue and have a hot-fix I can use.

(Sorry about the newb-ness of my patch... first time looking at the validator.😅)

Ah, okay. I was sending a "convey an idea" back at you :)
I'll do some testing and push to your branch later today.

The gist of my suggestion is that when BigTable receives before: None, it should return a set starting with the earliest signatures in BigTable, which is what we want if the last signature from Blockstore can't be found in BigTable.
I'm not sure why you were seeing an empty set here (#22115 (comment)), so I'm going to inspect that more closely.

omarkilani · 2021-12-28T20:40:13Z

Yeah, that makes sense. So you view Bigtable as a "continuation" of the Blockstore, and I'm naively looking at them as two distinct things, maybe? lol.

I always kind of go back to these two screenshots where... for some reason, the validator was returning a tx from the middle of the set (???) and then once the "event" passed it returned tx on either side of that tx.

I'll go back to the before = None code and instrument with logs to see what's going on, also.

omarkilani · 2021-12-28T22:05:34Z

Okay so the issue is the Bigtable get_confirmed_transaction check needs to happen regardless of if Blockstore returns a result or not. Otherwise:

[2021-12-28T21:49:15.000179633Z WARN  solana_rpc::rpc] get_signatures_for_address(DxqbceSbERMVJL3wWt1BEmmnxarBTQPmniLqhjLGTDrT)[1]: highest_slot 113763995, before Some(51wfz8aWsjGHrwj4MVEzw5bM2kqHTSZXFEXLPAnXtTTN8moercsKAwuyTs7WJMeRSWRCQCPCHnTJmCVScYKijZNn), until None, limit 1000
[2021-12-28T21:49:15.041292145Z WARN  solana_rpc::rpc] get_signatures_for_address(DxqbceSbERMVJL3wWt1BEmmnxarBTQPmniLqhjLGTDrT)[2]: results []
[2021-12-28T21:49:15.049681508Z WARN  solana_rpc::rpc] BigTableError(RowNotFound)

This is what I was trying to say above.

But then anything passed in to before that Bigtable hasn't seen would trigger a full tx history return?

omarkilani · 2021-12-29T00:28:07Z

Okay, the latest patch looks like it works in all cases.

Data only in Bigtable:

curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK"
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1634354482,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6",
      "slot": 101745666
    }
  ],
  "id": 1
}

Validator output:

[2021-12-29T00:08:38.466103985Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[1]: highest_slot 113778649, before None, until None, limit 1000
[2021-12-29T00:08:38.466897226Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[2]: results []
[2021-12-29T00:08:38.483379956Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[5]: bigtable_results [(ConfirmedTransactionStatusWithSignature { signature: 3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6, slot: 101745666, err: None, memo: None, block_time: Some(1634354482) }, 1198)]

At this point, I generate a new tx by moving the token.

Data spanning Blockstore and Bigtable (no overlap):

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK"
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1640736577,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB",
      "slot": 113778790
    },
    {
      "blockTime": 1634354482,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6",
      "slot": 101745666
    }
  ],
  "id": 1
}

Validator output:

[2021-12-29T00:10:06.056810858Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[1]: highest_slot 113778806, before None, until None, limit 1000
[2021-12-29T00:10:06.057613648Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[2]: results [ConfirmedTransactionStatusWithSignature { signature: cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB, slot: 113778790, err: None, memo: None, block_time: Some(1640736577) }]
[2021-12-29T00:10:06.066416052Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[3]: limit 999, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), bigtable_confirmed_tx Err(SignatureNotFound)
[2021-12-29T00:10:06.066442945Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[4]: before None
[2021-12-29T00:10:06.084344213Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[5]: bigtable_results [(ConfirmedTransactionStatusWithSignature { signature: 3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6, slot: 101745666, err: None, memo: None, block_time: Some(1634354482) }, 1198)]

Data spanning Blockstore and Bigtable (with tx overlap -- i.e. tx is present in both stores):

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK"
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1640736577,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB",
      "slot": 113778790
    },
    {
      "blockTime": 1634354482,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6",
      "slot": 101745666
    }
  ],
  "id": 1
}

Validator output:

[2021-12-29T00:13:45.587281216Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[1]: highest_slot 113779171, before None, until None, limit 1000
[2021-12-29T00:13:45.587674340Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[2]: results [ConfirmedTransactionStatusWithSignature { signature: cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB, slot: 113778790, err: None, memo: None, block_time: Some(1640736577) }]
[2021-12-29T00:13:45.621499103Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[3]: limit 999, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), bigtable_confirmed_tx Ok(Some(ConfirmedTransaction { slot: 113778790, transaction: TransactionWithStatusMeta { transaction: Transaction { signatures: [cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB], message: Message { header: MessageHeader { num_required_signatures: 1, num_readonly_signed_accounts: 0, num_readonly_unsigned_accounts: 7 }, account_keys: [68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty, CiNXU82JXd1FcK2TeZSZTPPsG6YvEoXL54SbovoGjSpt, CE5QG1hWPHEFvikBYRxzNQeJvXjhDG4h1Heb45H2r8gF, 9XhTddruTCvwsFELyXEaMoQGM4T9UyC9hWUABpWvAHTh, 5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK, 11111111111111111111111111111111, TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA, SysvarRent111111111111111111111111111111111, DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ, ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL], recent_blockhash: D7umKzWspnj4AkeuVPBMCnhT973p6b7zRc26dkkhr3oA, instructions: [CompiledInstruction { program_id_index: 8, accounts: [3], data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 9, accounts: [0, 1, 3, 4, 5, 6, 7], data: [] }, CompiledInstruction { program_id_index: 6, accounts: [2, 4, 1, 0], data: [12, 1, 0, 0, 0, 0, 0, 0, 0, 0] }] } }, meta: Some(TransactionStatusMeta { status: Ok(()), fee: 5000, pre_balances: [15237239, 0, 2039280, 6765960, 1461600, 1, 1089991680, 1009200, 1141440, 898174080], post_balances: [13192959, 2039280, 2039280, 6765960, 1461600, 1, 1089991680, 1009200, 1141440, 898174080], inner_instructions: Some([InnerInstructions { index: 1, instructions: [CompiledInstruction { program_id_index: 5, accounts: [0, 1], data: [2, 0, 0, 0, 240, 29, 31, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 5, accounts: [1], data: [8, 0, 0, 0, 165, 0, 0, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 5, accounts: [1], data: [1, 0, 0, 0, 6, 221, 246, 225, 215, 101, 161, 147, 217, 203, 225, 70, 206, 235, 121, 172, 28, 180, 133, 237, 95, 91, 55, 145, 58, 140, 245, 133, 126, 255, 0, 169] }, CompiledInstruction { program_id_index: 6, accounts: [1, 4, 3, 7], data: [1] }] }]), log_messages: Some(["Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ invoke [1]", "Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ consumed 578 of 200000 compute units", "Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ success", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL invoke [1]", "Program log: Transfer 2039280 lamports to the associated token account", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Allocate space for the associated token account", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Assign the associated token account to the SPL Token program", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Initialize the associated token account", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA invoke [2]", "Program log: Instruction: InitializeAccount", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA consumed 3449 of 179468 compute units", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA success", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL consumed 24662 of 200000 compute units", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL success", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA invoke [1]", "Program log: Instruction: TransferChecked", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA consumed 3772 of 200000 compute units", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA success"]), pre_token_balances: Some([TransactionTokenBalance { account_index: 2, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: Some(1.0), decimals: 0, amount: "1", ui_amount_string: "1" }, owner: "68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty" }]), post_token_balances: Some([TransactionTokenBalance { account_index: 1, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: Some(1.0), decimals: 0, amount: "1", ui_amount_string: "1" }, owner: "9XhTddruTCvwsFELyXEaMoQGM4T9UyC9hWUABpWvAHTh" }, TransactionTokenBalance { account_index: 2, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: None, decimals: 0, amount: "0", ui_amount_string: "0" }, owner: "68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty" }]), rewards: Some([]) }) }, block_time: Some(1640736577) }))
[2021-12-29T00:13:45.647038803Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[5]: bigtable_results [(ConfirmedTransactionStatusWithSignature { signature: 3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6, slot: 101745666, err: None, memo: None, block_time: Some(1634354482) }, 1198)]

With before filter spanning Blockstore and Bigtable (no overlap):

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK",
      {"before":"cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB"}
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1634354482,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6",
      "slot": 101745666
    }
  ],
  "id": 1
}

Validator output:

[2021-12-29T00:10:33.168672634Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[1]: highest_slot 113778845, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), until None, limit 1000
[2021-12-29T00:10:33.184584036Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[2]: results []
[2021-12-29T00:10:33.193288965Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[3]: limit 1000, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), bigtable_confirmed_tx Err(SignatureNotFound)
[2021-12-29T00:10:33.193326850Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[4]: before None
[2021-12-29T00:10:33.213047844Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[5]: bigtable_results [(ConfirmedTransactionStatusWithSignature { signature: 3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6, slot: 101745666, err: None, memo: None, block_time: Some(1634354482) }, 1198)]

With before filter spanning Blockstore and Bigtable (with tx overlap):

$ curl http://10.0.0.5:8899/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK",
      {"before":"cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB"}
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1634354482,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6",
      "slot": 101745666
    }
  ],
  "id": 1
}

Validator output:

[2021-12-29T00:13:03.872311266Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[1]: highest_slot 113779105, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), until None, limit 1000
[2021-12-29T00:13:03.892874349Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[2]: results []
[2021-12-29T00:13:03.925933395Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[3]: limit 1000, before Some(cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB), bigtable_confirmed_tx Ok(Some(ConfirmedTransaction { slot: 113778790, transaction: TransactionWithStatusMeta { transaction: Transaction { signatures: [cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB], message: Message { header: MessageHeader { num_required_signatures: 1, num_readonly_signed_accounts: 0, num_readonly_unsigned_accounts: 7 }, account_keys: [68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty, CiNXU82JXd1FcK2TeZSZTPPsG6YvEoXL54SbovoGjSpt, CE5QG1hWPHEFvikBYRxzNQeJvXjhDG4h1Heb45H2r8gF, 9XhTddruTCvwsFELyXEaMoQGM4T9UyC9hWUABpWvAHTh, 5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK, 11111111111111111111111111111111, TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA, SysvarRent111111111111111111111111111111111, DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ, ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL], recent_blockhash: D7umKzWspnj4AkeuVPBMCnhT973p6b7zRc26dkkhr3oA, instructions: [CompiledInstruction { program_id_index: 8, accounts: [3], data: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 9, accounts: [0, 1, 3, 4, 5, 6, 7], data: [] }, CompiledInstruction { program_id_index: 6, accounts: [2, 4, 1, 0], data: [12, 1, 0, 0, 0, 0, 0, 0, 0, 0] }] } }, meta: Some(TransactionStatusMeta { status: Ok(()), fee: 5000, pre_balances: [15237239, 0, 2039280, 6765960, 1461600, 1, 1089991680, 1009200, 1141440, 898174080], post_balances: [13192959, 2039280, 2039280, 6765960, 1461600, 1, 1089991680, 1009200, 1141440, 898174080], inner_instructions: Some([InnerInstructions { index: 1, instructions: [CompiledInstruction { program_id_index: 5, accounts: [0, 1], data: [2, 0, 0, 0, 240, 29, 31, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 5, accounts: [1], data: [8, 0, 0, 0, 165, 0, 0, 0, 0, 0, 0, 0] }, CompiledInstruction { program_id_index: 5, accounts: [1], data: [1, 0, 0, 0, 6, 221, 246, 225, 215, 101, 161, 147, 217, 203, 225, 70, 206, 235, 121, 172, 28, 180, 133, 237, 95, 91, 55, 145, 58, 140, 245, 133, 126, 255, 0, 169] }, CompiledInstruction { program_id_index: 6, accounts: [1, 4, 3, 7], data: [1] }] }]), log_messages: Some(["Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ invoke [1]", "Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ consumed 578 of 200000 compute units", "Program DeJBGdMFa1uynnnKiwrVioatTuHmNLpyFKnmB5kaFdzQ success", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL invoke [1]", "Program log: Transfer 2039280 lamports to the associated token account", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Allocate space for the associated token account", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Assign the associated token account to the SPL Token program", "Program 11111111111111111111111111111111 invoke [2]", "Program 11111111111111111111111111111111 success", "Program log: Initialize the associated token account", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA invoke [2]", "Program log: Instruction: InitializeAccount", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA consumed 3449 of 179468 compute units", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA success", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL consumed 24662 of 200000 compute units", "Program ATokenGPvbdGVxr1b2hvZbsiqW5xWH25efTNsLJA8knL success", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA invoke [1]", "Program log: Instruction: TransferChecked", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA consumed 3772 of 200000 compute units", "Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA success"]), pre_token_balances: Some([TransactionTokenBalance { account_index: 2, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: Some(1.0), decimals: 0, amount: "1", ui_amount_string: "1" }, owner: "68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty" }]), post_token_balances: Some([TransactionTokenBalance { account_index: 1, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: Some(1.0), decimals: 0, amount: "1", ui_amount_string: "1" }, owner: "9XhTddruTCvwsFELyXEaMoQGM4T9UyC9hWUABpWvAHTh" }, TransactionTokenBalance { account_index: 2, mint: "5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK", ui_token_amount: UiTokenAmount { ui_amount: None, decimals: 0, amount: "0", ui_amount_string: "0" }, owner: "68gtDKNNGoFvYzARR8D9C3t8pSZvJ2rsXnwyr5fYP2ty" }]), rewards: Some([]) }) }, block_time: Some(1640736577) }))
[2021-12-29T00:13:03.957011224Z WARN  solana_rpc::rpc] get_signatures_for_address(5aahDZL5ew1YAUF2HciSc6wat38jxiXoL7HMUdd19snK)[5]: bigtable_results [(ConfirmedTransactionStatusWithSignature { signature: 3CqzepYgiV1kWxrVx7cDVwBkzUH5eiryMdTLTF2qxdZdgSGEvvyT6E3FLYhr58LAkbgR2VhvT9Py14bzaGRwb8K6, slot: 101745666, err: None, memo: None, block_time: Some(1634354482) }, 1198)]

CriesofCarrots · 2021-12-29T03:49:01Z

Okay, the latest patch looks like it works in all cases.

Almost, but not quite. In the case of a query for a non-existent --before <SIG>, this patch will return all the signatures in BigTable. I think an empty set is more correct in that case.
Also, if the before signature in Blockstore happens to be written to Bigtable between line 1477 and 1486, that signature will be included in the result erroneously. So I do think we still need some duplicate checking. I will push a couple commits shortly.

… that span Blockstore and Bigtable. This causes Bigtable to return `RowNotFound` until the new tx is uploaded. Check that `before` exists in Bigtable, and if not, set it to `None` to return the full data set. References #21442 Closes #22110

omarkilani · 2021-12-29T04:03:08Z

Okay, the latest patch looks like it works in all cases.

Almost, but not quite. In the case of a query for a non-existent --before <SIG>, this patch will return all the signatures in BigTable. I think an empty set is more correct in that case. Also, if the before signature in Blockstore happens to be written to Bigtable between line 1477 and 1486, that signature will be included in the result erroneously. So I do think we still need some duplicate checking. I will push a couple commits shortly.

Ahh... yeah, I did have that Q above:

But then anything passed in to before that Bigtable hasn't seen would trigger a full tx history return?

lol. And yes to dupe checks. :)

CriesofCarrots · 2021-12-29T06:32:20Z

Okay, I basically combined all the bits of your various pushes, plus a new flag from Blockstore. This now handles all the edge cases I came up with. Do let me know if you think of any more 🙏
Thanks again for spotting the original issue, and for all your work on this @omarkilani !

omarkilani · 2021-12-29T12:26:27Z

That's amazing -- team work! :)

Tested all the different corner cases. There is (optimistically? :) one left, unfortunately.

(I'm not sure how you want to solve it in the Tyera-approved way so I'll just describe it. :)

My test case was this:

curl http://URL-TO-PATCHED-VALIDATOR/ -X POST -H "Content-Type: application/json" -d '
  {       
    "jsonrpc": "2.0",
    "id": 1,
    "method": "getSignaturesForAddress",
    "params": [                         
      "DxqbceSbERMVJL3wWt1BEmmnxarBTQPmniLqhjLGTDrT",
      {"before":"cQBk5Dittvn43vH4zjkgHBkLvc5BB9UHsuP7znyGUsw9P6f2wgXVH2GHjm67brfC9WZvwbpYe8L24d2NPnRqTnB"}
    ]                                               
  }
'

{
  "jsonrpc": "2.0",
  "result": [
    {
      "blockTime": 1640728079,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "51wfz8aWsjGHrwj4MVEzw5bM2kqHTSZXFEXLPAnXtTTN8moercsKAwuyTs7WJMeRSWRCQCPCHnTJmCVScYKijZNn",
      "slot": 113763906
    },
    {
      "blockTime": 1634925339,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5ed325MzMDrmvdbmaUYpeH1q9WJsTi4JtBqbBY8P65eycFNdkD5Suz4PBTRJJu23Y3sWh3QY3wRpUhEmiMeXJfC9",
      "slot": 102935564
    },
    {
      "blockTime": 1634167190,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "cFsHu3zYzHb9WRxERQYQ7MghTmkz4YK7EVhjdt1q8WLnYnCpLqTFp1fDcjijiiWDJtQSJWmqzjaTBj65PD5zoVK",
      "slot": 101350044
    },
    {
      "blockTime": 1634158823,
      "confirmationStatus": "finalized",
      "err": null,
      "memo": null,
      "signature": "5JNaV3djTAPZDcvsiw6vGUPYDvCo5197qAdY9rueE6JTcDge4KnMbd1Q7abLiV3o9Yem5jrEPNvdxe74FeWyxCJo",
      "slot": 101332261
    }
  ],
  "id": 1
}

This shouldn't return anything since the cQBk... tx is not associated with DxqbceSbERMVJL3wWt1BEmmnxarBTQPmniLqhjLGTDrT.

However it returned the full data set for the address instead.

I may be... missing something (?)... but in

get_confirmed_signatures_for_address2

before and until don't seem to be getting checked for address association?

CriesofCarrots · 2021-12-29T17:00:43Z

This shouldn't return anything since the cQBk... tx is not associated with DxqbceSbERMVJL3wWt1BEmmnxarBTQPmniLqhjLGTDrT.

This one may be working as expected. It is intended that a client can use a before signature that doesn't involve the specified address. The "other" signature is just a marker in time. In what slot was cQBk... processed? If cQBk... slot >=113763906, then I think all is well with this example.

Either way, thanks for the reminder. I'll do a focused testing of non-associated before signatures today.

omarkilani · 2021-12-29T17:05:45Z

Ah. That actually is a nice feature. Didn't quite make sense at 3am but makes sense now. :)

Okay, then I think... this is (hopefully) good.

(I've been running production stuff against the patched validator for ~4 hours and don't notice any perf degradation.)

CriesofCarrots · 2021-12-29T17:24:57Z

Non-associated before tests all check out. We can ofc iterate if more issue crop up, but for now... 🚢
Thanks @omarkilani !

… that span local and Bigtable sources (#22115) * get_signatures_for_address does not correctly account for result sets that span Blockstore and Bigtable. This causes Bigtable to return `RowNotFound` until the new tx is uploaded. Check that `before` exists in Bigtable, and if not, set it to `None` to return the full data set. References #21442 Closes #22110 * Differentiate between before sig not found and no newer signatures * Dedupe bigtable results to account for potential upload race Co-authored-by: Tyera Eulberg <tyera@solana.com> (cherry picked from commit bac6821) # Conflicts: # ledger/src/blockstore.rs

… that span local and Bigtable sources (#22115) * get_signatures_for_address does not correctly account for result sets that span Blockstore and Bigtable. This causes Bigtable to return `RowNotFound` until the new tx is uploaded. Check that `before` exists in Bigtable, and if not, set it to `None` to return the full data set. References #21442 Closes #22110 * Differentiate between before sig not found and no newer signatures * Dedupe bigtable results to account for potential upload race Co-authored-by: Tyera Eulberg <tyera@solana.com> (cherry picked from commit bac6821)

omarkilani · 2021-12-29T17:37:50Z

Thank you @CriesofCarrots !

… that span local and Bigtable sources (#22115) (#22168) * get_signatures_for_address does not correctly account for result sets that span Blockstore and Bigtable. This causes Bigtable to return `RowNotFound` until the new tx is uploaded. Check that `before` exists in Bigtable, and if not, set it to `None` to return the full data set. References #21442 Closes #22110 * Differentiate between before sig not found and no newer signatures * Dedupe bigtable results to account for potential upload race Co-authored-by: Tyera Eulberg <tyera@solana.com> (cherry picked from commit bac6821) Co-authored-by: Omar Kilani <omar.kilani@gmail.com>

… that span local and Bigtable sources (backport #22115) (#22167) * get_signatures_for_address does not correctly account for result sets that span local and Bigtable sources (#22115) * get_signatures_for_address does not correctly account for result sets that span Blockstore and Bigtable. This causes Bigtable to return `RowNotFound` until the new tx is uploaded. Check that `before` exists in Bigtable, and if not, set it to `None` to return the full data set. References #21442 Closes #22110 * Differentiate between before sig not found and no newer signatures * Dedupe bigtable results to account for potential upload race Co-authored-by: Tyera Eulberg <tyera@solana.com> (cherry picked from commit bac6821) # Conflicts: # ledger/src/blockstore.rs * Fix conflicts Co-authored-by: Omar Kilani <omar.kilani@gmail.com> Co-authored-by: Tyera Eulberg <tyera@solana.com>

omarkilani · 2021-12-30T15:58:46Z

@CriesofCarrots I'm not sure if you see notifications on merged PRs, but just a random observation since Bigtable is currently down (at least from Germany, but looks like Triton One from SF also isn't able to access it).

I presume it's okay that this new code path hangs on .await (until a very long timeout?) while Bigtable is down?

I mean... it is in an async function and the existing code has the same issue a little later.

But this change introduces 2 times the timeout wait?

Is it DoS-able?

CriesofCarrots · 2021-12-31T01:00:24Z

But this change introduces 2 times the timeout wait?

Yeah, that's a fair point. Perhaps we can mitigate that somewhat by changing that .ok().flatten().is_none() to instead only match SignatureNotFound errors and otherwise return whatever results we already have.

mergify Bot added the community Community contribution label Dec 25, 2021

mergify Bot requested a review from a team December 25, 2021 22:26

mvines requested a review from CriesofCarrots December 27, 2021 18:32

mvines added v1.8 labels Dec 27, 2021

CriesofCarrots reviewed Dec 27, 2021

View reviewed changes

Comment thread rpc/src/rpc.rs

Comment thread rpc/src/rpc.rs

vercel Bot temporarily deployed to Preview December 28, 2021 14:34 Inactive

vercel Bot temporarily deployed to Preview December 28, 2021 22:51 Inactive

vercel Bot temporarily deployed to Preview December 29, 2021 00:33 Inactive

vercel Bot temporarily deployed to Preview December 29, 2021 06:27 Inactive

Differentiate between before sig not found and no newer signatures

f724a98

vercel Bot temporarily deployed to Preview December 29, 2021 06:28 Inactive

Dedupe bigtable results to account for potential upload race

e924368

vercel Bot temporarily deployed to Preview December 29, 2021 06:43 Inactive

CriesofCarrots merged commit bac6821 into solana-labs:master Dec 29, 2021

mergify Bot mentioned this pull request Dec 29, 2021

get_signatures_for_address does not correctly account for result sets that span local and Bigtable sources (backport #22115) #22167

Merged

mergify Bot mentioned this pull request Dec 29, 2021

get_signatures_for_address does not correctly account for result sets that span local and Bigtable sources (backport #22115) #22168

Merged

omarkilani deleted the patch-1 branch December 29, 2021 17:36

CriesofCarrots mentioned this pull request Dec 31, 2021

Exit get_signatures_for_address on first BigTable error #22200

Merged

brooksprumo mentioned this pull request Jan 5, 2022

v1.9/fix build #22311

Closed

Conversation

omarkilani commented Dec 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Proposed Solution

Uh oh!

codecov Bot commented Dec 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

omarkilani commented Dec 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Case where only Bigtable contains tx history:

Case where both local storage and Bigtable contain non-overlapping sets of tx history:

Case where local storage and Bigtable have overlapping sets of tx history:

Uh oh!

CriesofCarrots left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

omarkilani commented Dec 28, 2021

Uh oh!

omarkilani commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarkilani commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarkilani commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CriesofCarrots commented Dec 28, 2021

Uh oh!

omarkilani commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarkilani commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarkilani commented Dec 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CriesofCarrots commented Dec 29, 2021

Uh oh!

omarkilani commented Dec 29, 2021

Uh oh!

CriesofCarrots commented Dec 29, 2021

Uh oh!

omarkilani commented Dec 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CriesofCarrots commented Dec 29, 2021

Uh oh!

omarkilani commented Dec 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CriesofCarrots commented Dec 29, 2021

Uh oh!

omarkilani commented Dec 29, 2021

Uh oh!

omarkilani commented Dec 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CriesofCarrots commented Dec 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

omarkilani commented Dec 25, 2021 •

edited

Loading

codecov Bot commented Dec 26, 2021 •

edited

Loading

omarkilani commented Dec 26, 2021 •

edited

Loading

omarkilani commented Dec 28, 2021 •

edited

Loading

omarkilani commented Dec 28, 2021 •

edited

Loading

omarkilani commented Dec 28, 2021 •

edited

Loading

omarkilani commented Dec 28, 2021 •

edited

Loading

omarkilani commented Dec 28, 2021 •

edited

Loading

omarkilani commented Dec 29, 2021 •

edited

Loading

omarkilani commented Dec 29, 2021 •

edited

Loading

omarkilani commented Dec 29, 2021 •

edited

Loading

omarkilani commented Dec 30, 2021 •

edited

Loading