-
Notifications
You must be signed in to change notification settings - Fork 0
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
added generaterandomcontract function #426
base: dev
Are you sure you want to change the base?
Conversation
internal/contracts/contracts.go
Outdated
fmt.print(c) | ||
} | ||
|
||
func (c *Contract) ContainsPublicKey(pk publickey.AurumPublicKey) (bool, error) { |
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.
Remove this function, it will be on the other branch.
internal/contracts/contracts.go
Outdated
|
||
} | ||
|
||
func(c *Contract) GenerateRandomContract() (Contract){ |
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.
- I don't believe we need a receiver, remove it
- Remove parentheses from return type
internal/contracts/contracts.go
Outdated
genStateNonce := binary.LittleEndian.Uint64(b[:]) | ||
|
||
|
||
c := &Contract{ |
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.
- instead of assigning
c
to this, just return it. - no need for pointer
&
internal/contracts/contracts.go
Outdated
StateNonce: genStateNonce, | ||
|
||
} | ||
return c, nil |
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.
- remove this return in favor of the above change.
internal/contracts/contracts.go
Outdated
Value: genValue, | ||
StateNonce: genStateNonce, | ||
|
||
}, nil |
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.
- There is no need to return a
nil
here, since your function only returns aContract
per the signature.
internal/contracts/contracts.go
Outdated
@@ -274,4 +275,31 @@ func (mc *JSONContract) Unmarshal() (Contract, error) { | |||
mc.StateNonce, | |||
} | |||
return c, nil | |||
fmt.print(c) |
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.
- remove print call here
internal/contracts/contracts.go
Outdated
func GenerateRandomContract() Contract{ | ||
b := rand.Read(make([]byte, 32)) | ||
|
||
genSenderPubKey := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) |
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.
-
ecdsa.GenerateKey
generates a private key. You need to extract the public key from the private key
How this is done: https://github.com/SIGBlockchain/project_aurum/blob/master/internal/publickey/publickey_test.go#L56
internal/contracts/contracts.go
Outdated
Value: genValue, | ||
StateNonce: genStateNonce, | ||
|
||
}, |
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.
- Remove this comma
internal/contracts/contracts.go
Outdated
genValue := binary.LittleEndian.Uint64(b[:]) | ||
genStateNonce := binary.LittleEndian.Uint64(b[:]) | ||
|
||
return Contract{ |
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.
Instead of defining the struct with Contract { ... }
, leverage the New
function that returns a contract, just feed it your gen_
parameters.
I can't remember, but I think it might return a pointer to a contract, in which case you'll need to adjust your return type.
internal/contracts/contracts.go
Outdated
genrecipPubKeyHash := rand.Read(make([]byte, 32)) | ||
genVersion := binary.LittleEndian.Uint16(b[:]) | ||
genSigLen := binary.LittleEndian.Uint8(b[:]); | ||
genSignature := rand.Read(make([]byte, genSigLen)); |
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.
Let's remove these two Sig
variables, since we don't need them in the New
function
internal/contracts/contracts.go
Outdated
b := rand.Read(make([]byte, 32)) | ||
|
||
genSenderPrivKey := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) | ||
genrecipPubKeyHash := rand.Read(make([]byte, 32)) |
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.
- Fix camelCase format
internal/contracts/contracts.go
Outdated
rand.Read(b) | ||
genSenderPrivKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) | ||
genRecipPubKeyHash := b | ||
genVersion := binary.LittleEndian.Uint16(b[:]) |
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.
Is there a way to make sure this is never 0? The only instance New
returns an error is if the version passed in is a zero.
- Generate only nonzero numbers for version
- Make return type be just a
Contract
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.
The new function returns 2 values, contract and error, so when I return a new *random contract, it also returns error
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.
I see. We can do what you had initially: define a return variable, with its error, then just return the return variable at the end. It doesn't make much sense to generate a random contract and need to do error checks on the call.
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, the new function already checks if the version is greater than 0. do u want it so that is doesn't make a contract with a zero value version in the first place?
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.
Yes this is exactly what I want.
I had to use error because rand.Read returns an error |
internal/contracts/contracts.go
Outdated
|
||
func GenerateRandomContract() (*Contract, error){ | ||
b := make([]byte, 32) | ||
_, err :=rand.Read(b) |
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.
When will this return a non-nil error
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.
The read function from rand is a helper function that uses io.readfull function from. The readFull function reads the length of the bytes and returns an error if fewer bytes were read. So, if for some reason not all the bytes were read, it would return an error
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 there's no identifiable reason that all the bytes will be read, then ignore the error. I don't want to approve a generate random contract function that returns any errors because that is impractical.
not sure why jsontest file was changed |
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.
I think after this it'll be good
internal/contracts/contracts.go
Outdated
min := 1 | ||
maxVer := 65535 | ||
b := make([]byte, 32) | ||
mrand.Read(b) |
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.
What is the purpose of this?
internal/contracts/contracts.go
Outdated
maxVer := 65535 | ||
b := make([]byte, 32) | ||
mrand.Read(b) | ||
genRecipPubKeyHash := b |
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.
Can you make this the result of the hash of a random number?
i.e. hashing.New([]byte{ someRandomNumber })
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.
does it matter how long the number inside the byte is, as in does it matter if it is a random Uint16 or Uint64
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.
I thought an array of bytes was multiple values, not just one singular number
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.
does it matter how long the number inside the byte is, as in does it matter if it is a random Uint16 or Uint64
I don't think it matters. Choose whichever is easier for you
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.
is it possible to have an array of bytes with just a single number
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.
i thought it was an array with multiple numbers
b := make([]byte, 8) | ||
binary.LittleEndian.PutUint64(b, mrand.Uint64()) | ||
|
||
genRecipPubKeyHash := hashing.New(b) |
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.
b
isn't random
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.
I tested it out on playground, and it generates a random []byte every time I call the function
(#423 )