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

Make CheckNonce refuse transactions signed by accounts with no providers #1578

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Make CheckNonce refuse transactions signed by accounts with no providers #1578

merged 2 commits into from
Oct 10, 2023

Conversation

zdave-parity
Copy link
Contributor

See #1453.

@zdave-parity zdave-parity requested review from a team September 14, 2023 18:12
bkchr
bkchr approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Please add a test.

@bkchr bkchr added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 26, 2023
@zdave-parity zdave-parity force-pushed the nonce-require-provider branch from 3d637c3 to a54a5ff Compare October 4, 2023 12:45
@zdave-parity zdave-parity force-pushed the nonce-require-provider branch from a54a5ff to aca2eba Compare October 5, 2023 11:16
@zdave-parity zdave-parity force-pushed the nonce-require-provider branch from aca2eba to 7589baa Compare October 5, 2023 12:41
zdave-parity
zdave-parity commented Oct 5, 2023
View reviewed changes
substrate/client/service/test/src/client/mod.rs
let mut header = Header {
parent_hash,
number,
state_root,
extrinsics_root,
state_root: Default::default(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AIUI state_root and extrinsics_root are not used by Core_initialize_block and should just be set to 0 here.

zdave-parity
zdave-parity commented Oct 5, 2023
View reviewed changes
substrate/client/service/test/src/client/mod.rs
vec![Transfer {
from: AccountKeyring::Alice.into(),
from: AccountKeyring::One.into(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alice has no balance in the genesis state. As this test runtime has no transaction fees I think this was just resulting in the transaction failing. With the CheckNonce change the transaction was invalid and causing the runtime to panic.

@bkchr bkchr enabled auto-merge (squash) October 9, 2023 20:26
@bkchr bkchr merged commit 93d9c8c into paritytech:master Oct 10, 2023
7 of 8 checks passed
@xlc
Copy link
Contributor

xlc commented Oct 10, 2023

This will make #1725 harder

@bkchr
Copy link
Member

bkchr commented Oct 10, 2023

This will make #1725 harder

Good point, but then we need to make it configurable for #1725.

ordian added a commit that referenced this pull request Oct 12, 2023
@ordian
Merge branch 'master' into ao-disabling-statement-distribution
e333fa8 
* master: (33 commits)
  ci: set CI_IMAGE back to (now updated) .ci-unified (#1854)
  ci: bump ci image to rust 1.73.0 (#1830)
  Refactor Identity to benchmark v2 (#1838)
  PVF worker: bump landlock, update ABI docs (#1850)
  Xcm emulator nits (#1649)
  Fixes path issue in derive-impl (#1823)
  upgrade to macro_magic 0.4.3 (#1832)
  Use safe math when pruning statuses (#1835)
  remote-ext: fix state download stall on slow connections and reduce memory usage (#1295)
  Update testnet bootnode dns name (#1712)
  [FRAME] Warn on unchecked weight witness (#1818)
  [xcm] Use `Weight::MAX` for `reserve_asset_deposited`, `receive_teleported_asset` benchmarks (#1726)
  Update bridges subtree (#1803)
  Check for parent of first ready block being on chain (#1812)
  Make CheckNonce refuse transactions signed by accounts with no providers (#1578)
  Fix Asset Hub collator crashing when starting from genesis (#1788)
  Mixnet integration (#1346)
  [xcm-emulator] Decouple the `AccountId` type from `AccountId32` (#1458)
  Treasury spends various asset kinds (#1333)
  chore: bump zombienter version (#1806)
  ...
kayabaNerve added a commit to serai-dex/polkadot-sdk that referenced this pull request Nov 6, 2023
@kayabaNerve
Revert "Make CheckNonce refuse transactions signed by accounts with n…
c2750ba 
…o providers (paritytech#1578)"

This reverts commit 93d9c8c.

Serai's coin-pallets doesn't set a provider as we plan to rip out these
systems. Accordingly, this error would prevent users from submitting
transactions.
@kiltbot kiltbot mentioned this pull request Nov 7, 2023
Closed
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v1-3-0/4614/1

@ahmadkaouk ahmadkaouk mentioned this pull request Nov 16, 2023
Closed
CertainLach added a commit to UniqueNetwork/pallet-sponsoring that referenced this pull request Nov 21, 2023
@CertainLach
feat: provide CheckNonce compatible with sponsoring
6071a67 
Broken in upstream in paritytech/polkadot-sdk#1578
@zdave-parity zdave-parity deleted the nonce-require-provider branch December 22, 2023 13:54
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@zdave-parity @bkchr
Make CheckNonce refuse transactions signed by accounts with no provid…
60e6c5e 
…ers (paritytech#1578)

See paritytech#1453.

Co-authored-by: Bastian Köcher <git@kchr.de>
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@svyatonik @bkchr
Do not wait for tx status forever (#1578)
415f28f 
* do not wait for tx status forever

* more logging
@breathx breathx mentioned this pull request May 16, 2024
Merged
This was referenced Jun 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zdave-parity @xlc @bkchr @Polkadot-Forum @ggwpez