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

[NONEVM-706][Solana] - Refactor TXM + Rebroadcast Expired Tx functionality #946

Merged
merged 60 commits into from
Dec 19, 2024

Conversation

Farber98
Copy link
Contributor

@Farber98 Farber98 commented Nov 26, 2024

Description

Refactor confirm and sendWithRetry funcs

Adding a new TxExpirationRebroadcast toggable routine:

  • Inside confirm loop, after checking confirmations, we will determine if we need to rebroadcast expired transactions that where not processed yet. Rebroadcasting a transaction involves:
    • Removing previous tx and creating a new one with updated blockhash
      • Consideration: txid will be maintained, in case it was set by caller
    • calling sendWithRetry directly without enqueuing
    • Consideration: Implicitly it may be "bumping" despite config being off. We may get a new higher price for the rebroadcasted tx when we build and get a new compute unit price.

https://smartcontract-it.atlassian.net/browse/NONEVM-706

prev branch: #928

Merge together with:

Soak Tests:

pkg/solana/txm/txm.go Outdated Show resolved Hide resolved
pkg/solana/relay.go Outdated Show resolved Hide resolved
pkg/solana/relay.go Outdated Show resolved Hide resolved
@aalu1418 aalu1418 merged commit 150f744 into develop Dec 19, 2024
36 checks passed
@aalu1418 aalu1418 deleted the backup-branch-fee-bumping branch December 19, 2024 17:34
dhaidashenko pushed a commit that referenced this pull request Dec 20, 2024
…ality (#946)

* refactor so txm owns blockhash assignment

* lastValidBlockHeight shouldn't be exported

* better comment

* refactor sendWithRetry to make it clearer

* confirm loop refactor

* fix infinite loop

* move accountID inside msg

* lint fix

* base58 does not contain lower l

* fix hash errors

* fix generate random hash

* remove blockhash as we only need block height

* expired tx changes without tests

* add maybe to mocks

* expiration tests

* send txes through queue

* revert pendingtx leakage of information. overwrite blockhash

* fix order of confirm loop and not found signature check

* fix mocks

* prevent confirmation loop to mark tx as errored when it needs to be rebroadcasted

* fix test

* fix pointer

* add comments

* reduce rpc calls + refactors

* tests + check to save rpc calls

* address feedback + remove redundant impl

* iface comment

* address feedback on compute unit limit and lastValidBlockHeight assignment

* blockhash assignment inside txm.sendWithRetry

* address feedback

* Merge branch 'develop' into nonevm-706-support-custom-bumping-strategy-rpc-expiration-within-confirmation

* refactors after merge

* fix interactive rebase

* fix whitespace diffs

* fix import

* fix mocks

* add on prebroadcaste error

* remove rebroadcast count and fix package

* improve docs

* fix comparison against blockHeight instead of slotHeight

* address feedback

* fix lint

* fix log

* address feedback

* remove useless slot height

* address feedback

* validate that tx doesn't exist in any of maps when adding new tx

* callers set lastValidBlockheight + get blockhash on expiration + integration tests

* add enq iface comm to help callers

* address feedback
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.

5 participants