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.

Sign up for GitHub

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

Jump to bottom

chore: upgraded subxt version #483

Merged
merged 7 commits into from
Jul 19, 2023
Merged

chore: upgraded subxt version #483

merged 7 commits into from
Jul 19, 2023

Conversation

nakul1010
Copy link
Member

@nakul1010 nakul1010 commented Jun 30, 2023 •
edited
Loading

PR Description:

Closes #475
Closes #418

Changes Implemented:

  • Modified dependencies.
  • Upgraded subxt version without utilizing the substrate-compat feature, thereby preventing conflicts with sp dependencies.
  • Imported necessary utils from subxt to the client, ensuring compatibility with client substrate dependencies.
  • Update test cases
  • Added custom implementation for AccountId, PairSigner and MultiSignature since these types are feature gated by substrate-compat feature in subxt and hence can't be imported from subxt.

Alternate Approach:

  • Considered forking subxt and updating the dependencies based on the node. However, this approach would require maintaining the forked version, thus it was discarded.

References to Issues Opened on Subxt:

@nakul1010 nakul1010 added the InProgress The PR is still in progress label Jun 30, 2023
@nakul1010 nakul1010 self-assigned this Jun 30, 2023
@nakul1010 nakul1010 linked an issue Jun 30, 2023 that may be closed by this pull request
Upgrade the subxt dependency #475
Closed
@nakul1010
Merge branch 'master' of https://github.com/interlay/interbtc-clients …
c10a1a9 
…into nakul/chore_upgrade_subxt_version

# Conflicts:
#	Cargo.lock
#	runtime/src/rpc.rs
@nakul1010 nakul1010 marked this pull request as draft June 30, 2023 04:57
nakul1010
nakul1010 commented Jun 30, 2023
View reviewed changes
runtime/src/types.rs Outdated Show resolved Hide resolved
@nakul1010 nakul1010 added question Further information is requested and removed question Further information is requested labels Jun 30, 2023
@nakul1010 nakul1010 mentioned this pull request Jul 4, 2023
Closed
gregdhill
gregdhill requested changes Jul 10, 2023
View reviewed changes
Copy link
Member

@gregdhill gregdhill left a comment

Choose a reason for hiding this comment

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

I've only reviewed about half of the files but I stopped because there are a lot of unnecessary changes. Please go through the diff yourself and revert anything unrelated to the subxt upgrade. In general please keep changes to a minimum so it is easier to review.

bitcoin/tests/electrs.rs Outdated Show resolved Hide resolved
bitcoin/tests/integration_test.rs Outdated Show resolved Hide resolved
faucet/Cargo.toml Outdated Show resolved Hide resolved
faucet/Cargo.toml Outdated Show resolved Hide resolved
runner/Cargo.toml Show resolved Hide resolved
vault/tests/vault_integration_tests.rs Outdated Show resolved Hide resolved
vault/src/refund.rs Outdated Show resolved Hide resolved
vault/src/refund.rs Outdated Show resolved Hide resolved
vault/src/relay/mod.rs Outdated Show resolved Hide resolved
vault/src/replace.rs Outdated Show resolved Hide resolved
@nakul1010 nakul1010 added ReviewNeeded The discussion can be reviewed by peers and removed InProgress The PR is still in progress labels Jul 11, 2023
gregdhill
gregdhill previously requested changes Jul 11, 2023
View reviewed changes
runtime/src/tests.rs Outdated Show resolved Hide resolved
runtime/Cargo.toml Outdated Show resolved Hide resolved
vault/Cargo.toml Outdated Show resolved Hide resolved
vault/src/cancellation.rs Outdated Show resolved Hide resolved
vault/src/cancellation.rs Outdated Show resolved Hide resolved
runtime/src/rpc.rs Outdated Show resolved Hide resolved
runtime/src/types.rs Show resolved Hide resolved
runtime/src/tests.rs Outdated Show resolved Hide resolved
runtime/src/rpc.rs Outdated Show resolved Hide resolved
runtime/src/rpc.rs Outdated Show resolved Hide resolved
@nakul1010 nakul1010 added InProgress The PR is still in progress and removed ReviewNeeded The discussion can be reviewed by peers labels Jul 12, 2023
@nakul1010 nakul1010 mentioned this pull request Jul 12, 2023
Merged
1 task
@nakul1010 nakul1010 requested a review from gregdhill July 12, 2023 20:35
@nakul1010 nakul1010 added ReviewNeeded The discussion can be reviewed by peers and removed InProgress The PR is still in progress labels Jul 12, 2023
gregdhill
gregdhill reviewed Jul 14, 2023
View reviewed changes
runtime/src/rpc.rs Outdated Show resolved Hide resolved
@nakul1010 nakul1010 marked this pull request as ready for review July 17, 2023 13:43
@nakul1010 nakul1010 requested a review from gregdhill July 17, 2023 13:43
sander2
sander2 reviewed Jul 17, 2023
View reviewed changes
runtime/src/rpc.rs Outdated Show resolved Hide resolved
gregdhill
gregdhill reviewed Jul 17, 2023
View reviewed changes
runtime/src/types.rs Show resolved Hide resolved
runtime/src/utils/account_id.rs Outdated Show resolved Hide resolved
vault/tests/vault_integration_tests.rs Outdated Show resolved Hide resolved
@nakul1010 nakul1010 force-pushed the nakul/chore_upgrade_subxt_version branch from 44b5036 to e74a940 Compare July 18, 2023 07:25
@nakul1010 nakul1010 force-pushed the nakul/chore_upgrade_subxt_version branch from e74a940 to 21a5c24 Compare July 18, 2023 07:38
@nakul1010
chore: rebased on master
8f4a4e1
@nakul1010
Merge branch 'master' of https://github.com/interlay/interbtc-clients …
b57f1b6 
…into nakul/chore_upgrade_subxt_version

# Conflicts:
#	Cargo.lock
#	faucet/src/http.rs
#	runner/src/runner.rs
#	runtime/Cargo.toml
#	runtime/src/lib.rs
#	runtime/src/tests.rs
#	vault/src/main.rs
#	vault/tests/vault_integration_tests.rs
@nakul1010 nakul1010 requested a review from sander2 July 18, 2023 08:21
@nakul1010 nakul1010 requested a review from sander2 July 18, 2023 08:44
@nakul1010 nakul1010 requested a review from sander2 July 19, 2023 06:04
@nakul1010 nakul1010 requested a review from gregdhill July 19, 2023 08:04
@sander2 sander2 dismissed gregdhill’s stale review July 19, 2023 08:04

points are addressed

@sander2 sander2 merged commit 434dc1d into master Jul 19, 2023
6 checks passed
@sander2 sander2 deleted the nakul/chore_upgrade_subxt_version branch July 19, 2023 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sander2 sander2 sander2 approved these changes

@gregdhill gregdhill Awaiting requested review from gregdhill

Assignees

@nakul1010 nakul1010

Labels
ReviewNeeded The discussion can be reviewed by peers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade the subxt dependency Instant seal panic
3 participants
@nakul1010 @sander2 @gregdhill