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

fix: wait on solana confirmed tx #11515

Merged
merged 2 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/blockchain-link/src/workers/solana/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ import type * as MessageTypes from '@trezor/blockchain-link-types/src/messages';
import { CustomError } from '@trezor/blockchain-link-types/src/constants/errors';
import { BaseWorker, ContextType, CONTEXT } from '../baseWorker';
import { MESSAGES, RESPONSES } from '@trezor/blockchain-link-types/src/constants';
import { Connection, Message, PublicKey } from '@solana/web3.js';
import {
BlockheightBasedTransactionConfirmationStrategy,
Connection,
Message,
PublicKey,
} from '@solana/web3.js';
import { solanaUtils } from '@trezor/blockchain-link-utils';

import {
Expand Down Expand Up @@ -95,6 +100,14 @@ const pushTransaction = async (request: Request<MessageTypes.PushTransaction>) =
const rawTx = request.payload.startsWith('0x') ? request.payload.slice(2) : request.payload;
const api = await request.connect();
const payload = await api.sendRawTransaction(Buffer.from(rawTx, 'hex'));
const { blockhash, lastValidBlockHeight } = await api.getLatestBlockhash('finalized');
const confirmStrategy: BlockheightBasedTransactionConfirmationStrategy = {
blockhash,
lastValidBlockHeight,
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these values be based of the blockhash included in the transaction? That's why I originally didn't do it because it would be harder to refactor the code to pass in the correct values.

However I suppose it's not a huge deal, as the only drawback is that the user will simply wait longer for the transaction to fail, but still.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it could be like that because it's working and also some thread on Solana StackOverflow is saying the same

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. If pending transactions ever are implemented then I'd suggest to fix this part as well to include the blockchash from the transaction and its corresponding lastValidBlockHeight. As I mentioned in the previous comment - the only drawback of the solution in this PR should be longer than necessary wait times for transaction failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the difference in blockhashes might be, that the txn will be considered expired later (due to this blockhash being younger than the one in the txn). But success / failure should return normally

signature: payload,
};

await api.confirmTransaction(confirmStrategy);

return {
type: RESPONSES.PUSH_TRANSACTION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const TransactionReviewModalContent = ({
const isActionAbortable = useSelector(selectIsActionAbortable);
const dispatch = useDispatch();
const [detailsOpen, setDetailsOpen] = useState(false);
const [isSending, setIsSending] = useState(false);

const deviceModelInternal = device?.features?.internal_model;

Expand Down Expand Up @@ -73,10 +74,6 @@ export const TransactionReviewModalContent = ({

const ethereumStakeType =
'ethereumStakeType' in precomposedForm ? precomposedForm.ethereumStakeType : null;
const actionText = getTransactionReviewModalActionText({
ethereumStakeType,
isRbfAction,
});

// omit other button requests (like passphrase)
const buttonRequests = device.buttonRequests.filter(
Expand Down Expand Up @@ -140,7 +137,10 @@ export const TransactionReviewModalContent = ({
broadcast={precomposedForm.options.includes('broadcast')}
detailsOpen={detailsOpen}
onDetailsClick={() => setDetailsOpen(!detailsOpen)}
actionText={actionText}
actionText={getTransactionReviewModalActionText({
ethereumStakeType,
isRbfAction,
})}
/>
<TransactionReviewOutputList
account={selectedAccount.account}
Expand All @@ -152,7 +152,13 @@ export const TransactionReviewModalContent = ({
outputs={outputs}
buttonRequestsCount={buttonRequestsCount}
isRbfAction={isRbfAction}
actionText={actionText}
actionText={getTransactionReviewModalActionText({
ethereumStakeType,
isRbfAction,
isSending,
})}
isSending={isSending}
setIsSending={() => setIsSending(true)}
/>
<TransactionReviewEvmExplanation account={selectedAccount.account} />
</StyledModal>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export interface TransactionReviewOutputListProps {
buttonRequestsCount: number;
isRbfAction: boolean;
actionText: TranslationKey;
isSending?: boolean;
setIsSending?: () => void;
}

export const TransactionReviewOutputList = ({
Expand All @@ -102,6 +104,8 @@ export const TransactionReviewOutputList = ({
buttonRequestsCount,
isRbfAction,
actionText,
isSending,
setIsSending,
}: TransactionReviewOutputListProps) => {
const dispatch = useDispatch();
const { networkType } = account;
Expand Down Expand Up @@ -141,6 +145,9 @@ export const TransactionReviewOutputList = ({
},
});
const handleSend = () => {
if (networkType === 'solana') {
setIsSending?.();
}
if (decision) {
decision.resolve(true);

Expand Down Expand Up @@ -228,6 +235,7 @@ export const TransactionReviewOutputList = ({
<StyledButton
data-test="@modal/send"
isDisabled={!signedTx}
isLoading={isSending}
onClick={handleSend}
>
<Translation id={actionText} />
Expand Down
4 changes: 4 additions & 0 deletions packages/suite/src/support/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6487,6 +6487,10 @@ export default defineMessages({
id: 'TR_REPLACE_TX',
defaultMessage: 'Replace transaction',
},
TR_CONFIRMING_TX: {
id: 'TR_CONFIRMING_TX',
defaultMessage: 'Confirming transaction',
},
TR_FINALIZE_TX: {
id: 'TR_FINALIZE_TX',
defaultMessage: 'Finalize transaction',
Expand Down
6 changes: 6 additions & 0 deletions packages/suite/src/utils/suite/transactionReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import { StakeFormState } from '@suite-common/wallet-types';
interface getTransactionReviewModalActionTextParams {
ethereumStakeType: StakeFormState['ethereumStakeType'] | null;
isRbfAction: boolean;
isSending?: boolean;
}

export const getTransactionReviewModalActionText = ({
ethereumStakeType,
isRbfAction,
isSending,
}: getTransactionReviewModalActionTextParams): TranslationKey => {
switch (ethereumStakeType) {
case 'stake':
Expand All @@ -24,5 +26,9 @@ export const getTransactionReviewModalActionText = ({
return 'TR_REPLACE_TX';
}

if (isSending) {
return 'TR_CONFIRMING_TX';
}

return 'SEND_TRANSACTION';
};
Loading