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

refactor: remove read state encoding #122

Merged
merged 30 commits into from
Jul 28, 2024

Conversation

Daanvdplas
Copy link
Collaborator

@Daanvdplas Daanvdplas commented Jul 23, 2024

Closes #101

  • Removes the decoding of the read state result
  • refactors the read state implementation by matching on the decoded key in the pallet in stead of the chain extension
  • adds a filter for state queries (useful for the different runtimes)

Contract size from 7.4K to 7.1K

Created an issue to look into state query error handling #123

@Daanvdplas Daanvdplas force-pushed the daan/feat-api_fungibles_pallet branch from 9d2276c to a8c4bb5 Compare July 23, 2024 17:28
@Daanvdplas Daanvdplas force-pushed the daan/refactor-remove_read_state_encoding branch from 0abd499 to 77c37f5 Compare July 23, 2024 17:32
@Daanvdplas Daanvdplas mentioned this pull request Jul 23, 2024
3 tasks
@Daanvdplas Daanvdplas marked this pull request as draft July 23, 2024 19:13
@Daanvdplas Daanvdplas marked this pull request as ready for review July 23, 2024 19:23
@Daanvdplas Daanvdplas force-pushed the daan/refactor-remove_read_state_encoding branch from 5c9e90f to b50830c Compare July 25, 2024 23:31
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

A few minor comments which need addressing.

pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
runtime/devnet/src/extensions/mod.rs Outdated Show resolved Hide resolved
runtime/devnet/src/extensions/mod.rs Outdated Show resolved Hide resolved
runtime/devnet/src/extensions/mod.rs Outdated Show resolved Hide resolved
@evilrobot-01
Copy link
Collaborator

  • refactor: charging weight for state queries

If it will always be one or more reads, we could attach an attribute to the read enum variants which defines the number of reads. That way the extension can just use that static number to precharge the relevant weight, but more importantly it is defined in the pallet. Alternatively it could be an actual weight value, but that might be something for the future.

Base automatically changed from daan/feat-api_fungibles_pallet to daan/api July 26, 2024 09:00
@Daanvdplas Daanvdplas requested a review from evilrobot-01 July 26, 2024 12:16
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

Looks good, has merge conflicts tho.

@Daanvdplas Daanvdplas merged commit 9820e7f into daan/api Jul 28, 2024
6 checks passed
@Daanvdplas Daanvdplas deleted the daan/refactor-remove_read_state_encoding branch July 28, 2024 10:07
chungquantin pushed a commit that referenced this pull request Sep 6, 2024
chungquantin pushed a commit that referenced this pull request Sep 6, 2024
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.

refactor: exclude encoding of reading state results in chain extension
3 participants