Bugfix/tx history restore bump #945

aristidesstaffieri · 2023-08-16T18:22:29Z

Ticket: https://stellarorg.atlassian.net/browse/WAL-1008

This adds handling for the bump or restore operations that are new in Soroban with preview 10.
Previously we assumed that any Soroban related transactions would be accounted for in our OPERATION_TYPES enum, and this was ok because we controlled what contracts we supported(just token basically).

P10 introduced state expiry which means that now any user may have restored or bumped state through a transaction which broke this model.

This is fixed in 2 ways -

We add the OPERATION_TYPES key for bump and restore ops.
We no longer assume anything about unknown op types and render them as generic host invoke functions.

Unrelated bug fix -
I found that the mint token history item was flawed because it assumed the the minter already had some of the token, and the token ID saved in their tokenIds list. It also didn't account for the mint to be to the current wallet, so it didn't properly show a change in balance.
This is fixed by attempting to fetch token details when the mint operation is for a token that we do not have in our tokenIds list, and accounting for a change in balance when the mint is to the current public key.

This also includes refactors/re-organization of Soroban related helpers/tooling.

moved helpers to the shared namespace.
added server helper for simTx.
added specific token helpers and refactored to use new simTx helper.
removed helpers that could be replaced by ones in soroban-client.

Unresolved Issues -
Working on this uncovered interesting UX fallout from now potentially dealing with tokens where the state or code can be expired. For either account details or account history, we could run into a scenario where the contract has an expired entry and then we fail to fetch token details through a transaction simualtion, we can do a few things -

If the contract is a SAC, we can get this info from Horizon and we should anyway because it's free.
Quote from Dima about knowing when a contract is a SAC -

that's a good question. I'm not sure how SAC tokens should be added. they can be added either via classic Asset or via contract id. in the latter case you can easily figure out that it's SAC (just lookup executable in the contract instance entry), and then I believe you can get Asset spec from the token name (it's ASSET_NAME:ISSUER_KEY)

We could also index Soroban to always have up to date data about every token contract that we care about, quote from Tomer on that

Using SAC optimizations is an ok temporary fix. With that said I think we should take a step back and acknowledge that soroban-rpc is actually a poor wallet backend. and that’s ok - that’s not what it was built for. So obviously we can tackle building a wallet backend, though that might be a lot of work. We can see if we can utilize the work of someone else in the ecosystem. maybe stellar.expert (I suspect orbit will have something like this ready) or one of the incoming indexers. Or, we can potentially solve some of these issues by having more robust client-side state management

There is also the option to fetch the ledger entry directly, but I am still gathering details about it and don't yet understand how that would be different from the RPC getting it.

aristidesstaffieri · 2023-08-16T18:23:19Z

extension/src/popup/components/accountHistory/HistoryItem/index.tsx

