-
Notifications
You must be signed in to change notification settings - Fork 664
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
feat: add receipt action for using global contract code #12749
Conversation
56b2cc7
to
805c06e
Compare
@@ -126,7 +126,7 @@ impl ReceiptPreparationPipeline { | |||
for (action_index, action) in actions.iter().enumerate() { | |||
let account_id = account_id.clone(); | |||
match action { | |||
Action::DeployContract(_) => { | |||
Action::DeployContract(_) | Action::UseGlobalContract(_) => { |
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.
cc @nagisa
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!
@@ -270,6 +306,7 @@ pub enum Action { | |||
DeleteAccount(DeleteAccountAction), | |||
Delegate(Box<delegate::SignedDelegateAction>), | |||
DeployGlobalContract(DeployGlobalContractAction), | |||
UseGlobalContract(Box<UseGlobalContractAction>), |
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.
Was wondering when do we use a Box vs when not? Example DeployGlobalContract doesn't have a Box
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.
there is static assert below:
const _: () = assert!(
// 1 word for tag plus the largest variant `DeployContractAction` which is a 3-word `Vec`.
// The `<=` check covers platforms that have pointers smaller than 8 bytes as well as random
// freak nightlies that somehow find a way to pack everything into one less word.
std::mem::size_of::<Action>() <= 32,
"Action <= 32 bytes for performance reasons, see #9451"
);
UseGlobalContract { | ||
code_hash: CryptoHash, | ||
}, | ||
UseGlobalContractByAccountId { |
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.
Checking if it's fine to have two separate enums here while we have just one in Action::UseGlobalContract
?
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.
In my understanding it is OK, since Action
is our internal representation while ActionView
is public API. So I decided to have single Action
because it is more concise. Having separate ActionView
for deploying by account id avoids overcomplicating public api with choosing the identifier.
805c06e
to
8dfb3dd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12749 +/- ##
==========================================
- Coverage 70.69% 70.63% -0.07%
==========================================
Files 848 849 +1
Lines 174376 174470 +94
Branches 174376 174470 +94
==========================================
- Hits 123280 123235 -45
- Misses 45950 46086 +136
- Partials 5146 5149 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR is similar to #12737, but for using global contracts.
Part of #12716.