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

soroban-rpc: getLedgerEntries may interleave incorrect keys and values. #1084

Closed
Shaptic opened this issue Nov 15, 2023 · 0 comments · Fixed by #1093
Closed

soroban-rpc: getLedgerEntries may interleave incorrect keys and values. #1084

Shaptic opened this issue Nov 15, 2023 · 0 comments · Fixed by #1093
Labels
bug Something isn't working

Comments

@Shaptic
Copy link
Contributor

Shaptic commented Nov 15, 2023

Bug Report

What version are you using?

Testnet, which I believe is v20.0.0-rc.4.1.

What did you do?

I requested two ledger entries: a ContractData key alongside its corresponding ExpirationData key. Here is a bash script:

#! /bin/bash

set -e
curl -s -H 'Content-Type: application/json' --data '{
  "jsonrpc": "2.0",
  "id": 12345,
  "method": "getLedgerEntries",
  "params": [[
    "AAAABgAAAAHm/PXiV0Tr/3LrABRnZtg8JYtnHiDjyuf2WN1Wx6xuBAAAABAAAAABAAAAAgAAAA8AAAAHUG9vbEVQUwAAAAASAAAAARlJVwVvl0UjM1CLV/d+MOmzOCgtTG85KqcIOlvOYObNAAAAAQ==",
    "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLL"
  ]]
}' 'https://soroban-testnet.stellar.org' | jq '.result.entries'

What did you expect to see?

I expected to see two returned entries: one with key as the same key and xdr as the corresponding LedgerDataEntry.

What did you see instead?

On occasion (approx. one in a dozen runs), the values would interleave incorrectly with the keys. In other words, the LedgerKeyContractData in key would have the ExpirationData in xdr and vice-versa. Observe:

// $ ./repro.sh
[
  {
    "key": "AAAABgAAAAHm/PXiV0Tr/3LrABRnZtg8JYtnHiDjyuf2WN1Wx6xuBAAAABAAAAABAAAAAgAAAA8AAAAHUG9vbEVQUwAAAAASAAAAARlJVwVvl0UjM1CLV/d+MOmzOCgtTG85KqcIOlvOYObNAAAAAQ==",
    "xdr": "AAAABgAAAAAAAAAB5vz14ldE6/9y6wAUZ2bYPCWLZx4g48rn9ljdVsesbgQAAAAQAAAAAQAAAAIAAAAPAAAAB1Bvb2xFUFMAAAAAEgAAAAEZSVcFb5dFIzNQi1f3fjDpszgoLUxvOSqnCDpbzmDmzQAAAAEAAAAKAAAAAAAAAAAAAAAAABt3QA==",
    "lastModifiedLedgerSeq": "2535782"
  },
  {
    "key": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLL",
    "xdr": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLLACpiZg==",
    "lastModifiedLedgerSeq": "2535782"
  }
]
// $ ./repro.sh
[
  {
    "key": "AAAABgAAAAHm/PXiV0Tr/3LrABRnZtg8JYtnHiDjyuf2WN1Wx6xuBAAAABAAAAABAAAAAgAAAA8AAAAHUG9vbEVQUwAAAAASAAAAARlJVwVvl0UjM1CLV/d+MOmzOCgtTG85KqcIOlvOYObNAAAAAQ==",
    "xdr": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLLACpiZg==",
    "lastModifiedLedgerSeq": "2535782"
  },
  {
    "key": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLL",
    "xdr": "AAAABgAAAAAAAAAB5vz14ldE6/9y6wAUZ2bYPCWLZx4g48rn9ljdVsesbgQAAAAQAAAAAQAAAAIAAAAPAAAAB1Bvb2xFUFMAAAAAEgAAAAEZSVcFb5dFIzNQi1f3fjDpszgoLUxvOSqnCDpbzmDmzQAAAAEAAAAKAAAAAAAAAAAAAAAAABt3QA==",
    "lastModifiedLedgerSeq": "2535782"
  }
]
// $ ./repro.sh
[
  {
    "key": "AAAABgAAAAHm/PXiV0Tr/3LrABRnZtg8JYtnHiDjyuf2WN1Wx6xuBAAAABAAAAABAAAAAgAAAA8AAAAHUG9vbEVQUwAAAAASAAAAARlJVwVvl0UjM1CLV/d+MOmzOCgtTG85KqcIOlvOYObNAAAAAQ==",
    "xdr": "AAAABgAAAAAAAAAB5vz14ldE6/9y6wAUZ2bYPCWLZx4g48rn9ljdVsesbgQAAAAQAAAAAQAAAAIAAAAPAAAAB1Bvb2xFUFMAAAAAEgAAAAEZSVcFb5dFIzNQi1f3fjDpszgoLUxvOSqnCDpbzmDmzQAAAAEAAAAKAAAAAAAAAAAAAAAAABt3QA==",
    "lastModifiedLedgerSeq": "2535782"
  },
  {
    "key": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLL",
    "xdr": "AAAACa/lybv+NkUUDNmdxpiLnu6JZif6hKYdbMERMpzUNQLLACpiZg==",
    "lastModifiedLedgerSeq": "2535782"
  }
]

The first and third execution are correct. However, in the second execution, the xdr fields do not match their key fields. Again, this happens sporadically.

Additional Investigation

I believe the culprit lies in these lines of code:

https://github.com/stellar/soroban-tools/blob/3fcab5b4d46c441548c78b003c2c0c2513aa070f/cmd/soroban-rpc/internal/methods/get_ledger_entries.go#L111-L115

There is an inherent assumption that the earlier call to tx.GetLedgerEntries(ledgerKeys...) (line 90) will return the entries in the same order as they were requested. However, this empirically does not appear to be the case. Instead, we should encode the key the same way we encode the XDR when building the entry rather than relying on the i index.

I believe this should also be backported to the current testnet release (i.e. some rc.4.2), which should not be difficult because that file has not changed since then, so there shouldn't be conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant