Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete share and TxInclusion code #850

Merged
merged 16 commits into from
Sep 14, 2022

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Sep 12, 2022

Description

Closes: #842

NOTE: This won't work as is because pkg/prove/proof.go relies on types
and functions that were deleted.
@rootulp rootulp self-assigned this Sep 12, 2022
@rootulp rootulp force-pushed the rp/delete-share-code branch from 6d6aaf4 to 4f56734 Compare September 12, 2022 17:56
@rootulp rootulp force-pushed the rp/delete-share-code branch from 469fde2 to 5ed323f Compare September 12, 2022 19:57
@@ -98,7 +98,7 @@ func TestBlockchainMessageVectors(t *testing.T) {
BlockRequest: &bcproto.BlockRequest{Height: math.MaxInt64}}},
"0a0a08ffffffffffffffff7f"},
{"BlockResponseMessage", &bcproto.Message{Sum: &bcproto.Message_BlockResponse{
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a97010a94010a5b0a02080b1803220b088092b8c398feffffff012a0212003a20269ece38583f42aaf53fdd3abe1f570ab9b0d08284d080900966040a29df504c6a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b85512350a0b48656c6c6f20576f726c641a00220028013220269ece38583f42aaf53fdd3abe1f570ab9b0d08284d080900966040a29df504c"},
BlockResponse: &bcproto.BlockResponse{Block: bpb}}}, "1a500a4e0a390a02080b1803220b088092b8c398feffffff012a0212006a20e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b85512110a0b48656c6c6f20576f726c641a002200"},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the expected bytes were updated in this test b/c the data hash in a block was updated

@@ -1039,124 +1027,17 @@ type Data struct {

// Hash returns the hash of the data
func (data *Data) Hash() tmbytes.HexBytes {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rootulp rootulp marked this pull request as ready for review September 12, 2022 20:00
@rootulp rootulp marked this pull request as draft September 12, 2022 20:27
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! left one blocking comment, but the rest are nits or comments

@rootulp
Copy link
Collaborator Author

rootulp commented Sep 12, 2022

Converted back to draft because long tests in replay_test.go are failing. It takes a while to run them (8 to 10 mins) so iteration speed is slow. I suspect there may be a case where the test expects a block's data hash to be set but it's nil, still investigating.

@rootulp rootulp force-pushed the rp/delete-share-code branch from af16e15 to e77731e Compare September 13, 2022 17:02
@rootulp rootulp marked this pull request as ready for review September 13, 2022 19:31
@rootulp rootulp requested a review from evan-forbes September 13, 2022 19:31
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: 🪓

@rootulp rootulp merged commit c268d1b into celestiaorg:v0.34.x-celestia Sep 14, 2022
@rootulp rootulp deleted the rp/delete-share-code branch September 14, 2022 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete share related code
2 participants