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

feat: add subscribe flag and event listener #448

Closed
wants to merge 3 commits into from

Conversation

neutrinoks
Copy link
Contributor

@neutrinoks neutrinoks commented Oct 14, 2024

Description

This PR attacks an existing bug in the polkadot-SDK, where subscribing to finalized block events won't be returned in any case. A flag has also been added to decide whether one wants to wait for the finalization of the sent extrinsic.

To workaround the existing bug mentioned in paritytech/subxt#1668, we are now manually listening to events and filtering them for the expected event. A couple of conversions are obsolete by returning the expected event directly. Contained event data can be accessed directly.

Before, one always had to wait for the finalization of the sent extrinsic. Now, one can choose between sending only (not waiting for the extrinsic's result) and waiting for finalization and returning the event and block hash.

Extensive testing on these changes is too much and heavy for one PR - therefore, it has been skipped. The new wait-for-finalisation-flag and the new CLI option --skip-finalisation have been evaluated manually. See follow-up tasks.

maat's real_world_use_case has also been fixed until the step about submitting the windowed PoSt. See #477.

There are multiple follow-up tasks:

Solves: #415, #381.

Checklist

  • Make sure that you described what this change does.
  • Are there any follow-up tasks? See description above.
  • Have you tested this solution?
    • Yes and no, see description.
  • Were there any alternative implementations considered?

@neutrinoks neutrinoks linked an issue Oct 14, 2024 that may be closed by this pull request
@neutrinoks neutrinoks self-assigned this Oct 14, 2024
@neutrinoks neutrinoks added bug Something isn't working enhancement New feature or request labels Oct 14, 2024
@neutrinoks neutrinoks added this to the Phase 2 milestone Oct 14, 2024
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 5 times, most recently from 87897f4 to 7a1d9a1 Compare October 20, 2024 06:58
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 3 times, most recently from 55aedbc to 50fbf67 Compare October 23, 2024 11:54
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 2 times, most recently from d98a2d6 to 7f0ba40 Compare October 23, 2024 12:30
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

The current PR is much larger in scope in comparison with the issue.

The ideas are clearly here, but the PR needs to be trimmed down.

cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 3 times, most recently from 2eb965e to 0ea6109 Compare October 24, 2024 04:40
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 3 times, most recently from 18f45b0 to 75a4435 Compare October 24, 2024 14:18
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

Needs to be drastically simplified.

runtime/src/configs/mod.rs Outdated Show resolved Hide resolved
cli/polka-storage-provider/server/src/rpc.rs Show resolved Hide resolved
cli/polka-storage/storagext-cli/src/main.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
cli/polka-storage/storagext-cli/src/cmd/market.rs Outdated Show resolved Hide resolved
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 2 times, most recently from 504a2ae to f38ed52 Compare October 29, 2024 07:37
Cargo.toml Show resolved Hide resolved
cli/polka-storage/storagext/src/runtime/client.rs Outdated Show resolved Hide resolved
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch 8 times, most recently from 0ca4fb1 to 4c5add8 Compare October 31, 2024 05:11
@neutrinoks neutrinoks added ready for review Review is needed and removed ready for review Review is needed labels Oct 31, 2024
@neutrinoks neutrinoks enabled auto-merge (squash) October 31, 2024 07:40
@neutrinoks neutrinoks changed the title feat: add submission result type flag feat: add subscribe flag and event listener Oct 31, 2024
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch from 4c5add8 to 2c25b81 Compare October 31, 2024 11:42
@neutrinoks neutrinoks added ready for review Review is needed and removed ready for review Review is needed labels Oct 31, 2024
@neutrinoks neutrinoks force-pushed the feat/428/storagext-add-subscribe-flag branch from 2c25b81 to 3dee807 Compare November 4, 2024 04:36
@jmg-duarte jmg-duarte force-pushed the feat/428/storagext-add-subscribe-flag branch 2 times, most recently from 1e1919f to 3dee807 Compare November 4, 2024 16:08
auto-merge was automatically disabled November 4, 2024 18:36

Pull request was closed

@jmg-duarte jmg-duarte deleted the feat/428/storagext-add-subscribe-flag branch November 12, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[storagext] Add subscribe flag instead of always waiting for success
4 participants