-
Notifications
You must be signed in to change notification settings - Fork 66
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
Contract callback #190
Contract callback #190
Conversation
This reverts commit dcc90df.
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 move fairyring_contract
to inside the tests directory.
x/pep/module/module.go
Outdated
contracts, found := am.keeper.GetContractEntriesByID(ctx, entry.Identity) | ||
if found { | ||
if len(contracts.Contracts) != 0 { | ||
for _, contract := range contracts.Contracts { |
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 would add some check to the fields in contract
before ExecuteContract()
just to make sure is not empty
and I think we can: if found && len(contracts.Contracts) != 0 {}
instead 2 if
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.
-
its really not possible to verify anything about the contract since the contracts are executed in a kind of sandboxed env.
-
merged the two ifs into one
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 mean just to make sure entry.Identity
, entry.Pubkey
and entry.AggrKeyshare
are not empty before executing ?
@@ -114,3 +115,11 @@ message MsgGetPrivateKeyshares { | |||
|
|||
message MsgGetPrivateKeysharesResponse {} | |||
|
|||
message MsgRegisterContract { |
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.
Do we need MsgDeRegisterContract
too ?
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 think that is required.
@bowenyou your thoughts on this?
} | ||
} | ||
} else { | ||
entry.Contracts = make([]*types.ContractDetails, 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.
Don't think this is needed ?
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.
remember the chain crashed last time whenever a general keyshare request was made. and that was because a particular slice had not been made and as a result there was a nil pointer.
i just am being overly safe here not to repeat another chain crash because of a malformed request or something. so, I made doubly sure by initializing a blank slice
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 that's the case isn't this suppose to be executed if the entry not found ?
entry, found := k.GetContractEntriesByID(ctx, msg.Identity)
if found {
....
} else {
entry.Contracts = make([]*types.ContractDetails, 0)
}
If len(entry.Contracts) == 0
then I guess it is already an empty array ?
echo "# Testing contract callback on source chain #" | ||
echo "#############################################" | ||
|
||
RSP=$($BINARY q wasm contract-state smart $CONTRACT_ADDR '{"decrypt_data": {"pubkey": "a2a4472488440341db3252447af1c31e06fd32d7067e300ed60052fcdd131fd702bf901e1dd0122a312bb582a9a375a3", "aggr_keyshare": "a3b49bbffd655aa37e0b71a4d90862e1f70bdd0aab48587307ef74c2b3e12fd2ea42d88fc5f592e5caf83d33d7f93454196f32137817ceb5ecb41fbe48c3734bb11510febd6988302dd2c362deb3479b4946daa399fb149e63c0a5c45b48292d", "encrypted_data": "6167652d656e6372797074696f6e2e6f72672f76310a2d3e20646973744942450a686e4a7641376d5655797679397166465230447849417464374c3152586371484542687736306a316f325a446e567453626a4759374a4d2f5a524752654e536b0a574d6f56567966674d55546f363944502f4f624a6544424e6f47694b50746a6b316a523075464276536372326d766948543238524f6e473755647835683077510a6c734767656554424336786e7834626e496d737874410a2d2d2d20793668724135506e5233563568414a35646f732b574e325932334b72742b383946306d4d743138595a59490a43129dfd9ddbb210374314a96ab1b06260b4e1abf7d3fac77029043c8bdbe0a6efd2b73f95f75be0"}}' --node $CHAIN1_NODE -o json) |
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 hardcoded pub key & aggr keyshare instead of getting it from the chain ?
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.
did you mean to check of the query works without explicitly providing the pubkey?
i added the test for that
No description provided.