This repository has been archived by the owner. It is now read-only.

/Sia

Consensus blocks api #2046

Closed
wants to merge 9 commits into
base: master
from

Conversation

Reviewers

@DavidVorick DavidVorick

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@S-anasol @DavidVorick @tbenz9
@S-anasol
Contributor

S-anasol commented Jun 28, 2017 •
edited

Mini real-time blocks explorer.

Since internal block explorer unusable, we need at least possibility to get 'raw' blocks from siad.
I know that explorer have similar function, but:

  1. Explorer unusable, very long and incorrect DB build. #1990
  2. Explorer block api unavailable with disabled explorer module(you dont say.jpg).

Also i believe that blocks api should be in consensus module.
@S-anasol

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Jun 28, 2017

Contributor

Response (block) example: https://gist.github.com/S-anasol/3fb462145ee8c13f16cf80f081b5642a

Human readable view:
image
Contributor

S-anasol commented Jun 28, 2017 •
edited

Response (block) example: https://gist.github.com/S-anasol/3fb462145ee8c13f16cf80f081b5642a

Human readable view:
image

S-anasol added some commits Jun 28, 2017

@S-anasol
Too much tabs, got fmt, stop please
Loading status checks…
71cfa2e
@S-anasol
Merge pull request #3 from NebulousLabs/master 
Update to head
Loading status checks…
f14d496
@S-anasol

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Jul 2, 2017

Contributor

No commets?
let me know if you want some additions.

But this is must have and useful 👍
https://explorer.siahub.info/ powered by this PR
Contributor

S-anasol commented Jul 2, 2017

No commets?
let me know if you want some additions.

But this is must have and useful 👍
https://explorer.siahub.info/ powered by this PR
api/api.go
@@ -162,6 +162,7 @@ func New(requiredUserAgent string, requiredPassword string, cs modules.Consensus
if api.cs != nil {
router.GET("/consensus", api.consensusHandler)
router.POST("/consensus/validate/transactionset", api.consensusValidateTransactionsetHandler)
router.GET("/consensus/blocks/:height", api.consensusBlocksHandler)

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

Wondering if we should let the user request by both height and id, or if just height is enough
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

Wondering if we should let the user request by both height and id, or if just height is enough

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

since at zero point we dont have any hash - easier way to get first data is just height of block.

Optionally ofc can be queried by hash, but i see no need for me.
Copy link
@S-anasol

S-anasol Aug 1, 2017

Contributor

since at zero point we dont have any hash - easier way to get first data is just height of block.

Optionally ofc can be queried by hash, but i see no need for me.
api/consensus.go
@@ -19,6 +21,55 @@ type ConsensusGET struct {
Difficulty types.Currency `json:"difficulty"`
}
type ConsensusFileContract struct {

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

needs a docstring
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

needs a docstring
api/consensus.go
RevisionNumber uint64 `json:"revisionnumber"`
}
type ConsensusFileContractRevision struct {

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

docstring
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

docstring
api/consensus.go
NewUnlockHash types.UnlockHash `json:"newunlockhash"`
}
type ConsensusTransaction struct {

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

docstring
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

docstring
api/consensus.go
SiacoinInputs map[string]types.SiacoinInput `json:"siacoininputs"`
SiacoinOutputs map[string]types.SiacoinOutput `json:"siacoinoutputs"`
FileContracts map[string]ConsensusFileContract `json:"filecontracts"`
FileContractRevisions map[string]ConsensusFileContractRevision `json:"filecontractrevisions"`

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

Why are these newly defined types instead of just using the types package?
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

Why are these newly defined types instead of just using the types package?

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

need this for child maps with hash as key
ValidProofOutputs map[string]types.SiacoinOutput

but actually not sure that we need these output/input hashes, it should be in blockchain in usual way too as simple out/in not in contract?
Copy link
@S-anasol

S-anasol Aug 1, 2017

Contributor

need this for child maps with hash as key
ValidProofOutputs map[string]types.SiacoinOutput

but actually not sure that we need these output/input hashes, it should be in blockchain in usual way too as simple out/in not in contract?

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

Is there any particular reason that you can't just use a types.Block to define the whole data structure? I don't see why we need these custom definitions, but I'm also not fully sure what all you are using it for.
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

Is there any particular reason that you can't just use a types.Block to define the whole data structure? I don't see why we need these custom definitions, but I'm also not fully sure what all you are using it for.

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

using custom structures with map[string] instead of types with map[] to fill it with any type hashes to make possibility index it with external tools. i.e.

[
[hash] -> [data array]
[hash] -> [data array]
[hash] -> [data array]
]

While default

[
[] -> [data array]
[] -> [data array]
[] -> [data array]
]

Sia internally generate hash every time when it need, like this:
https://github.com/NebulousLabs/Sia/blob/master/types/transactions.go#L217-L248

So default structures doesn't contain hash of anything.

Check this out sia explorer vs my indexer:
https://github.com/S-anasol/SiaConsensusIndexer/blob/master/update.php#L58-L119

https://github.com/NebulousLabs/Sia/blob/master/modules/explorer/update.go#L59

Sia Explorer uses internal functions to calculate hash(ID) for data array,
So making indexer in other language with full copy of Sia hashes calculation is complicated and overheaded.

I hope understandable explanation :)
Copy link
@S-anasol

S-anasol Aug 1, 2017 •
edited

Contributor

using custom structures with map[string] instead of types with map[] to fill it with any type hashes to make possibility index it with external tools. i.e.

[
[hash] -> [data array]
[hash] -> [data array]
[hash] -> [data array]
]

While default

[
[] -> [data array]
[] -> [data array]
[] -> [data array]
]

Sia internally generate hash every time when it need, like this:
https://github.com/NebulousLabs/Sia/blob/master/types/transactions.go#L217-L248

So default structures doesn't contain hash of anything.

Check this out sia explorer vs my indexer:
https://github.com/S-anasol/SiaConsensusIndexer/blob/master/update.php#L58-L119

https://github.com/NebulousLabs/Sia/blob/master/modules/explorer/update.go#L59

Sia Explorer uses internal functions to calculate hash(ID) for data array,
So making indexer in other language with full copy of Sia hashes calculation is complicated and overheaded.

I hope understandable explanation :)

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

Final result of my indexer

image
Copy link
@S-anasol

S-anasol Aug 1, 2017

Contributor

Final result of my indexer

image

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

Maybe there is better way to get same result, but i don't know it :)
Copy link
@S-anasol

S-anasol Aug 1, 2017

Contributor

Maybe there is better way to get same result, but i don't know it :)
api/consensus.go
WindowEnd types.BlockHeight `json:"windowend"`
Payout types.Currency `json:"payout"`
ValidProofOutputs map[string]types.SiacoinOutput `json:"validproofoutputs"`
MissedProofOutputs map[string]types.SiacoinOutput `json:"missedproofoutputs"`

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

I don't think using a map here is the right move, maps in go are randomly ordered, but valid and missed proofs have a specific order, and the order is meaningful.
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

I don't think using a map here is the right move, maps in go are randomly ordered, but valid and missed proofs have a specific order, and the order is meaningful.

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

(renter is first, host is second, void is third)
Copy link
@DavidVorick

DavidVorick Aug 1, 2017

Member

(renter is first, host is second, void is third)

This comment has been minimized.

Show comment
Hide comment
@S-anasol

S-anasol Aug 1, 2017

Contributor

comment above should answer this too :)

