-
Notifications
You must be signed in to change notification settings - Fork 14
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
Hubble 526 protocol22 changes #283
Conversation
} | ||
case xdr.HostFunctionTypeHostFunctionTypeUploadContractWasm: | ||
details["type"] = "upload_wasm" | ||
transactionEnvelope := getTransactionV1Envelope(operation.transaction.Envelope) | ||
details["ledger_key_hash"] = ledgerKeyHashFromTxEnvelope(transactionEnvelope) | ||
details["contract_code_hash"] = contractCodeHashFromTxEnvelope(transactionEnvelope) | ||
case xdr.HostFunctionTypeHostFunctionTypeCreateContractV2: |
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.
Operation cases are duplicated due to how the original stellar-etl was written
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.
To-do: fix unit tests and looks good 🤷♀️
// This will initially be handled the same as InvokeContractParams until a different | ||
// model is found necessary. |
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 you say "until a different model is found necessary" do you mean if we need to update the schema/data model? Or is there a possibility core will change how they present this data?
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.
Yeah my assumption right now is that because these constructorArgs are also []ScVal that they can be handled the same way as the invoke params. But it is possible that core designed constructorArgs to be different but there were no existing examples at the time of writing 😅
So the comment is more like "hey these two things look similar and currently process similarly but that doesn't mean they will look and process similarly in the future until p23 is released and made permanent"
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.
LGTM
Is now the right time to throw a deprecation warning that we plan to drop stellar-core support from ETL directly?
internal/transform/operation_test.go
Outdated
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.
nice
That's a good idea. I'll throw in the deprecation warning |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: golang/github.com/stellar/[email protected] |
PR Checklist
PR Structure
otherwise).
Thoroughness
Release planning
semver, and I've changed the name of the BRANCH to release/_ , feature/_ or patch/* .
What
Add support for Protocol 22 changes
Largely this is supporting the new constructor params in the form of a new createContractV2 from InovkeHostFunction
The relevant bls changes are only updating the
ContractCostType
enum with bls changes. This is automatically taken care of because stellar-etl config_settings handlesContractCost
as map/dict-like{type: value}
. If needed this can be made more user friendly with dbt transformations into a flattened tableCAP60 may be moved out of P22
Why
To support P22
Known limitations