chore!: rename qgb to blobstream #2639

rach-id · 2023-10-09T12:26:35Z

Overview

Checklist

New and updated code has appropriate documentation
New and updated code has new and/or updated testing
Required CI checks are passing
Visual proof for any user facing features like CLI or documentation updates
Linked issues closed with keywords

rach-id · 2023-10-09T12:29:09Z

proto/celestia/blobstream/v1/tx.proto


 import "gogoproto/gogo.proto";
 import "cosmos_proto/cosmos.proto";
 import "google/api/annotations.proto";

-option go_package = "github.com/celestiaorg/celestia-app/x/qgb/types";
+option go_package = "github.com/celestiaorg/celestia-app/x/blobstream/types";


[note to reviewers]
This is API breaking

evan-forbes · 2023-10-09T12:29:45Z

proto/celestia/blobstream/v1/tx.proto

-package celestia.qgb.v1;
+package celestia.blobstream.v1;


since this is consensus breaking, we should not backport this change

why? we don't want to include the API breaking change in v1? I think it's okey as it won't require any hardspoon or so, just update the binary, no?

the path is included in the msg name, so changes this will result in different bytes included in any transactions or state. nodes with this change will know how to parse these txs, where nodes without the change will not

therefore this can only be included in v2. It will also require a migration script, which I think we should include in this change. That likely means waiting for the upgrade and e2e code to be finished so that we can properly test it.

Recall our conversaions about not being able to change the name of the qgb in the code before mainnet

so changes this will result in different bytes included in any transactions or state. nodes with this change will know how to parse these txs, where nodes without the change will not

This provided people are sending register evm address transactions on mocha, which I assume is not the case. If not, then I can revert the tx related changes and only leave the query ones.

For the store, I kept the same keys so it shouldn't be a problem.

codecov-commenter · 2023-10-09T12:31:29Z

Codecov Report

Merging #2639 (ba08ef1) into main (c9f681d) will not change coverage.
The diff coverage is 9.75%.

@@           Coverage Diff           @@
##             main    #2639   +/-   ##
=======================================
  Coverage   21.11%   21.11%           
=======================================
  Files         138      138           
  Lines       15767    15767           
=======================================
  Hits         3329     3329           
  Misses      12115    12115           
  Partials      323      323

Files	Coverage Δ
app/version.go	`50.00% <ø> (ø)`
x/blobstream/abci.go	`59.55% <100.00%> (ø)`
x/blobstream/genesis.go	`0.00% <ø> (ø)`
x/blobstream/keeper/keeper.go	`69.23% <ø> (ø)`
x/blobstream/keeper/keeper_attestation.go	`66.66% <ø> (ø)`
x/blobstream/keeper/keeper_data_commitment.go	`50.44% <ø> (ø)`
x/blobstream/keeper/keeper_valset.go	`44.73% <ø> (ø)`
x/blobstream/keeper/msg_server.go	`70.00% <ø> (ø)`
x/blobstream/keeper/query_attestation.go	`0.00% <ø> (ø)`
x/blobstream/keeper/query_data_commitment.go	`0.00% <ø> (ø)`
... and 24 more

rach-id · 2023-10-11T17:29:57Z

I will update this PR not to include any consensus breaking change, so that we can merge it.
Then, we leave the consensus breaking changes to until we implement the upgrade handlers.

cc @evan-forbes

# Conflicts: # cmd/celestia-appd/cmd/root.go # test/util/testnode/full_node.go # x/blobstream/handler.go # x/blobstream/types/query.pb.go

Co-authored-by: Josh Stein <[email protected]>

Co-authored-by: Sanaz Taheri <[email protected]>

Co-authored-by: Josh Stein <[email protected]>

cmwaters

LGTM.

Can we add a quick note in the README.md that this was formerly known as the qgb module (just for posterity)

rach-id · 2023-10-13T13:00:57Z

Can we add a quick note in the README.md that this was formerly known as the qgb module (just for posterity)

We're planning to write a short ADR documenting this, and this we can maybe add a small section in the README referencing the ADR.

scripts/single-node.sh

scripts/build-run-single-node.sh

…ename-qgb-to-blobstream

## Overview The dependencies part of #2639 ## Checklist  - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords

## Overview The dependencies part of #2639 ## Checklist  - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords (cherry picked from commit c517bd2) # Conflicts: # go.mod # x/qgb/client/verify.go

## Overview The dependencies part of celestiaorg/celestia-app#2639 ## Checklist  - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords

chore: rename qgb to blobstream

cf0e30e

rach-id added x/qgb backport:v1.x PR will be backported automatically to the v1.x branch upon merging x/blobstream item is directly relevant to the blob module labels Oct 9, 2023

rach-id self-assigned this Oct 9, 2023

rach-id requested review from evan-forbes, liamsi, adlerjohn, rootulp and cmwaters as code owners October 9, 2023 12:26

celestia-bot requested a review from a team October 9, 2023 12:26

rach-id commented Oct 9, 2023

View reviewed changes

rach-id added the warn:api breaking item will be break an API and require a major bump label Oct 9, 2023

