Add new bls operators and an operator compatibility mode

[prozacchiwawa]

--operators-version can now be used in situations where disassembly is output to select an older version of the operator set.

[coveralls-official]

Pull Request Test Coverage Report for Build 5764461447

• 128 of 179 (71.51%) changed or added relevant lines in 17 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 78.559%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/classic/clvm/sexp.rs	5	6	83.33%
src/classic/clvm/mod.rs	10	12	83.33%
src/compiler/compiler.rs	4	6	66.67%
src/classic/clvm_tools/cmds.rs	30	33	90.91%
src/classic/clvm_tools/clvmc.rs	6	10	60.0%
src/classic/clvm_tools/stages/stage_2/optimize.rs	4	14	28.57%
src/classic/clvm_tools/stages/stage_2/operators.rs	21	34	61.76%
src/classic/clvm_tools/stages/stage_2/compile.rs	2	18	11.11%

Totals
Change from base Build 5743833607:	0.2%
Covered Lines:	7306
Relevant Lines:	9300

---

💛 - Coveralls

[prozacchiwawa]

Gonna add tests of the bls operators then set it Ready.

[prozacchiwawa]

ok waiting for green then gonna mark it ready.

[cameroncooper]

I think @arvidn wanted to remove the bls prefix from g1 and g2 ops: https://github.com/Chia-Network/clvm_rs/blob/main/op-tests/test-bls-ops.txt#L136-L267

[prozacchiwawa]

ahh thanks, hadn't heard anything since i saw the definitions in a previous version of clvm_rs. will change them here.

…

On Thu, Jun 22, 2023 at 12:49 PM cameroncooper ***@***.***> wrote: I think @arvidn <https://github.com/arvidn> wanted to remove the bls prefix from g1 and g2 ops: https://github.com/Chia-Network/clvm_rs/blob/main/op-tests/test-bls-ops.txt#L136-L267 — Reply to this email directly, view it on GitHub <https://github.com/Chia-Network/clvm_tools_rs/pull/180#issuecomment-1603228191>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYIEC4CJKGBYZZ3TMZYYDXMSOUHANCNFSM6AAAAAAY4Y2TVI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[freddiecoleman]

Any chance this could be updated to use clvmr 0.2.6 to get the secp operators as well? Thanks.

[prozacchiwawa]

yes, i'll check on what the final opcode for the secp operator turned out to be.

…

On Tue, Jun 27, 2023 at 6:30 AM Freddie Coleman ***@***.***> wrote: Any chance this could be updated to use clvmr 0.2.6 to get the secp operators as well? Thanks. — Reply to this email directly, view it on GitHub <https://github.com/Chia-Network/clvm_tools_rs/pull/180#issuecomment-1609511400>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYIEBLYSBCVPMUP25GQU3XNLN6VANCNFSM6AAAAAAY4Y2TVI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[prozacchiwawa]

ok changed the operator names and added the secp256 operators. requires a clvmr bump which changes clvmr's api just a bit.

[freddiecoleman]

I manually compiled and ran a Chialisp program that uses secp256r1_verify and can confirm it returns () when run with a valid pk/msg/sig and throws FAIL: secp256r1_verify failed when the signature is invalid.

[aqk]

These two comments about opbuf could be more helpful, especially since opbuf is no longer bound.

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-56585dbab0da8912246c3ad09e10672b7e24ae2d8246ab236fe774d06b385b36R131

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-56585dbab0da8912246c3ad09e10672b7e24ae2d8246ab236fe774d06b385b36R143

[aqk]

Much nicer!

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-56585dbab0da8912246c3ad09e10672b7e24ae2d8246ab236fe774d06b385b36R369

[aqk]

Is get_disassembly_ver used everywhere it could / should?

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-56585dbab0da8912246c3ad09e10672b7e24ae2d8246ab236fe774d06b385b36R779

[aqk]

Is removing val_stack_limit connected with this change? If not, it could be a separate PR.

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-2074b76a9ca8bff0a579d5ef151d660b817ccdfab3ece092a64780493ca2e636L312

[aqk]

Is the softfork extension argument mandatory?

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-2074b76a9ca8bff0a579d5ef151d660b817ccdfab3ece092a64780493ca2e636R340

[arvidn]

Is the softfork extension argument mandatory?

technically it's not, in order to be backwards compatible. but we will require it in all our soft-forks, by convention.

[aqk]

For _extension: OperatorSet, is there a way to specify a combination of several OperatorSets?

Or were you thinking "just specify a complete set for each new dialect" ?

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-2074b76a9ca8bff0a579d5ef151d660b817ccdfab3ece092a64780493ca2e636R354

[prozacchiwawa]

regarding the extension set, apologies, that particular thing fell through the cracks. pr here.
https://github.com/Chia-Network/clvm_tools_rs/pull/202

[prozacchiwawa]

added a comment to OPERATORS_LATEST_VERSION.

[prozacchiwawa]

much of the rest of the commentary here was about the clvmr upgrade, which went into a separate pr and got merged.

[aqk]

Line too long!!

https://github.com/Chia-Network/clvm_tools_rs/pull/180/files#diff-1831f1f42490a8fe847e2516a87bd681c15b6883489dffc8a597ef09eaffcae8R2

[arvidn]

For _extension: OperatorSet, is there a way to specify a combination of several OperatorSets?

Or were you thinking "just specify a complete set for each new dialect" ?

At the core of it, we can do whatever we want, since anything is a soft-fork anyway. Regarding the CLVM implementation, and how it interprets this field, it's an enum that essentially specifies the soft-fork version. We don't expect to have a lot of soft-fork in flight in parallel, so a version is enough. We considered a bitmask, but that would limit us to 64 soft-forks (in practice, then we would need a wider type), without really providing any benefit, since later soft-forks are expected to include all features from previous ones.

[prozacchiwawa]

This was changed to pass through the extension we were given at the top. Initially i was thinking: "we should provide a consistent set for compile time", but realized that can change (and should change) according to the user's --operators-version preference. That change was in a separate pr.

…

On Tue, Aug 1, 2023 at 9:04 AM Arvid Norberg ***@***.***> wrote: For _extension: OperatorSet, is there a way to specify a combination of several OperatorSets? Or were you thinking "just specify a complete set for each new dialect" ? At the core of it, we can do whatever we want, since anything is a soft-fork anyway. Regarding the CLVM implementation, and how it interprets this field, it's an enum that essentially specifies the soft-fork version. We don't expect to have a lot of soft-fork in flight in parallel, so a version is enough. We considered a bitmask, but that would limit us to 64 soft-forks (in practice, then we would need a wider type), without really providing any benefit, since later soft-forks are expected to include all features from previous ones. — Reply to this email directly, view it on GitHub <https://github.com/Chia-Network/clvm_tools_rs/pull/180#issuecomment-1660628549>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYIEBHHG4OG4TN35TBIWDXTESHLANCNFSM6AAAAAAY4Y2TVI> . You are receiving this because you authored the thread.Message ID: ***@***.***>