feat: implement ext_offchain_index_set_version_1 for wasmer runtime

[edwardmack]

Changes

• Implement ext_offchain_index_set_version_1 for wasmer runtime library.
• The spec states write a key value pair to offchain database in buffered fashion, we've understood this to mean that data should be stored in both persistent and local storage databases.

Tests

Currently this test is skipped, it fails due to exported function not found. Is there a test runtime that has this?

go test -run ^Test_ext_offline_index_set_version_1$ github.com/ChainSafe/gossamer/lib/runtime/wasmer

Issues

• closes implement ext_offchain_index_set_version_1 #1695

Primary Reviewer

• @arijitAD

[EclesioMeloJunior]

what if change from HOST_API_TEST_RUNTIME to NODE_RUNTIME?

[edwardmack]

I tried with NODE_RUNTIME, NODE_RUNTIME_V098, POLKADOT_RUNTIME and DEV_RUNTIME all with same results. I think we need to build a updated test runtime that contains the "rtm_" imports that reflect to call the actual "ext" functions.

[noot]

@EclesioMeloJunior the HOST_API_TEST_RUNTIME is created by the w3f spec team, all it does is directly call the ext_ function that is specified with the rtm_ prefix. however it doesn't contain all the exports we need, so we might need to update it ourselves

[noot]

it looks like neither of these are the raw database:

	localStorage, err := newInMemoryDB(st.DB().Path())
	if err != nil {
		return nil, err
	}

	ns := runtime.NodeStorage{
		LocalStorage:      localStorage,
		PersistentStorage: chaindb.NewTable(st.DB(), "offlinestorage"),
	}

I think you'll need to add a new field to the context that's the BaseDB (ie. non prefixed, peristent db) and then write to that

[edwardmack]

I added BaseDB to NodeStorage of runtime context, let me know if this is how we should handle this.

[noot]

please update to write to BaseDB!

[codecov]

Codecov Report

Merging #1739 (8ae7638) into development (a96f06a) will increase coverage by 0.04%.
The diff coverage is 82.43%.

@@               Coverage Diff               @@
##           development    #1739      +/-   ##
===============================================
+ Coverage        59.60%   59.65%   +0.04%     
===============================================
  Files              186      186              
  Lines            19770    19764       -6     
===============================================
+ Hits             11784    11790       +6     
+ Misses            5968     5959       -9     
+ Partials          2018     2015       -3     

Flag	Coverage Δ
unit-tests	59.65% <82.43%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dot/state/base.go	54.05% <0.00%> (-1.51%)	⬇️
lib/runtime/types.go	0.00% <ø> (ø)
lib/grandpa/grandpa.go	60.41% <33.33%> (+0.47%)	⬆️
dot/core/test_helpers.go	90.47% <57.14%> (-3.04%)	⬇️
dot/sync/test_helpers.go	85.58% <57.14%> (-1.92%)	⬇️
lib/runtime/wasmer/imports.go	42.71% <81.81%> (+0.20%)	⬆️
dot/rpc/subscription/listeners.go	73.57% <95.00%> (+1.90%)	⬆️
dot/digest/digest.go	67.16% <100.00%> (-0.80%)	⬇️
dot/rpc/http.go	80.95% <100.00%> (-0.18%)	⬇️
dot/rpc/modules/api_mocks.go	100.00% <100.00%> (ø)
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad2d85e...8ae7638. Read the comment docs.

[noot]

you can use st.Base for this

[edwardmack]

Yes, replaced with st.Base.

[noot]

would be preferred to use stateSrvc.BaseDB here if stateSrvc != nil

[edwardmack]

Updated.

[noot]

same here

[edwardmack]

Updated.

[noot]

same here

[edwardmack]

Update.

[noot]

why is this updated? I think finalVal is more descriptive, as this isn't an encoded length

[edwardmack]

This was updated because DeepSource was complaining that append was appending to a variable with a different name. Yes, finalVal is more descriptive, I've update to use that instead.

[noot]

looks good, main comments are to update the test helpers to use the stateSrvc.BaseDB instead of spinning up a new one which is going to slow down the tests

[noot]

looks good!

[edwardmack]

This is ready for review.

[EclesioMeloJunior]

lgmt! just this pice of code that repeats in some places, would be nice to create a test function helper that receives a stateSrvc and returns a NodeStorage

[edwardmack]

I created this:

func InitNodeStorage(baseDB runtime.BasicStorage, dataDir string) (runtime.NodeStorage, error) {
    nodeStorage := runtime.NodeStorage{}
    var err error
    if baseDB != nil {
        nodeStorage.BaseDB = baseDB
    } else {
        nodeStorage.BaseDB, err = utils.SetupDatabase(filepath.Join(dataDir, "offline_storage"), false)
    }
    return nodeStorage, err
}

however it seems that I could find a place to put this where it wouldn't cause a cycle import error in one of the files that used it.

[EclesioMeloJunior]

where did you place it? If you could not find a place to put there is no problem to keep the code as it is.

[edwardmack]

Tried using it in core/test_helpers.go, then sync/test_helpers.go and babe/babe_test.go

[edwardmack]

Can someone override the requirement for DeepSource on this PR, it's complaining that there are unused parameters for the methods in wamer/imports.go, which is true, however when I try to remove them I got these errors when compiling:

_cgo_export.c:67:61: error: conflicting types for ‘_’
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                                     ~~~~~~~~^
_cgo_export.c:67:50: note: previous definition of ‘_’ was here
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                            ~~~~~~^
_cgo_export.c:67:72: error: redefinition of parameter ‘_’
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                                                ~~~~~~~~^
_cgo_export.c:67:61: note: previous definition of ‘_’ was here
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                                     ~~~~~~~~^
_cgo_export.c:67:83: error: redefinition of parameter ‘_’
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                                                           ~~~~~~~~^
_cgo_export.c:67:72: note: previous definition of ‘_’ was here
   67 | void ext_transaction_index_index_version_1(void* _, int32_t _, int32_t _, int32_t _)
      |                                                                ~~~~~~~~^
_cgo_export.c: In function ‘ext_transaction_index_index_version_1’:

...

[github-actions]

🎉 This PR is included in version 0.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