Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Add StateFraudProof and BadEncodingFraudProof to networking.md #190

Merged
merged 40 commits into from
Sep 6, 2021

Conversation

nusret1996
Copy link
Contributor

Added struct for state fraud proofs. To see how the data types within the struct will be used to verify state fraud proofs, see the link https://docs.google.com/document/d/1do1_CMiiMFU9se9nXN9uIrQLN3GqcyeEAjZcwSUBx2w/edit?usp=sharing.

Added struct for state fraud proofs. To see how the data types within the struct will be used to verify state fraud proofs, see the link https://docs.google.com/document/d/1do1_CMiiMFU9se9nXN9uIrQLN3GqcyeEAjZcwSUBx2w/edit?usp=sharing.
@nusret1996 nusret1996 closed this Jul 20, 2021
@nusret1996 nusret1996 reopened this Jul 20, 2021
@nusret1996
Copy link
Contributor Author

Data type for the StateFraudProof is now ready for review.

Separated headers[ ] into headerForTxns and headerForISRs.
Fixed the type for intermediateStateElements.
@adlerjohn

This comment has been minimized.

@nusret1996 nusret1996 changed the title Update data_structures.md Add StateFraudProof to data_structures.md Jul 24, 2021
@nusret1996

This comment has been minimized.

src/specs/data_structures.md Outdated Show resolved Hide resolved
src/specs/data_structures.md Outdated Show resolved Hide resolved
src/specs/data_structures.md Outdated Show resolved Hide resolved
src/specs/data_structures.md Outdated Show resolved Hide resolved
@nusret1996
Copy link
Contributor Author

Commits above address the reviews made by John and (i) adds the new heading to ToC, (ii) ends sentences with '.', (iii) fixes the issues around 'isCol'.

@nusret1996 nusret1996 changed the title Add StateFraudProof to data_structures.md Add StateFraudProof and BadEncodingFraudProof to networking.md Jul 26, 2021
StateFraudProof added to networking.md.
BadEncodingFraudProof added to networking.md
Each row  encapsulates the text with no whitespace padding.
Fixed the links to ./data_structures.md.
Added whitespace padding.
Padding fixed on networking.md
@musalbas
Copy link
Member

Where in the specs would the logic for verifying these proofs sit?

Changed the name of 'stateShareProofs' to 'isrShareProofs'.
@nusret1996
Copy link
Contributor Author

Shall I create a new file in the specs to contain the pseudocodes for the verify and create methods of 'StateFraudProof's and 'BadEncodingFraudProof's since these pseudocodes are part of my ToDo list for the next week?

@adlerjohn
Copy link
Member

Hmm. For now the verification pseudocode can be placed under the respective data structure definitions IMO. If it looks messy or too busy, they can be moved.

src/specs/networking.md Outdated Show resolved Hide resolved
Removed trailing spaces.
Added references to the protobuf definitions for the fraud proofs.
@nusret1996
Copy link
Contributor Author

Shall I add protobuf references for only the fraud proofs or shall I add them for every data structure that are used within the fraud proofs and defined elsewhere, e.g NamespaceMerkleTreeInclusionProof? In this case, I will also have to add a comment above their protobuf definitions stating that they are an anchor.

@nusret1996
Copy link
Contributor Author

nusret1996 commented Aug 2, 2021

Created PR#192 for adding protobuf definitions.

@adlerjohn
Copy link
Member

To avoid a circular dependency, I would just transclude #192 into this PR. Otherwise, #192 depends on this PR to define the structures, and this PR depends on #192 to have the proto files to include inline. Generally, PRs should be conceptually encapsulated, and not separated by files. A single PR could touch multiple files.

Edited types.proto.
Added protobuf definitions for all Celestia state elements.
Fixed anchor notation.
@nusret1996
Copy link
Contributor Author

Latest state of the PRs:

  • Adding protobuf definitions to types.proto to support fraud proofs #192 closed and the changes there are included in this PR Add StateFraudProof and BadEncodingFraudProof to networking.md #190.
  • Protobuf definitions for state elements, contents of the fraud proofs and fraud proofs themselves are added to types.proto.
  • Shall I put links for all of the data structures in data_structures.md and networking.md pointing at the respective protobuf definitions like I did for the fraud proofs or is this not so necessary, i.e 'anchor' them within the proto file?
  • Inside the StateElement data structure/protobuf definition, I propose to add a third field for an array of MessagePaid. It seems like these structures are not part of any state element, e.g MessagePaidHead only contains the transaction hash at the tip of the list. Does this make sense or are those already included by some state element that I have missed?

@adlerjohn
Copy link
Member

Shall I put links for all of the data structures in data_structures.md and networking.md pointing at the respective protobuf definitions like I did for the fraud proofs or is this not so necessary, i.e 'anchor' them within the proto file?

It's not necessary at this time. If we do it it should be for all data structures, sometime in the future.

Inside the StateElement data structure/protobuf definition, I propose to add a third field for an array of MessagePaid. It seems like these structures are not part of any state element, e.g MessagePaidHead only contains the transaction hash at the tip of the list. Does this make sense or are those already included by some state element that I have missed?

I'm a bit confused. Why is messages paid a separate field? Why not just have it be the same as everything else in the state.

https://github.com/nusret1996/celestia-specs/blob/610cdc72510ea940a8aa3e1ade0edd44063ccaf4/src/specs/proto/types.proto#L232

i.e.

message StateElement {
  // key of the state element
  // 32-bytes
  bytes key = 1;
  // value can be of different types depending on the state element.
  // There exists a unique protobuf for different state elements, given at the bottom of this file.
  oneof value = {
    Account account = 2;
    Delegation delegation = 3;
    Validator validator = 4;
    ActiveValidatorCount activeValidatorCount = 5;
    ActiveVotingPower activeVotingPower = 6;
    ProposerBlockReward proposerBlockReward = 7;
    ProposerInitialVotingPower proposerInitialVotingPower = 8;
    ValidatorQueueHead validatorQueueHead = 9;
    MessagePaid messagePaid = 10; 
    MessagePaidHead messagePaidHead = 11;
  }
}

Included MessagePaid inside the oneof.
@nusret1996
Copy link
Contributor Author

Included the MessagePaid inside oneof. The next item on my agenda is to add the pseudocodes for create and verify methods of fraud proofs to networking.md.

Validator.proto added to the specs. Validator type is defined here and is used by the state fraud proof type defined in types.proto.
Validator type commented out.
This is Tendermint related and will be deleted from celestia-specs.
Validator name changed to StateValidator to avoid confusion with Tendermint's validator. Type for block height changed to uint64 from int64.
Places where block height is used are updated to reflect the latest changes to uint64.
Reverted height back to int64 from uint64 as talked before.
@adlerjohn adlerjohn merged commit ac81082 into celestiaorg:master Sep 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants