Mandos test benchmarking #3527
Mandos test benchmarking #3527
Conversation
andrei-diaconescu
commented
Oct 21, 2021
•
andrei-diaconescu
commented
- added mandos test benchmarking
Codecov Report
@@ Coverage Diff @@
## development #3527 +/- ##
============================================
Coverage 73.78% 73.78%
============================================
Files 589 589
Lines 76218 76218
============================================
+ Hits 56237 56241 +4
+ Misses 15532 15530 -2
+ Partials 4449 4447 -2
Continue to review full report at Codecov.
|
| account.SetCode(accountCode) | ||
| ownerAddress := mandosAcc.GetOwner() | ||
| account.SetOwnerAddress(ownerAddress) | ||
| account.SetCodeMetadata([]byte{0, 0}) |
There was a problem hiding this comment.
Does mandosAcc contain code metadata to use here instead of 0?
There was a problem hiding this comment.
no, mandos does not yet support code metadata as far as I know
| for i, mandosTx := range mandosTxs { | ||
| if bytes.Equal(mandosTx.GetReceiverAddress(), addressToBeReplaced) { | ||
| mandosTx.WithReceiverAddress(deployedScAddress) | ||
| mandosTxs[i] = mandosTx |
There was a problem hiding this comment.
Is this line necessary?
| } | ||
|
|
||
| // DeploySCsFromMandosDeployTxs deploys all smartContracts correspondent to "scDeploy" in a mandos test, then replaces with the correct computed address in all the transactions. | ||
| func DeploySCsFromMandosDeployTxs(testContext *vm.VMTestContext, deployMandosTxs []*mge.Transaction, mandosTxs []*mge.Transaction, deployedScAccounts []*mge.TestAccount) (err error) { |
There was a problem hiding this comment.
Consider splitting this function in two, separating the tasks of (1) deploying the SCs and collecting their new addresses and then (2) injecting the new addresses in the Mandos transactions that need to call the newly deployed SCs. As it is, the function is somewhat opaque and it's not clear whether it does what the author intended.
| transactions = make([]*transaction.Transaction, 0) | ||
| for _, mandosTx := range mandosTxs { | ||
| gasLimit, gasPrice := mandosTx.GetGasLimitAndPrice() | ||
| var data []byte |
There was a problem hiding this comment.
Move this line at the top of the function.
| codeHash := account.GetCodeHash() | ||
| code := accAdapter.GetCode(codeHash) | ||
| require.Equal(t, len(mandosAcc.GetCode()), len(code)) | ||
|
|
There was a problem hiding this comment.
If the mandosAcc contains CodeMetadata, it should be asserted here as well against the CodeMetadata in accHandler.
There was a problem hiding this comment.
As far as I know, CodeMetadata is currently not available in mandos
| } | ||
| defer testContext.Close() | ||
| if benchmarkTxPos == mge.InvalidBenchmarkTxPos { | ||
| fmt.Println("There is not benchmark to be made") |
There was a problem hiding this comment.
| fmt.Println("There is not benchmark to be made") | |
| fmt.Println("no transactions marked for benchmarking") |
| func BenchmarkMandosSpecificTx(b *testing.B, mandosTestPath string) { | ||
| testContext, transactions, benchmarkTxPos, err := SetStateFromMandosTest(mandosTestPath) | ||
| if err != nil { | ||
| fmt.Println("Setting state went wrong: ", err) |
There was a problem hiding this comment.
Consider using the logger instead of fmt.Println().
andrei-diaconescu
dismissed stale reviews from sasurobert and camilbancioiu
via
3f046b9
| @@ -0,0 +1,185 @@ | |||
| package mandosConverter | |||
There was a problem hiding this comment.
mandosConverterUtils :| - not quite happy with the naming
