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

Updates soroban-client version, updates use of helpers #13

Merged
merged 7 commits into from
Sep 25, 2023

Conversation

aristidesstaffieri
Copy link
Contributor

Updates to soroban-client@0.11.2 and makes use of the new auth/tx builder related helpers.

@aristidesstaffieri aristidesstaffieri self-assigned this Sep 12, 2023
@aristidesstaffieri aristidesstaffieri changed the title [WIP] updates soroban-client version, updates use of helpers Updates soroban-client version, updates use of helpers Sep 13, 2023
Copy link

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

first-pass drive-by review 🏎️

const txSim = await server.simulateTransaction(tx);
const txSim = (await server.simulateTransaction(
tx,
)) as SorobanRpc.SimulateTransactionSuccessResponse;
Copy link

Choose a reason for hiding this comment

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

as SorobanRpc.SimulateTransactionSuccessResponse

ballsy 😝

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah lol fair call out I won't cast it 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 736cc1d

// set auth entry to expire when contract data expires, but could any number of blocks in the future
console.log(parsed);
// expirationLedgerSeq = parsed.expiration().expirationLedgerSeq();
expirationLedgerSeq = 49431 + 1000000;
Copy link

Choose a reason for hiding this comment

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

magic numbers? deserves at least a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah didn't mean to leave this, still figuring out why I can't fetch the expirationLedgerSeq the way I used to and have this placeholder value for now.

Copy link

Choose a reason for hiding this comment

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

ah well that's because the entry doesn't have expiration ledger anymore 😂 😭 we may put it back in later versions but tl;dr it's because the expiration is in a separate entry now from core's perspective, so you'll need to fetch it from there if you want. you have to look up the entry with the hash of the ledger key of the data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that would do it, hey thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait isn't that what I'm already doing here?

const key = xdr.LedgerKey.contractData(
  new xdr.LedgerKeyContractData({
    contract: new Address(contractID).toScAddress(),
    key: xdr.ScVal.scvLedgerKeyContractInstance(),
    durability: xdr.ContractDataDurability.persistent(),
  }),
);

// Fetch the current contract ledger seq
// eslint-disable-next-line no-await-in-loop
const entryRes = await server.getLedgerEntries((key));
if (entryRes.entries && entryRes.entries.length) {
  const parsed = xdr.LedgerEntryData.fromXDR(
    entryRes.entries[0].xdr,
    "base64",
  );
  // set auth entry to expire when contract data expires, but could any number of blocks in the future
  console.log(parsed);
  // expirationLedgerSeq = parsed.expiration().expirationLedgerSeq();
  expirationLedgerSeq = 81365 + 1000000;
} else {
  throw new Error(ERRORS.CANNOT_FETCH_LEDGER_ENTRY);
}

It seems like the sig changed from xdr.LedgerKey[] to xdr.LedgerKey but it still wants a LedgerKey and not a Buffer or am I missing something?

Copy link

Choose a reason for hiding this comment

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

Uhh no, not quite. So yes, getLedgerEntries takes a spread rather than an array now (better ux, I hope you'd agree), but now the returned entries, as far as I understand it, probably do not have the expiration() field (see e.g. ContractDataEntry).

This is because the expiration data is now stored separately, in a separate ledger entry. What you need to do instead is request that entry, too. This means taking the hash of the ledger key that you're actually looking up: see LedgerKeyExpiration. In the above case,

const expirationKey = new xdr.LedgerKeyExpiration(SorobanClient.hash(key));

Then, you can request both: getLedgerEntries(key, expirationKey).

(and this may change with either the client or the RPC in the future, but it is what it is right now 🤷)

Does that help?

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 sure does, thanks for that. Fixed in 881f8c4

@aristidesstaffieri aristidesstaffieri removed the request for review from piyalbasu September 20, 2023 19:31
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.

2 participants