-
Notifications
You must be signed in to change notification settings - Fork 2
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validation Refactor and Error Codes #52
Conversation
c86b3f2
to
d17bd41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
馃憤
@@ -73,11 +73,11 @@ func DecodeBlock(r io.Reader) *Block { | |||
|
|||
// ContainsTransaction returns true and the transaction itself if the Block | |||
// contains the transaction. | |||
func (b *Block) ContainsTransaction(t *Transaction) (bool, *Transaction) { | |||
for _, tr := range b.Transactions { | |||
func (b *Block) ContainsTransaction(t *Transaction) (bool, uint32) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why uint32
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. uint32
for blocks because thats baked into the data structure, avoid overflow if we have billions of blocks. Thought we might as well keep the same datatype for transactions as well, no such thing as a negative transaction index.
blockchain/transaction.go
Outdated
|
||
// InputsEqualOutputs returns true if t.Inputs == other.Outputs, as well | ||
// as the difference between the two (outputs - inputs). | ||
func (t *Transaction) InputsEqualOutputs(other ...*Transaction) (bool, int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking that the addresses match is done somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, why are we returning the diff as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case we want to verify that the diff is equal to some fee.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we aren't using it, I'd leave it out until we need it. Fees will eventually be part of the transaction so we can verify as part of this function anyway right?
blockchain/validation.go
Outdated
|
||
const ( | ||
// ValidTransaction is returned when transaction is valid. | ||
ValidTransaction TransactionCode = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use iota
(https://golang.org/ref/spec#Iota)
blockchain/validation.go
Outdated
) | ||
|
||
const ( | ||
// ValidBlock is returned when the block is valid. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same -- use iota
blockchain/validation_test.go
Outdated
package blockchain | ||
|
||
import ( | ||
crand "crypto/rand" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use rand
if we aren't distinguishing between math/rand and crypto/rand
Status Ready for review.
What Changed
馃椇 Refactored the validation functions into their own file (along with tests).
鈿欙笍 Added error codes to enforce proper failing in test suite.
TODO
[]*Transactions
data structure.Testbc.GetInputTransaction
method.