chore: clean up the params keeper

[zivkovicmilos]

Description

This PR cleans up a bit the params keeper, and starts a discussion on some functionality that seems sketchy. It moves some testing code outside the binary, and into test files.

Additionally, I've updated some parsing methods so they're quicker now, by ~40ns:

goos: darwin
goarch: arm64
pkg: github.com/gnolang/gno/tm2/pkg/sdk/params
cpu: Apple M3 Max
BenchmarkParamsHandler_Query
BenchmarkParamsHandler_Query-14    	15104251	        79.11 ns/op
PASS

Before:

goos: darwin
goarch: arm64
pkg: github.com/gnolang/gno/tm2/pkg/sdk/params
cpu: Apple M3 Max
BenchmarkParamsHandler_Query
BenchmarkParamsHandler_Query-14    	 6789453	       156.6 ns/op
PASS

The keeper could probably be fully made generic, but for the sake of readability, this was not pursued.

cc @gfanton @moul

Topics that need discussing:

Internal funky returns

gno/tm2/pkg/sdk/params/keeper.go

Lines 118 to 128 in 4f27a57

	func (pk ParamsKeeper) getIfExists(ctx sdk.Context, key string, ptr interface{}) {
	stor := ctx.Store(pk.key)
	bz := stor.Get([]byte(key))
	if bz == nil {
	return
	}
	err := amino.UnmarshalJSON(bz, ptr)
	if err != nil {
	panic(err)
	}
	}

The method is called GetXXX, but it doesn't alter the pointer value internally if the value is missing.

Keeper (store) not being thread safe

The params keeper can be concurrently called, but it's not thread safe, actually. The underlying store is not thread safe

What if different modules concurrently call the keeper to store params?

Contributors' checklist...

• Added new tests, or not needed, or not feasible
• Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
• Updated the official documentation or not needed
• No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
• Added references to related issues and PRs
• Provided any useful hints for running manual tests

[codecov]

Codecov Report

Attention: Patch coverage is 75.78947% with 46 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gno.land/pkg/sdk/vm/keeper.go	36.36%	14 Missing and 7 partials ⚠️
gno.land/pkg/sdk/vm/builtins.go	21.42%	8 Missing and 3 partials ⚠️
tm2/pkg/sdk/params/keeper.go	93.82%	4 Missing and 1 partial ⚠️
gno.land/pkg/sdk/vm/params.go	60.00%	4 Missing ⚠️
gno.land/pkg/gnoland/param.go	81.25%	2 Missing and 1 partial ⚠️
gno.land/pkg/gnoland/app.go	0.00%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[zivkovicmilos]

@moul @gfanton
Ready for your eyes, I needed to get the expected functionality down after changes in #3003

[gfanton]

Looking good 👍

[zivkovicmilos]

@moul
Waiting on your review before merging 🙏

[Gno2D2]

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

• IGNORE the bot requirements for this PR (force green CI check)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[moul]

Started to review, but I need a little bit more time. Blocking to avoid merging without my review.

[zivkovicmilos]

@moul

Please check this PR when you get the chance 🙏

I've integrated changes from previously merged params PRs on master