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.

Sign up for GitHub

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

Jump to bottom

PublishBlob #2014

Merged
merged 1 commit into from
May 23, 2024
Merged

PublishBlob #2014

merged 1 commit into from
May 23, 2024

Conversation

ndr-ds
Copy link
Contributor

@ndr-ds ndr-ds commented May 10, 2024 •
edited
Loading

Motivation

Currently users are able to upload bytecode that can be used to create applications. Other chains have to subscribe to the publisher chain to be able to use the bytecode. We want to generalize this so that:

  • Users can publish arbitrary data that can be used as bytecode or read by existing applications.
  • One doesn’t have to subscribe to the publisher chain (and wait for it to handle the subscription request) to use the blob.
  • Blobs are content-addressed, so that if two users upload identical blobs, they are stored only once.

Proposal

This PR implements the PublishBlob operation

Test Plan

Unit test

Release Plan

Since we're adding a new SystemOperation, I'm guessing we have to do this:

  • Need to update the developer manual.

@ndr-ds Graphite App
Copy link
Contributor Author

ndr-ds commented May 10, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @andresilva91 and the rest of your teammates on Graphite Graphite

afck
afck reviewed May 13, 2024
View reviewed changes
linera-service/src/linera/client_options.rs Outdated Show resolved Hide resolved
linera-service/src/linera/client_options.rs Outdated Show resolved Hide resolved
linera-base/src/crypto.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
@ndr-ds ndr-ds force-pushed the 05-08-publishblob branch 5 times, most recently from 3255a2b to d32dcc5 Compare May 14, 2024 02:13
@ndr-ds ndr-ds marked this pull request as ready for review May 14, 2024 02:56
@ndr-ds ndr-ds linked an issue May 14, 2024 that may be closed by this pull request
Create an operation PublishBlob that allows publishing a blob while a block is proposed #1991
Closed
@graphite-app Graphite App
Copy link

graphite-app bot commented May 14, 2024

Graphite Automations

"Assign reviewers" took an action on this PR • (05/14/24)

6 reviewers were added to this PR based on Andre da Silva's automation.

ma2bd
ma2bd reviewed May 14, 2024
View reviewed changes
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
afck
afck requested changes May 14, 2024
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

Servers must never trust blob IDs that were submitted by a client. In my view the safest way to do this is to not make them part of the serialized block proposal or certificate request.

linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-base/src/identifiers.rs Outdated Show resolved Hide resolved
linera-chain/src/data_types.rs Outdated Show resolved Hide resolved
linera-core/src/local_node.rs Outdated Show resolved Hide resolved
linera-core/src/updater.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Show resolved Hide resolved
@ndr-ds ndr-ds force-pushed the 05-08-publishblob branch 2 times, most recently from 979622e to 9e44e1a Compare May 15, 2024 02:11
@ndr-ds ndr-ds requested review from afck and ma2bd May 15, 2024 02:22
afck
afck requested changes May 15, 2024
View reviewed changes
linera-storage/src/db_storage.rs Outdated Show resolved Hide resolved
linera-service/src/linera/client_context.rs Outdated Show resolved Hide resolved
linera-service/src/linera/client_context.rs Outdated Show resolved Hide resolved
linera-core/src/data_types.rs Outdated Show resolved Hide resolved
linera-base/src/data_types.rs Outdated Show resolved Hide resolved
linera-base/src/data_types.rs Outdated Show resolved Hide resolved
linera-base/src/data_types.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
linera-core/src/worker.rs Outdated Show resolved Hide resolved
@ndr-ds ndr-ds requested review from christos-h and afck May 16, 2024 01:06
afck
afck reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

We'll also need to store the pending blobs associated with the client's pending block in the wallet, together with the block, to be able to re-propose it.

linera-base/src/data_types.rs Outdated Show resolved Hide resolved
linera-base/src/data_types.rs Outdated Show resolved Hide resolved
linera-core/src/client.rs Show resolved Hide resolved
linera-core/src/client.rs Outdated Show resolved Hide resolved
linera-core/src/client.rs Show resolved Hide resolved
linera-core/src/updater.rs Show resolved Hide resolved
linera-core/src/worker.rs Show resolved Hide resolved
@ndr-ds ndr-ds requested a review from afck May 17, 2024 19:37
afck
afck reviewed May 17, 2024
View reviewed changes
linera-chain/src/manager.rs Outdated Show resolved Hide resolved
afck
afck reviewed May 17, 2024
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

It would be good to test a few cases on a multi-owner chain (can be in a different PR):

  • Client A makes a block proposal with a blob. One validator is down. The other three validated the block but don't confirm it. Now the validator comes back up but another one goes down. Client B (who doesn't have the blob) then re-proposes that block in the next round.
  • Client A proposes a block with a blob, and it gets confirmed, but one of the four validators is down and doesn't learn about it at all. Then that validator comes back up but another goes down (so now only two are up and have the blob). Client B proposes a child of that block.

In both cases client B should succeed, and update the validator as needed.

linera-core/src/client.rs Outdated Show resolved Hide resolved
@ndr-ds ndr-ds mentioned this pull request May 17, 2024
Merged
@afck afck mentioned this pull request May 21, 2024
Merged
@ndr-ds ndr-ds requested review from afck and MathieuDutSik May 21, 2024 23:56
afck
afck approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

Happy to have the tests in a separate PR.

@ndr-ds ndr-ds merged commit be08aad into main May 23, 2024
6 of 7 checks passed
@ndr-ds ndr-ds deleted the 05-08-publishblob branch May 23, 2024 04:11
@ndr-ds ndr-ds mentioned this pull request Jun 21, 2024
Merged
ndr-ds pushed a commit that referenced this pull request Jun 21, 2024
Replace [Blobs|ApplicationBytecodes|ApplicationBytecodesAndBlobs]NotF…
ecc785f 
…ound by ApplicationBytecodesOrBlobsNotFound (#2169)

## Motivation

This cleans up the three error types that were partially introduced in this PR #2014. We don't need the three error types, we can cover all cases with just one

## Proposal

Replace the three errors with one

## Test Plan

CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afck afck afck approved these changes

@Twey Twey Awaiting requested review from Twey

@jvff jvff Awaiting requested review from jvff

@ma2bd ma2bd Awaiting requested review from ma2bd

@christos-h christos-h Awaiting requested review from christos-h

@MathieuDutSik MathieuDutSik Awaiting requested review from MathieuDutSik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create an operation PublishBlob that allows publishing a blob while a block is proposed
5 participants
@ndr-ds @ma2bd @MathieuDutSik @afck @christos-h