If we dont need this maps at all, it can be removed and used types package.
Copy link
@S-anasol

S-anasol Aug 1, 2017

Contributor

comment above should answer this too :)

If we dont need this maps at all, it can be removed and used types package.

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 2, 2017

Member

Ok. My very strong preference is that we just return a normal types.Block. If someone does end up needing the hashes to be computed for them, we can return all the ids (txids, block id, output ids, etc.) in a separate struct that mirrors the block, except it only has ids as fields. But if nobody is using it, we don't need it.
Copy link
@DavidVorick

DavidVorick Aug 2, 2017

Member

Ok. My very strong preference is that we just return a normal types.Block. If someone does end up needing the hashes to be computed for them, we can return all the ids (txids, block id, output ids, etc.) in a separate struct that mirrors the block, except it only has ids as fields. But if nobody is using it, we don't need it.
@DavidVorick

This comment has been minimized.

Show comment
Hide comment
@DavidVorick

DavidVorick Aug 1, 2017

Member

All changes to the API require two things:

  1. Updates to doc/API.md and the corresponding doc/api/module.md, explaining the full route, all the fields and their meanings, and providing basic example usages.

  2. A test in the api package demonstrating that the route works. Doesn't need to be super comprehensive but does need to show that the fields returned have non-zero values and that basic error handling (incorrect height, id, etc.) is in place.

I'm not sure what the reason was for making the valid and missed proofs part of a map, but I think we should look at a different way of accomplishing your goal. I think it would be much cleaner if the base types were not changed, and any extra/augmenting information should be in a separate data structure.
Member

DavidVorick commented Aug 1, 2017

All changes to the API require two things:

  1. Updates to doc/API.md and the corresponding doc/api/module.md, explaining the full route, all the fields and their meanings, and providing basic example usages.

  2. A test in the api package demonstrating that the route works. Doesn't need to be super comprehensive but does need to show that the fields returned have non-zero values and that basic error handling (incorrect height, id, etc.) is in place.

I'm not sure what the reason was for making the valid and missed proofs part of a map, but I think we should look at a different way of accomplishing your goal. I think it would be much cleaner if the base types were not changed, and any extra/augmenting information should be in a separate data structure.
@tbenz9

This comment has been minimized.

Show comment
Hide comment
@tbenz9

tbenz9 Nov 10, 2017

Collaborator

@S-anasol are you still interested in working on this PR or should we close it?
Collaborator

tbenz9 commented Nov 10, 2017

@S-anasol are you still interested in working on this PR or should we close it?

@S-anasol S-anasol closed this Nov 10, 2017

@S-anasol S-anasol referenced this pull request Feb 10, 2018

Merged

Add block-by-id and id-by-height consensus endpoints #2756

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.