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

[Build Operations] Include Soroban's extend-ttl #1235

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jeesunikim
Copy link
Contributor

@jeesunikim jeesunikim commented Jan 29, 2025

Summary:

  • Add a soroban operation
  • I completely separated soroban operation from the classic one since soroban is evolving
  • Soroban operation is disabled in the dropdown. I will enable it once I update the submit page in the next PR

}}
placement="right"
// if we can't build a txn to simulate, we can't fetch the min resource fee
disabled={!(isValid.params && isOperationValidForSimulateTx)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@quietbits disabling the button until all operation params (except the resource fee) is filled

@stellar-jenkins
Copy link

1 similar comment
@stellar-jenkins
Copy link

// Used only for a Soroban operation
// Includes a simulate transaction button to fetch
// the minimum resource fee
export const ResourceFeePickerWithQuery = ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the component's name to include Query


// Soroban Operation only allows one operation
// Add Operation button should be disabled
await expect(page.getByText("Add Operation")).toBeDisabled();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using disabled instead of not visible

@stellar-jenkins
Copy link

@stellar-jenkins
Copy link

@jeesunikim jeesunikim requested a review from quietbits January 29, 2025 19:36
@@ -932,6 +1089,204 @@ export const Operations = () => {
);
};

/* Soroban Operations */
// Unlike classic transactions, Soroban tx can only have one operation
if (soroban.operation.operation_type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could move Soroban and Classic operation blocks to their own components. This file is getting very long, and that might clean it up a bit. What do you think?

Comment on lines +74 to +80
const sorobanData = getSorobanTxDataResult();

useEffect(() => {
if (sorobanData.xdr) {
updateSorobanBuildXdr(sorobanData.xdr);
}
}, [sorobanData.xdr, updateSorobanBuildXdr]);
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be safer to create a variable for sorobanData.xdr because sorobanData could be undefined.

Suggested change
const sorobanData = getSorobanTxDataResult();
useEffect(() => {
if (sorobanData.xdr) {
updateSorobanBuildXdr(sorobanData.xdr);
}
}, [sorobanData.xdr, updateSorobanBuildXdr]);
const sorobanData = getSorobanTxDataResult();
const sorobanDataXdr = sorobanData?.xdr || "";
useEffect(() => {
if (sorobanDataXdr) {
updateSorobanBuildXdr(sorobanDataXdr);
}
}, [sorobanDataXdr, updateSorobanBuildXdr]);

Comment on lines +96 to +148
<ValidationResponseCard
variant="success"
title="Success! Soroban Transaction Envelope XDR:"
response={
<Box gap="xs" data-testid="build-soroban-transaction-envelope-xdr">
<div>
<div>Network Passphrase:</div>
<div>{network.passphrase}</div>
</div>
<div>
<div>Hash:</div>
<div>{txnHash}</div>
</div>
<div>
<div>XDR:</div>
<div>{sorobanData.xdr}</div>
</div>
</Box>
}
note={
<>
In order for the transaction to make it into the ledger, a
transaction must be successfully signed and submitted to the
network. The Lab provides the{" "}
<SdsLink href={Routes.SIGN_TRANSACTION}>
Transaction Signer
</SdsLink>{" "}
for signing a transaction, and the{" "}
<SdsLink href={Routes.SUBMIT_TRANSACTION}>
Post Transaction endpoint
</SdsLink>{" "}
for submitting one to the network.
</>
}
footerLeftEl={
<>
<Button
size="md"
variant="secondary"
onClick={() => {
updateSignImportXdr(sorobanData.xdr);
updateSignActiveView("overview");

router.push(Routes.SIGN_TRANSACTION);
}}
>
Sign in Transaction Signer
</Button>

<ViewInXdrButton xdrBlob={sorobanData.xdr} />
</>
}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

If this component is almost the same as the Classic one, we should create a component to use in both places to make it easier to update them in the future.

}
return builtXdr;
} catch (e) {
setErrorMessage(`${e}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the final message note that this is for the fee? I'm not sure if it would be clear to the user that these error messages relate to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog (Not Ready)
Development

Successfully merging this pull request may close these issues.

3 participants