-
Notifications
You must be signed in to change notification settings - Fork 493
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
replace the KMD usage in pingpong with direct signing #2653
replace the KMD usage in pingpong with direct signing #2653
Conversation
Still should have local storage for generated accounts |
shared/pingpong/pingpong.go
Outdated
@@ -208,12 +220,12 @@ func fundAccounts(accounts map[string]uint64, client libgoal.Client, cfg PpConfi | |||
} | |||
|
|||
fmt.Printf("adjusting account balance to %d\n", minFund) | |||
for addr, balance := range accounts { | |||
for addr, val := range accounts { |
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.
val
-> acct
shared/pingpong/pingpong.go
Outdated
cfg PpConfig | ||
accounts map[string]uint64 | ||
cfg PpConfig | ||
//accounts map[string]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.
drop commented out lines.
shared/pingpong/pingpong.go
Outdated
return | ||
} | ||
|
||
psig = accounts[signer].sk.Sign(logic.Program(cfg.Program)) |
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 could be wrong, but I think that you might want to do
progb := logic.Program(cfg.Program)
psig = accounts[signer].sk.Sign(&progb)
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.
This PR seems to fail for the following test cases:
asset_transfer_small, asset_transfer_large, stateful_teal_small, stateful_teal_medium, stateful_teal_large.
Could you manually test that locally and make sure it generates valid transactions ?
Codecov Report
@@ Coverage Diff @@
## master #2653 +/- ##
==========================================
+ Coverage 47.04% 47.05% +0.01%
==========================================
Files 349 349
Lines 55814 55814
==========================================
+ Hits 26256 26262 +6
+ Misses 26608 26605 -3
+ Partials 2950 2947 -3
Continue to review full report at Codecov.
|
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.
looks good.
This reverts commit 8835bfd.
Summary
Removes signing calls to KMD by creating internal hashmap and signing transactions locally.
Resolves #2644
Test Plan
Tested using performance system.