-
Notifications
You must be signed in to change notification settings - Fork 490
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
tools: prepare block-generator for configuring apps #5443
Conversation
// TX Distribution / ID's | ||
paymentTx TxTypeID = "pay" | ||
assetTx TxTypeID = "asset" | ||
applicationTx TxTypeID = "appl" |
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 was un-commented
assetXfer TxTypeID = "asset_xfer" | ||
assetClose TxTypeID = "asset_close" | ||
|
||
// App kind TX Distribution / ID's don't exist because these are flattened |
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.
all the rest of the constants are new
// TX Distribution | ||
PaymentTransactionFraction float32 `yaml:"tx_pay_fraction"` | ||
AssetTransactionFraction float32 `yaml:"tx_asset_fraction"` | ||
AppTransactionFraction float32 `yaml:"tx_app_fraction"` |
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 is new
// App kind TX Distribution | ||
AppSwapFraction float32 `yaml:"app_swap_fraction"` | ||
AppBoxesFraction float32 `yaml:"app_boxes_fraction"` | ||
|
||
// App Swap TX Distribution | ||
AppSwapCreateFraction float32 `yaml:"app_swap_create_fraction"` | ||
AppSwapUpdateFraction float32 `yaml:"app_swap_update_fraction"` | ||
AppSwapDeleteFraction float32 `yaml:"app_swap_delete_fraction"` | ||
AppSwapOptinFraction float32 `yaml:"app_swap_optin_fraction"` | ||
AppSwapCallFraction float32 `yaml:"app_swap_call_fraction"` | ||
AppSwapCloseFraction float32 `yaml:"app_swap_close_fraction"` | ||
AppSwapClearFraction float32 `yaml:"app_swap_clear_fraction"` | ||
|
||
// App Boxes TX Distribution | ||
AppBoxesCreateFraction float32 `yaml:"app_boxes_create_fraction"` | ||
AppBoxesUpdateFraction float32 `yaml:"app_boxes_update_fraction"` | ||
AppBoxesDeleteFraction float32 `yaml:"app_boxes_delete_fraction"` | ||
AppBoxesOptinFraction float32 `yaml:"app_boxes_optin_fraction"` | ||
AppBoxesCallFraction float32 `yaml:"app_boxes_call_fraction"` | ||
AppBoxesCloseFraction float32 `yaml:"app_boxes_close_fraction"` | ||
AppBoxesClearFraction float32 `yaml:"app_boxes_clear_fraction"` |
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 rest of these app-related fields are new
func initializeConfigFile(configFile string) (config GenerationConfig, err error) { | ||
var data []byte | ||
data, err = os.ReadFile(configFile) | ||
if err != nil { | ||
return | ||
} | ||
err = yaml.Unmarshal(data, &config) | ||
if err != nil { | ||
return | ||
} | ||
|
||
err = config.validateWithDefaults(true) | ||
return | ||
} |
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.
moved from generate.go
and using validateWithDefaults()
to fail fast.
// sumIsCloseToOneWithDefault returns no error if the sum of the params is close to 1. | ||
// It returns an error if any of the params are negative. | ||
// Finally, in the case that all the params are zero, it sets the first param to 1 and returns no error. | ||
func sumIsCloseToOneWithDefault(defaults bool, params ...*float32) 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.
sumIsCloseToOneWithDefault
and validateSumCloseToOne
replace sumIsCloseToOne
and add the ability to default missing sections.
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.
Defaults is so you don't have to set anything for unused types? Maybe it could be 1 or 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.
Why do they need to be pointers? it would be nice to avoid asPtrSlice
everywhere.
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.
Defaults is so you don't have to set anything for unused types? Maybe it could be 1 or 0?
When an entire section is 0-valued, and defaults == true
the first pointer is set to 1.
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 do they need to be pointers? it would be nice to avoid
asPtrSlice
everywhere.
Because these when defaults == true
they modify the given values so I need to pass by pointer.
Codecov Report
@@ Coverage Diff @@
## master #5443 +/- ##
=======================================
Coverage 55.61% 55.62%
=======================================
Files 447 447
Lines 63395 63395
=======================================
+ Hits 35259 35264 +5
+ Misses 25757 25754 -3
+ Partials 2379 2377 -2 see 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
app_swap_optin_fraction: 0.1 | ||
app_swap_call_fraction: 0.98 | ||
app_swap_close_fraction: 0.005 | ||
app_swap_clear_fraction: 0.003 |
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.
could you please elaborate what you have mind for an app swap scenario? are these apps using the swap opcode?
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 is out of context. #5450 sheds a bit of light. In particular, I'm creating two apps with teal code:
That PR doesn't actually make boxes / swap assets yet, but it does create the apps and set some local state.
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.
prepare for apps child PR
Summary
Adding functionality to handle app transaction configuration, but NOT generating such transactions. A followup PR uses this functionality in generating app transactions.
Breaking out block generator configuration related code into
tools/block-generator/config.go
tools/block-generator/config_test.go
Blocked by
tools: bugfix block-generator to handle conduit's Init block requests #5449Test Plan
This is only tested in CI. However, a followup PR that includes all this code is passing local end-to-end testing.