chore: proto-gen

3576c6a

evan-forbes reviewed Oct 9, 2023

View reviewed changes

celestia-bot requested a review from a team October 9, 2023 12:30

evan-forbes added consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules and removed backport:v1.x PR will be backported automatically to the v1.x branch upon merging labels Oct 9, 2023

chore: gofumpt

7abda29

rach-id changed the title ~~chore: rename qgb to blobstream~~ chore!: rename qgb to blobstream Oct 9, 2023

rootulp modified the milestones: Post-mainnet, v2 Oct 9, 2023

rach-id added 2 commits October 11, 2023 19:43

Merge branch 'main' into rename-qgb-to-blobstream

1e922d7

# Conflicts: # cmd/celestia-appd/cmd/root.go # test/util/testnode/full_node.go # x/blobstream/handler.go # x/blobstream/types/query.pb.go

chore: revert consensus breaking changes

a27b5a0

rach-id changed the title ~~chore!: rename qgb to blobstream~~ chore: rename qgb to blobstream Oct 11, 2023

rach-id removed consensus breaking modifies block validity rules in a way that will break consensus unless all nodes update their rules warn:api breaking item will be break an API and require a major bump labels Oct 11, 2023

Update app/test/qgb_rpc_test.go

57888af

Co-authored-by: Josh Stein <[email protected]>

rach-id dismissed staheri14’s stale review via 57888af October 13, 2023 09:50

celestia-bot requested a review from a team October 13, 2023 09:50

rach-id and others added 4 commits October 13, 2023 11:51

Update x/blobstream/client/tx.go

76a6adf

Co-authored-by: Sanaz Taheri <[email protected]>

Update x/blobstream/integration_test.go

6b8522c

Co-authored-by: Josh Stein <[email protected]>

Update x/blobstream/types/valset.go

d4e6dd3

Co-authored-by: Josh Stein <[email protected]>

Update x/blobstream/keeper/keeper.go

deddb7b

cmwaters previously approved these changes Oct 13, 2023

View reviewed changes

jcstein self-requested a review October 13, 2023 14:51

jcstein approved these changes Oct 13, 2023

View reviewed changes

staheri14 previously approved these changes Oct 17, 2023

View reviewed changes

Merge branch 'main' into rename-qgb-to-blobstream

478e397

celestia-bot requested a review from a team October 17, 2023 20:20

rach-id commented Oct 17, 2023

View reviewed changes

scripts/single-node.sh Outdated Show resolved Hide resolved

rach-id dismissed stale reviews from staheri14 and cmwaters via a93181f October 17, 2023 22:30

Update scripts/single-node.sh

a93181f

rach-id commented Oct 17, 2023

View reviewed changes

scripts/build-run-single-node.sh Outdated Show resolved Hide resolved

rach-id added 3 commits October 18, 2023 00:31

Update scripts/build-run-single-node.sh

1edbd01

chore: bstream => blobstream

a493b88

Merge remote-tracking branch 'origin/rename-qgb-to-blobstream' into r…

47ed3c8

…ename-qgb-to-blobstream

staheri14 approved these changes Oct 17, 2023

View reviewed changes

rach-id mentioned this pull request Oct 17, 2023

chore(deps): replace old qgb contracts repo with blobstream repo #2709

Merged

5 tasks

rach-id enabled auto-merge (squash) October 17, 2023 22:51

cmwaters approved these changes Oct 18, 2023

View reviewed changes

rach-id merged commit 9cefa0d into celestiaorg:main Oct 18, 2023
26 of 27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore!: rename qgb to blobstream #2639

chore!: rename qgb to blobstream #2639

rach-id commented Oct 9, 2023

rach-id Oct 9, 2023

evan-forbes Oct 9, 2023 •

edited

Loading

rach-id Oct 9, 2023

evan-forbes Oct 9, 2023

evan-forbes Oct 9, 2023 •

edited

Loading

rach-id Oct 9, 2023 •

edited

Loading

codecov-commenter commented Oct 9, 2023 •

edited

Loading

rach-id commented Oct 11, 2023

cmwaters left a comment

rach-id commented Oct 13, 2023

		package celestia.qgb.v1;
		package celestia.blobstream.v1;

chore!: rename qgb to blobstream #2639

chore!: rename qgb to blobstream #2639

Conversation

rach-id commented Oct 9, 2023

Overview

Checklist

rach-id Oct 9, 2023

Choose a reason for hiding this comment

evan-forbes Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

rach-id Oct 9, 2023

Choose a reason for hiding this comment

evan-forbes Oct 9, 2023

Choose a reason for hiding this comment

evan-forbes Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

rach-id Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Oct 9, 2023 • edited Loading

Codecov Report

rach-id commented Oct 11, 2023

cmwaters left a comment

Choose a reason for hiding this comment

rach-id commented Oct 13, 2023

evan-forbes Oct 9, 2023 •

edited

Loading

evan-forbes Oct 9, 2023 •

edited

Loading

rach-id Oct 9, 2023 •

edited

Loading

codecov-commenter commented Oct 9, 2023 •

edited

Loading