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

Luiz/KOKU-118 fold #45

Merged
merged 31 commits into from
Apr 27, 2022
Merged

Luiz/KOKU-118 fold #45

merged 31 commits into from
Apr 27, 2022

Conversation

luizalbertocviana
Copy link
Contributor

@luizalbertocviana luizalbertocviana commented Apr 8, 2022

This PR brings a contract illustrating a simple usage of fold over some indexed data stored as a map.
Unit tests are provided.

@luizalbertocviana luizalbertocviana changed the title Luiz/KOKU-118 Luiz/KOKU-118 fold Apr 8, 2022
draft/fold-example.clar Outdated Show resolved Hide resolved
Copy link
Contributor

@ilyabukalov ilyabukalov left a comment

Choose a reason for hiding this comment

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

  1. Please just move it to final version of futuresMarket.clar since it looks like almost complete part of "position updating" in the tech document description. And the rest of business logic will be based on this data structures. So lets start creating basement this way.
  2. Rename accordingly to data structures in our tech description instead of "a", "b" for example it will be get-position-size instead of get-a, get-position-holder instead of get-b, etc.
  3. Minor refactoring if possible, to introduce definition of data structure if possible separately, if language does allow it somehow. It is not complain about workability, the architecture is correct. It is rather about human readability and possible future refactoring (imagine what would happen if you want to introduce the 3rd field "c" in this data structure? you have to go through all the file to make it compilable). But as I said if language does not allow it somehow it is fine, because the data structure itself is correct.

draft/fold-example.clar Outdated Show resolved Hide resolved
@luizalbertocviana
Copy link
Contributor Author

Minor refactoring if possible, to introduce definition of data structure if possible separately, if language does allow it somehow. It is not complain about workability, the architecture is correct. It is rather about human readability and possible future refactoring (imagine what would happen if you want to introduce the 3rd field "c" in this data structure? you have to go through all the file to make it compilable). But as I said if language does not allow it somehow it is fine, because the data structure itself is correct.

That is currently not possible.
And yes, I acknowledge the issues you have raised. Unfortunately the language does not address them, as there is no way of defining type aliases or typedefs as in other languages.

Refer to stacks-network/stacks-core#1171 to see the progress of such a feature proposal.

draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Show resolved Hide resolved
draft/fold-example.clar Show resolved Hide resolved
@luizalbertocviana
Copy link
Contributor Author

luizalbertocviana commented Apr 20, 2022

I got idea as well. I guess it is possible on clarity.
What if we don't reset each time next-indices-to-update. So the array next-indices-to-update will be constant forever (0...100) but map function will just evaluate it into local list variable (not sure in Clarity syntax, I guess it is keyword "let"). So this way we can save writing to storage because it is only local variable in the memory. And as well we can achieve random access (by offset) iterator. because any time you can generate sequence from any index.

Yes, that is a good idea. Let me see if I got it clearly:

  • instead of keeping a contract variable which always holds the next (let's say) 100 indices to update, we introduce a constant like BASE_CHUNK_OFFSET (u0 u1 u2 u3 ...) as an index offset for each position in the next chunk.
  • then we trade a contract variable holding an entire list of indices for a contract variable holding the "next chunk index" we want to update
  • that would enable us to calculate the next indices to update at a function variable, with no storage cost, like map nth-chunk-offset-step BASE_CHUNK_OFFSET where nth-chunk-offset-step would return (+ (* CHUNK_SIZE next-chunk-index) idx) so our "next chunk index" would start at zero.

That is achievable as I have described. Is that the same you have proposed?

@luizalbertocviana
Copy link
Contributor Author

I got idea as well. I guess it is possible on clarity.
What if we don't reset each time next-indices-to-update. So the array next-indices-to-update will be constant forever (0...100) but map function will just evaluate it into local list variable (not sure in Clarity syntax, I guess it is keyword "let"). So this way we can save writing to storage because it is only local variable in the memory. And as well we can achieve random access (by offset) iterator. because any time you can generate sequence from any index.

This is implemented in commits ranging from 7e3018e to 8032337

draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
draft/fold-example.clar Show resolved Hide resolved
draft/fold-example.clar Outdated Show resolved Hide resolved
only should worry about position update and its public callers shoud
be worried only about collecting fees and providing executor rewards
dependency DAG: sources on the top, sinks on the bottom, a contract
must appear only after all of its dependencies.
getter function for retrieving the timestamp of the block in which a
position was updated for the last time
cooldown inside position-maintenance. Also brings further adjustments to update-position
@luizalbertocviana luizalbertocviana merged commit ccba19b into master Apr 27, 2022
@luizalbertocviana luizalbertocviana deleted the luiz/KOKU-118 branch April 27, 2022 12:51
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