fix: don't misinterprent alt_bn128_g1 costs as action costs

[matklad]

Before this PR, we store costs for alt_bn128_g1 and costs of certain actions in the same array index, becaues, when adding alt_bn128_g1, we forgot to update the number of costs. The fix is to make this mistake impossible, by including an explicit sentinel node at the end.

Test plan

Add sanity check to existing ext tests that they don't increase action costs.

[frol]

This hack LGTM, but I would love to use strum::EnumCount (it is an extra dependency for primitives-core, so it might be problematic; it is clearly not a problem for nearcore since we use strum elsewhere, but it might be problematic for non-std environments, e.g. for near-sdk-rs)

[matklad]

This hack LGTM, but I would love to use strum::EnumCount (it is an extra dependency for primitives-core, so it might be problematic; it is clearly not a problem for nearcore since we use strum elsewhere, but it might be problematic for non-std environments, e.g. for near-sdk-rs)

I personally try to avoid such "convenience" proc macros: for me, they actually hinder readability and understandability of the code, not to mention compile time implications. In rust-analyzer, we use only serde and salsa procedural macros and regularly pester our deps to remove gratuitous use of proc macros. Though, some of my distaste for proc-macros stems from the fact that they used to be poorly supported by IDEs for a long time :)

nearcore however seems to embrace convenience dependencies much more enthusiastically than I am, and I am not yet ready to fight the battle for that (I am not even sure it is an objective problem, and not my personal taste). So, for the time being, I agree that I should make use of strum, derive more and such.

However, in this particular case, primiteves-core is a published crate and a public API (as it is part of sdk). For libraries, I think keeping the number of deps low is objectively good, as library dep is also imposed on all of the consumers of the library. So it seems like the __count solution is the right one in this particular case.

[ailisp]

What is NB?

[matklad]

Note Bene. I guess it's better to change this to Note.

[frol]

TL;DR: let's merge this fix.

I personally try to avoid such "convenience" proc macros

I prefer dedicated tools with clear naming that address a specific problem we have with a descriptive name and enforces proper use. This implementation requires non-zero attention to detail.

Though, some of my distaste for proc-macros stems from the fact that they used to be poorly supported by IDEs for a long time :)

I read code to understand it, and I only use IDEs where I cannot follow the logic and need to jump around. A dedicated derive implementation is 1 line that I need to read whereas such IDE workarounds only bring more code to read to me.

However, in this particular case, primiteves-core is a published crate and a public API (as it is part of sdk). For libraries, I think keeping the number of deps low is objectively good, as library dep is also imposed on all of the consumers of the library. So it seems like the __count solution is the right one in this particular case.

This concern is valid, though I would say it falls into the optimization space where you need to measure first, and keep measuring the important metrics with all the future changes to the code, otherwise it is only a matter of taste, and I am not going to argue the taste decisions unless they are intrusive.