Skip to content

refactor: remove the code to receive the contract address from wallet signing portal#813

Merged
AlexD10S merged 4 commits intomainfrom
refactor/wallet-integration-portal
Dec 4, 2025
Merged

refactor: remove the code to receive the contract address from wallet signing portal#813
AlexD10S merged 4 commits intomainfrom
refactor/wallet-integration-portal

Conversation

@AlexD10S
Copy link
Copy Markdown
Member

@AlexD10S AlexD10S commented Dec 3, 2025

We previously added a patch to manually calculate the smart contract address in the wallet signing portal when the Instantiated event didn't exist. Receiving the address in pop-cli.
The event was reintroduced in paritytech/polkadot-sdk#8789
Since the address can now be obtained directly from the events in pop-cli, the manual calculation is no longer necessary in the wallet signing portal and can be removed.

Is basically removing undoing this commit where this was introduced: 3b7e059

The changes in the wallet signing portal that needs to be reviewed too are in: r0gue-io/pop-wallet-signing-portal#12

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the wallet integration to simplify the contract instantiation flow by removing the need to pass a contract address parameter. The contract address is now always extracted from the ContractInstantiated event after successful submission, rather than being optionally provided by the wallet.

Key changes:

  • Simplified instantiate_contract_signed to extract contract address from events only
  • Removed SubmitRequest struct in favor of direct String payload
  • Updated all wallet integration callers to use the simplified API

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/pop-contracts/src/up.rs Removed maybe_contract_address parameter and parse_h160_account import; contract address now always extracted from ContractInstantiated event
crates/pop-cli/src/wallet_integration.rs Removed SubmitRequest struct and contract_address field from StateHandler; simplified submit handler to accept String directly
crates/pop-cli/src/common/wallet.rs Updated request_signature to return Option<String> instead of SubmitRequest; removed unused import
crates/pop-cli/src/commands/up/contract.rs Updated to use simplified request_signature and instantiate_contract_signed APIs
crates/pop-cli/src/commands/call/contract.rs Updated to use simplified request_signature API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 40.90909% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.63%. Comparing base (2a27f8f) to head (3af9db0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/pop-contracts/src/up.rs 0.00% 5 Missing ⚠️
crates/pop-cli/src/commands/up/contract.rs 0.00% 4 Missing ⚠️
crates/pop-cli/src/common/wallet.rs 0.00% 3 Missing ⚠️
crates/pop-cli/src/commands/call/contract.rs 0.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main     #813      +/-   ##
==========================================
+ Coverage   75.61%   75.63%   +0.02%     
==========================================
  Files         115      115              
  Lines       26833    26813      -20     
  Branches    26833    26813      -20     
==========================================
- Hits        20289    20281       -8     
+ Misses       4374     4362      -12     
  Partials     2170     2170              
Files with missing lines Coverage Δ
crates/pop-cli/src/wallet_integration.rs 95.71% <100.00%> (-0.10%) ⬇️
crates/pop-cli/src/commands/call/contract.rs 71.93% <0.00%> (+0.08%) ⬆️
crates/pop-cli/src/common/wallet.rs 11.53% <0.00%> (+0.21%) ⬆️
crates/pop-cli/src/commands/up/contract.rs 14.63% <0.00%> (+0.10%) ⬆️
crates/pop-contracts/src/up.rs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlexD10S AlexD10S changed the title refactor: remove the need to pass contract address refactor: remove the code to receive the contract address from wallet signing portal Dec 3, 2025
Comment thread crates/pop-cli/src/assets/index.html
@AlexD10S AlexD10S marked this pull request as ready for review December 3, 2025 14:06
@moliholy
Copy link
Copy Markdown
Collaborator

moliholy commented Dec 3, 2025

@AlexD10S if using a polkadot-sdk version older than 2509 pallet-revive won't have that commit, isn't it?

@AlexD10S
Copy link
Copy Markdown
Member Author

AlexD10S commented Dec 3, 2025

@AlexD10S if using a polkadot-sdk version older than 2509 pallet-revive won't have that commit, isn't it?

How did you see the 2509? PassetHub I believe has the 2507 and it has the Instantiated event

@moliholy
Copy link
Copy Markdown
Collaborator

moliholy commented Dec 3, 2025

How did you see the 2509?

That PR is from June this year, so it was a mere deduction.

@AlexD10S
Copy link
Copy Markdown
Member Author

AlexD10S commented Dec 4, 2025

That PR is from June this year, so it was a mere deduction.

Ah, I see your point. Since pop-cli already assumes this event exists when deploying a contract, this PR simply mirrors that behavior in the wallet-integration-portal. I think it’s reasonable to assume the chain will have this event.

Copy link
Copy Markdown
Collaborator

@moliholy moliholy left a comment

Choose a reason for hiding this comment

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

Then let's go ahead!

@AlexD10S AlexD10S merged commit ffcc7a9 into main Dec 4, 2025
19 checks passed
@AlexD10S AlexD10S deleted the refactor/wallet-integration-portal branch December 4, 2025 11:56
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.

3 participants