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: update ResourceConfig to match Rootstock #10

Open
wants to merge 1 commit into
base: rsk/poc-v0
Choose a base branch
from

Conversation

illuque
Copy link

@illuque illuque commented Feb 20, 2024

Update ResourceConfig to match Rootstock

Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@illuque It's not clear to me how we extracted the new values. Could we please add a clarification on them? Furthermore, I've also noticed that the same parameters are repeated in different parts. Was there any way to define them only once?

Copy link
Member

@jurajpiar jurajpiar left a comment

Choose a reason for hiding this comment

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

LGTM
(I would not bother extracting the values as it would have to be in env vars or some other project-wide config and this is just a POC)

@antomor
Copy link
Collaborator

antomor commented Mar 5, 2024

(I would not bother extracting the values as it would have to be in env vars or some other project-wide config and this is just a POC)

It's ok for me for the scope of the PoC not extracting them, but I'd at least document how we got them

@illuque
Copy link
Author

illuque commented Mar 5, 2024

@illuque It's not clear to me how we extracted the new values. Could we please add a clarification on them? Furthermore, I've also noticed that the same parameters are repeated in different parts. Was there any way to define them only once?

Values were calculated in a proportional way to our block size compared to theirs as agreed during team discusions. But I will document it, good point.

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