From 880367d5df3fc7531067d41aad171a2203dc0ddf Mon Sep 17 00:00:00 2001 From: Thomas E Lackey Date: Wed, 29 Nov 2023 00:05:56 +0000 Subject: [PATCH] 122: Fix attribute index key collision (#123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #122, where the structure of the index key allowed unintended collisions (see below). This also adds a test cases which _fails_ under the old scheme, but passes now: **Before:** ``` --- FAIL: TestKeeperTestSuite (0.31s) --- FAIL: TestKeeperTestSuite/TestGrpcGetRecordLists (0.09s) grpc_query_test.go:143: Error Trace: /home/telackey/cerc/laconicd/x/registry/keeper/grpc_query_test.go:143 /home/telackey/cerc/laconicd/x/registry/keeper/suite.go:91 Error: Not equal: expected: 0 actual : 1 Test: TestKeeperTestSuite/TestGrpcGetRecordLists --- FAIL: TestKeeperTestSuite/TestGrpcGetRecordLists/Case_Filter_with_typ_(https://git.vdb.to/cerc-io/laconicd/issues/122)_ (0.00s) testing.go:1490: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test FAIL FAIL github.com/cerc-io/laconicd/x/registry/keeper 0.765s FAIL make: *** [Makefile:333: run-tests] Error 1 ❯ laconic cns record list --all --typ eWebsiteRegistrationRecord [ { "id": "bafyreies5he2mxyrjso2quewwlmh6cisekkjnf7yijpd2742wrzxgzuazi", "names": null, "owners": [ "FC9B9FB065D70DBB10C8F511348421C16669B37D" ], "bondId": "a9c7161fc154cf44ee61efc699a59c8a27c804bb23ce4c8a7ffa0a39fcc540b1", "createTime": "2023-11-28T19:09:03Z", "expiryTime": "2024-11-27T19:09:03Z", "attributes": { "url": "https://hello-urbit.laconic.com", "repo_registration_record_cid": "QmTZQ8ZJS6mALjEM2wY71msFno6zzxFftVCiZELj9xREPx", "build_artifact_cid": "~lostex-rabdur-labtul-moltev/hello-urbit", "tls_cert_cid": "QmR1acEmQt7Tjmhp9cFtymie2eFcrHURQKt9kGto1TQTW1", "type": "WebsiteRegistrationRecord", "version": "0.2.4" } } ... ❯ laconic cns record list --all --type WebsiteRegistrationRecord [ { "id": "bafyreies5he2mxyrjso2quewwlmh6cisekkjnf7yijpd2742wrzxgzuazi", "names": null, "owners": [ "FC9B9FB065D70DBB10C8F511348421C16669B37D" ], "bondId": "a9c7161fc154cf44ee61efc699a59c8a27c804bb23ce4c8a7ffa0a39fcc540b1", "createTime": "2023-11-28T19:09:03Z", "expiryTime": "2024-11-27T19:09:03Z", "attributes": { "url": "https://hello-urbit.laconic.com", "repo_registration_record_cid": "QmTZQ8ZJS6mALjEM2wY71msFno6zzxFftVCiZELj9xREPx", "build_artifact_cid": "~lostex-rabdur-labtul-moltev/hello-urbit", "tls_cert_cid": "QmR1acEmQt7Tjmhp9cFtymie2eFcrHURQKt9kGto1TQTW1", "type": "WebsiteRegistrationRecord", "version": "0.2.4" } } ... ``` **After:** ``` ok github.com/cerc-io/laconicd/x/registry/keeper 1.573s ❯ laconic cns record list --all --typ eWebsiteRegistrationRecord [] ❯ laconic cns record list --all --type WebsiteRegistrationRecord [ { "id": "bafyreies5he2mxyrjso2quewwlmh6cisekkjnf7yijpd2742wrzxgzuazi", "names": null, "owners": [ "FC9B9FB065D70DBB10C8F511348421C16669B37D" ], "bondId": "a9c7161fc154cf44ee61efc699a59c8a27c804bb23ce4c8a7ffa0a39fcc540b1", "createTime": "2023-11-28T19:09:03Z", "expiryTime": "2024-11-27T19:09:03Z", "attributes": { "url": "https://hello-urbit.laconic.com", "repo_registration_record_cid": "QmTZQ8ZJS6mALjEM2wY71msFno6zzxFftVCiZELj9xREPx", "build_artifact_cid": "~lostex-rabdur-labtul-moltev/hello-urbit", "tls_cert_cid": "QmR1acEmQt7Tjmhp9cFtymie2eFcrHURQKt9kGto1TQTW1", "type": "WebsiteRegistrationRecord", "version": "0.2.4" } } ... ``` Reviewed-on: https://git.vdb.to/cerc-io/laconicd/pulls/123 Co-authored-by: Thomas E Lackey Co-committed-by: Thomas E Lackey --- x/registry/keeper/grpc_query_test.go | 20 +++++++++++++++++++- x/registry/keeper/keeper.go | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/x/registry/keeper/grpc_query_test.go b/x/registry/keeper/grpc_query_test.go index 4e580432..6d5073de 100644 --- a/x/registry/keeper/grpc_query_test.go +++ b/x/registry/keeper/grpc_query_test.go @@ -79,6 +79,24 @@ func (suite *KeeperTestSuite) TestGrpcGetRecordLists() { false, 1, }, + { + "Filter test for key collision (https://git.vdb.to/cerc-io/laconicd/issues/122)", + ®istrytypes.QueryListRecordsRequest{ + Attributes: []*registrytypes.QueryListRecordsRequest_KeyValueInput{ + { + Key: "typ", + Value: ®istrytypes.QueryListRecordsRequest_ValueInput{ + Type: "string", + String_: "eWebsiteRegistrationRecord", + }, + }, + }, + All: true, + }, + true, + false, + 0, + }, { "Filter with attributes ServiceProviderRegistration", ®istrytypes.QueryListRecordsRequest{ @@ -123,7 +141,7 @@ func (suite *KeeperTestSuite) TestGrpcGetRecordLists() { } else { sr.NoError(err) sr.Equal(test.noOfRecords, len(resp.GetRecords())) - if test.createRecords { + if test.createRecords && test.noOfRecords > 0 { recordId = resp.GetRecords()[0].GetId() sr.NotZero(resp.GetRecords()) sr.Equal(resp.GetRecords()[0].GetBondId(), suite.bond.GetId()) diff --git a/x/registry/keeper/keeper.go b/x/registry/keeper/keeper.go index ebeaaa93..f3ec26f1 100644 --- a/x/registry/keeper/keeper.go +++ b/x/registry/keeper/keeper.go @@ -367,7 +367,7 @@ func (k Keeper) ProcessAttributes(ctx sdk.Context, record types.RecordType) erro } func GetAttributesIndexKey(key string, value interface{}) []byte { - keyString := fmt.Sprintf("%s%s", key, value) + keyString := fmt.Sprintf("%s=%s", key, value) return append(PrefixAttributesIndex, []byte(keyString)...) }