@@ -80,7 +84,8 @@ export const HistoryItem = ({
    sourceAssetCode = operation.source_asset_code;
  }
  const operationType = camelCase(type) as keyof typeof OPERATION_TYPES;
-  const operationString = `${OPERATION_TYPES[operationType]}${
+  const opTypeStr = OPERATION_TYPES[operationType] || t("Transaction");


this was basically the source of the bug

aristidesstaffieri · 2023-08-16T18:23:52Z

extension/src/popup/components/accountHistory/HistoryItem/index.tsx

-        </>
-      );
-      IconComponent = <Icon.ArrowUp className="HistoryItem__icon--sent" />;
+  React.useEffect(() => {


I needed to make this async to better handle the mint token case, explained in the PR description.

piyalbasu · 2023-08-17T20:32:22Z

@shared/api/internal.ts

+  const decimals = await getDecimals(contractId, server, txBuilders.decimals);
+  const name = await getName(contractId, server, txBuilders.name);
+  const symbol = await getSymbol(contractId, server, txBuilders.symbol);
+  const balance = await getBalance(


nice - I find this pattern a lot more straightforward!

piyalbasu · 2023-08-17T20:33:51Z

@shared/helpers/soroban/server.ts

+  try {
+    // handle a case where scValToNative doesn't properly handle scvString
+    convertedScVal = scVal.str().toString();
+    return convertedScVal;


Ah, I believe this should be fixed now in the last soroban-client. So I don't think you need to handle this scvString case

oh yeah I did hear that also, I'll test it out to make sure.

done in 1ba0450

piyalbasu · 2023-08-17T20:37:38Z

@shared/helpers/soroban/token.ts

+    .build();
+
+  const result = await simulateTx<string>(tx, server);
+  return result;


no need to do this now since things could still change - but if this API holds, in the future we could probably refactor these helpers to share some logic as they're all kinda doing the same thing.

Also, @Shaptic - do we consider this a common enough use case (getting balance + decimals + name + symbol) that soroban-client should offer this?

Additionally - there's also a good convo happening about how we should be fetching data for Soroban now that expired state can be involved. It seems like we'll have to at least augment our approach anyway pretty soon. Some good ideas that came up includes -

Caching things like decimals/name/symbol when a user firsts gets a balance of a token.

Fetching the ledger entries directly(this is cheaper than the rpc call).

Getting these details from Horizon when token is a SAC.

Allowing users to restore their data when their token data is expired(implies we start storing expiration time along with other details).

I don't think this belongs in soroban-client, per se, but maybe this be handled by using soroban-cli to generate the TypeScript bindings for the SAC? cc @chadoh @willemneal

piyalbasu · 2023-08-17T20:39:36Z

extension/src/popup/components/accountHistory/HistoryItem/index.tsx

-    const token = tokenBalances.find(
-      (balance) => attrs && balance.contractId === attrs.contractId,
-    );
+  const [txDetails, setTxDetails] = React.useState(transactionDetailPropsBase);


Super NIT: but in other files in the repo, we usually just import { useState } from "react" at the top of the file so we just call useState directly instead of calling React.useState. So minor, but would be good to keep the pattern consistent

Same deal with useEffect

ah whoops, yup will update.

done in 1ba0450

Shaptic · 2023-08-17T22:45:31Z

extension/src/constants/transaction.ts

@@ -28,6 +28,7 @@ export enum OPERATION_TYPES {
  revokeTrustlineSponsorship = "Revoke Trustline Sponsorship",
  setOptions = "Set Options",
  setTrustLineFlags = "Set Trustline Flags",
+  bumpFootprintExpiration = "Bump Footprint Expiration",


missing restoreFootprint?

yes it is, thanks!

done in 05fa8fe

Shaptic · 2023-08-17T22:45:42Z

extension/src/popup/SorobanContext.tsx

@@ -9,9 +9,11 @@ import {

 import { settingsNetworkDetailsSelector } from "./ducks/settings";

+const BASE_FEE = "100";


you should use SorobanClient.BASE_FEE, I think

thanks, I keep meaning to update that. Done in 05fa8fe

Shaptic · 2023-08-17T22:47:02Z

extension/src/popup/components/sendPayment/SendConfirm/TransactionDetails/index.tsx

+      const params = [
+        new SorobanClient.Address(publicKey).toScVal(), // from
+        new SorobanClient.Address(destination).toScVal(), // to
+        SorobanClient.nativeToScVal(parsedAmount.toNumber(), { type: "i128" }), // amount


new SorobanClient.XdrLargeInt('i128', parsedAmount).toScVal() may be faster/more direct, I think?

done in 05fa8fe

… i128 helper

socket-security · 2023-08-18T15:47:10Z

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
soroban-client	0.9.1...0.9.2	None	`+3/-3`	11.2 MB	stellar-npm-ci

aristidesstaffieri · 2023-08-18T17:30:52Z

New Entries look like this, also improved mint & transfer a bit.

aristidesstaffieri added 5 commits August 15, 2023 13:36

adds the bump footprint expiration op type to tx history

60ba6e7

WIP, refactors history item to build async rows

1976f25

clean up and organize soroban related helpers

73b39c8

Added translations

4bf2718

remove unwanted files from translations commit

30b4c0a

aristidesstaffieri self-assigned this Aug 16, 2023

aristidesstaffieri requested a review from piyalbasu August 16, 2023 18:22

aristidesstaffieri commented Aug 16, 2023

View reviewed changes

piyalbasu reviewed Aug 17, 2023

View reviewed changes

piyalbasu approved these changes Aug 17, 2023

View reviewed changes

Shaptic reviewed Aug 17, 2023

View reviewed changes

aristidesstaffieri added 3 commits August 18, 2023 08:44

removes helper shim and fixes import pattern

1ba0450

use sdk base fee, add restore footprint header title, use more direct…

05fa8fe

… i128 helper

fixes soroban tx submission error handling, bumps soroban-client

b52103c

aristidesstaffieri added 3 commits August 18, 2023 09:59

fix use of source account in SorobanContext newTxBuilder

bb75977

split transfer and mint history items

40de096

Added translations

8a265ce

aristidesstaffieri changed the base branch from master to release/5.5.0 August 18, 2023 17:29

aristidesstaffieri merged commit e756283 into release/5.5.0 Aug 18, 2023
6 checks passed

aristidesstaffieri deleted the bugfix/tx-history-restore-bump branch August 18, 2023 19:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix/tx history restore bump #945

Bugfix/tx history restore bump #945

aristidesstaffieri commented Aug 16, 2023

aristidesstaffieri Aug 16, 2023

aristidesstaffieri Aug 16, 2023

piyalbasu Aug 17, 2023

piyalbasu Aug 17, 2023

aristidesstaffieri Aug 17, 2023

aristidesstaffieri Aug 18, 2023

piyalbasu Aug 17, 2023

aristidesstaffieri Aug 17, 2023 •

edited

Loading

Shaptic Aug 17, 2023

piyalbasu Aug 17, 2023

piyalbasu Aug 17, 2023

aristidesstaffieri Aug 17, 2023

aristidesstaffieri Aug 18, 2023

Shaptic Aug 17, 2023

aristidesstaffieri Aug 18, 2023

aristidesstaffieri Aug 18, 2023

Shaptic Aug 17, 2023

aristidesstaffieri Aug 18, 2023

Shaptic Aug 17, 2023

aristidesstaffieri Aug 18, 2023

socket-security bot commented Aug 18, 2023

aristidesstaffieri commented Aug 18, 2023

		@@ -9,9 +9,11 @@ import {

		import { settingsNetworkDetailsSelector } from "./ducks/settings";

		const BASE_FEE = "100";

Bugfix/tx history restore bump #945

Bugfix/tx history restore bump #945

Conversation

aristidesstaffieri commented Aug 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aristidesstaffieri Aug 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

socket-security bot commented Aug 18, 2023

aristidesstaffieri commented Aug 18, 2023

aristidesstaffieri Aug 17, 2023 •

edited

Loading