Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Update docs for eager rent collection #10348

Merged
merged 17 commits into from
Jun 10, 2020

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented Jun 1, 2020

Problem

Docs for rent is a bit outdated regarding its timings with the debut of eager rent collection #9527 on mainnet-beta in 1.5 day (epoch 34~).

Ideally, I think the docs update should go first, but the eager rent collection is pursued due to rather urgent security concern and performance degradation due to unbounded live account data growth (#8931).

Summary of Changes

Update the doc for eager rent collection with some bonus clarifications.

Also, note that I intentionally didn't introduce eager-/lazy- rent collection terminology because these are implementation details and named from the historical background. From user's perspective, the eager rent collection looks lazy now, exactly opposite to the naming!

CCs

Any comments are appreciated!

@sakridge: as a reviewer of the actual implementation pr, does this represents the behind intention of the PR well?
@ericlwilliams, @aeyakovenko: regarding recent discussion on economic design for rent, I believe this is exactly what this is implemented in code. Is there any unclear parts the docs still is missing?
@CriesofCarrots: partly related to #10133, originating from #10054, I've added some clarification along with the new mention of eager rent collection. Is there any other omission?

Fixes #10133

@ryoqun ryoqun requested a review from garious June 1, 2020 06:34
@ryoqun
Copy link
Contributor Author

ryoqun commented Jun 2, 2020

Another FYI!:

@jstarry: Do you have something else to add here, regarding #7413 and various comments you made recently for eager rent collection?

@CriesofCarrots
Copy link
Contributor

@ryoqun , it will be great to get the design docs updated.

I also created a stub page here for a developer-focused doc on rent and accounts: https://docs.solana.com/apps/rent
I was hoping we could offer a straightforward, functional introduction to rent here. If you want to take an initial crack at it, that would be great. But if not, let's keep #10133 open.
CC @danpaul000, I think that would fall under your current focus?

@mvines mvines removed the v1.1 label Jun 3, 2020
@ryoqun ryoqun mentioned this pull request Jun 3, 2020
5 tasks
@ryoqun
Copy link
Contributor Author

ryoqun commented Jun 3, 2020

I was hoping we could offer a straightforward, functional introduction to rent here. If you want to take an initial crack at it, that would be great.

Okay, I'll take the initial crack. :)

@ryoqun ryoqun force-pushed the eager-rent-collection-docs branch from 2ef4cdb to e9eea03 Compare June 5, 2020 13:14
@garious garious requested a review from ericlwilliams June 5, 2020 13:23
Copy link
Contributor

@ericlwilliams ericlwilliams left a comment

Choose a reason for hiding this comment

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

Just a couple of comments / questions on the write-up. Thanks!

@ryoqun
Copy link
Contributor Author

ryoqun commented Jun 5, 2020

@ericlwilliams Thanks for reviewing! I've addressed your comments and clarified the text a bit. :)

Thank you so much!

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
@ryoqun ryoqun requested a review from CriesofCarrots June 8, 2020 04:31
@ryoqun
Copy link
Contributor Author

ryoqun commented Jun 8, 2020

@CriesofCarrots I think I've addressed all your comments. :)

Copy link
Contributor

@garious garious left a comment

Choose a reason for hiding this comment

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

No need to block on my review. I haven't had a chance to review in depth, but looks quite detailed! Thanks for writing this up.

ryoqun and others added 2 commits June 9, 2020 18:01
@ryoqun
Copy link
Contributor Author

ryoqun commented Jun 9, 2020

No need to block on my review. I haven't had a chance to review in depth, but looks quite detailed! Thanks for writing this up.

@garious I see!

From @CriesofCarrots:

I also created a stub page here for a developer-focused doc on rent and accounts: https://docs.solana.com/apps/rent
I was hoping we could offer a straightforward, functional introduction to rent here. If you want to take an initial crack at it, that would be great. But if not, let's keep #10133 open.
CC @danpaul000, I think that would fall under your current focus?

@danpaul000 Hi! Do you mind having a look at this pr? If there is no objections, I want to merge this because I think it's getting mature to merge. :)

@danpaul000
Copy link
Contributor

No need to block on my review. I haven't had a chance to review in depth, but looks quite detailed! Thanks for writing this up.

@garious I see!

From @CriesofCarrots:

I also created a stub page here for a developer-focused doc on rent and accounts: https://docs.solana.com/apps/rent
I was hoping we could offer a straightforward, functional introduction to rent here. If you want to take an initial crack at it, that would be great. But if not, let's keep #10133 open.
CC @danpaul000, I think that would fall under your current focus?

@danpaul000 Hi! Do you mind having a look at this pr? If there is no objections, I want to merge this because I think it's getting mature to merge. :)

Looks good to me @ryoqun , thanks for the write-up!

@ryoqun ryoqun merged commit 40ffc56 into solana-labs:master Jun 10, 2020
mergify bot pushed a commit that referenced this pull request Jun 10, 2020
* Update docs for eager rent collection

* Add rent doc and clarify account doc for app devs

* Clarify some and pass the grammarly

* Fix units notation

* Fix link

* Fix link really

* Fix link really really

* More grammarly

* Apply suggestions from code review

Thank you so much!

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Add explanation of 19.055441478439427

* Fix unit...

* Fix unit...

* Clarify rent duration reasoning

* Tweak a text for more clarification

* Tweak more..

* Apply suggestions from code review

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Revert too detailed out-of-context explanations

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
(cherry picked from commit 40ffc56)
solana-grimes pushed a commit that referenced this pull request Jun 10, 2020
danpaul000 pushed a commit to danpaul000/solana that referenced this pull request Jul 8, 2020
* Update docs for eager rent collection

* Add rent doc and clarify account doc for app devs

* Clarify some and pass the grammarly

* Fix units notation

* Fix link

* Fix link really

* Fix link really really

* More grammarly

* Apply suggestions from code review

Thank you so much!

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Add explanation of 19.055441478439427

* Fix unit...

* Fix unit...

* Clarify rent duration reasoning

* Tweak a text for more clarification

* Tweak more..

* Apply suggestions from code review

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>

* Revert too detailed out-of-context explanations

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rent implementation is undocumented
8 participants