Conversation
|
I vaguely remember that there was a discussion in the early days of NEAR Protocol, and we excluded such a host function in a fear that it will be misused and cause more incidents due to contract code relying on testnet vs mainnet behavior. I am not against adding this host function. I just wanted to provide some context I recalled. |
Here are my conclusions from looking at the nearcore code. Tldr I think it is safe to mark this concern as resolved.
Code references: |
|
|
||
| The gas cost MUST be computed as: | ||
|
|
||
| `base + write_register_base + write_register_byte * num_bytes` |
There was a problem hiding this comment.
wouldn't it be better to make the gas cost not dependent on the length of the chain_id string? This way gas consumption on different chains won't vary unnecessarily. And there is no way to abuse the constant cost of this simple getter function.
|
What are the next steps here? On the nearcore side we could ship it rather fast, so it would be great if we could get this approved and available to the developers. |
|
Thank you @mitinarseny for submitting this NEP. I would now request @near/wg-protocol to assign two reviewers т(see expectations below). Just for clarity, Technical Reviewers play a crucial role in scaling the NEAR ecosystem as they provide their in-depth expertise in the niche topic while work group members can stay on guard of the NEAR ecosystem. The discussions may get too deep and it would be inefficient for each WG member to dive into every single comment, so NEAR Developer Governance designed this process that includes subject matter experts helping us to scale by writing a summary with the raised concerns and how they were addressed. Technical Review Guidelines* First, review the proposal within one week. If you have any suggestions that could be fixed, leave them as comments to the author. It may take a couple of iterations to resolve any open comments. * Second, once all the suggestions are addressed, produce a Technical Summary, which helps the working group members make a weighted decision faster. Without the summary, the working group will have to read the whole discussion and potentially miss some details. Technical Summary guidelines: * A recommendation for the working group if the NEP is ready for voting (it could be approving or rejecting recommendation). Please note that this is the reviewer's personal recommendation. * A summary of benefits that surfaced in previous discussions. This should include a concise list of all the benefits that others raised, not just the ones that the reviewer personally agrees with. * A summary of concerns or blockers, along with their current status and resolution. Again, this should reflect the collective view of all commenters, not just the reviewer's perspective. Here is a [nice example](https://github.com//pull/399#issuecomment-1462492128) and a template for your convenience: ``` ### Recommendation Add recommendation ### Benefits * Benefit * Benefit ### Concerns | # | Concern | Resolution | Status | | - | - | - | - | | 1 | Concern | Resolution | Status | | 2 | Concern | Resolution | Status | ``` Please tag the @near/nep-moderators once you are done, so we can move this NEP to the voting stage. Thanks again. |
bowenwang1996
left a comment
There was a problem hiding this comment.
As a working group member, I approve this NEP
|
@gagdiez we discussed this proposal at the monthly Protocol Working Group review meeting. We concluded a separate SME review is not needed for this proposal because it is simple enough that the working group members feel qualified to asses its technical quality directly. Introducing a new host function does increase the complexity of the protocol, however as my analysis above showed, this particular host function is rather low-hanging fruit as the Therefore, as a working group member I vote to approve this proposal. |
|
@bowenwang1996 @birchmd thank you for the review of this NEP! The final question is whether we should make gas costs constant as @IkerAlus suggested. Do you have any thoughts on that? |
|
I agree it makes sense to have the gas cost be constant to simplify things for developers. Conveniently |
|
@birchmd amazing, then for all intended purposes this PR is approved, @mitinarseny please tag me when all minor reviews are finished so I can approve and merge the NEP |
Now that I think about it, making it const might not bring any additional advantages but only complicate the protocol by having one more Nice, we now have a fixed gas cost (per chain). But with the current suggested length-dependent formula it is also fixed, but calculated from existing parameters rather than introducing a new one. What's more, it still doesn't help if you want to deploy your Thus, I believe it might be better to keep things simple and use currently suggested formula: @birchmd Does it make sense to you? |
|
I agree if we can avoid adding a new parameter to the config then that is better. My confusion was that I thought we needed a new parameter for the constant portion of this host function regardless, but I see now it is just using the generic host function cost. @graphite could you confirm if the cost proposed by @mitinarseny is sufficient to cover the resource usage? |
|
I believe the calculation is correct |
|
Great, thanks for the confirmation. Then let's leave the gas cost as @mitinarseny originally specified it. |
|
@gagdiez I guess we've reached the consensus now. Please, feel free to merge the PR 🙏🏻 |
Proposal to add
chain_idas a host function in the runtime.NEP text: https://github.com/mitinarseny/NEPs/blob/feat/chain_id/neps/nep-0638.md