Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: draft implementation of NEP-366 #7497

Closed
wants to merge 31 commits into from
Closed

feat: draft implementation of NEP-366 #7497

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ce474eb
Draft implementation of DelegateAction (NEP-366)
Aug 29, 2022
3eb7f02
Rename 'publisher_id' to 'relayer_id'
Aug 31, 2022
433b158
Updated according to the discussion in NEP-366
Aug 31, 2022
a7bcf89
Add fees for DelegateAction.
Oct 26, 2022
529ca2c
Remove redundant code. Add DelegateActionMustBeOnlyOne error
Nov 1, 2022
cfb9780
Add the inner actions' send fees to the total send fee
Nov 1, 2022
602baca
Charge the send fee for the inner actions twice
Nov 2, 2022
3b78721
Refactor deserialization the inner actions
Oct 31, 2022
106dc67
Applied max_block_height
Nov 1, 2022
ee124a6
Fix delegate_cost.exec_fee todo
Nov 3, 2022
11c29fc
Charge DelegateAction's 'send_fee' in two steps
Nov 8, 2022
b344298
Fixed review issues. Added a test for DelegateAction deserialization
Nov 9, 2022
d6ef8ba
Use `signer_public_key` instead of `delegate_action.public_key` in a …
Nov 9, 2022
df86e3a
Add tests to actions.rs. Fix tests in transaction.rs
Nov 10, 2022
59a5986
Return `AccountDoesNotExist` error if `sender` doesn't exist
Nov 10, 2022
cd289d4
Fix review issues
Nov 10, 2022
9a9c0da
Add protocol_feature_delegate_action
Nov 17, 2022
f34acda
Add "protocol_feature_delegate_action" to "nightly" features
Nov 17, 2022
704c524
Create test_delegate_action_deserialization for stable build
Nov 17, 2022
88973ce
Applied cargo fmt
Nov 17, 2022
335537e
Updated snapshots for tests::test_json_unchanged test
Nov 17, 2022
7575c90
Merge branch 'master' into NEP-366
Nov 17, 2022
010d718
Update DelegateAction feature version number
Nov 17, 2022
1935499
Apply cargo fmt
Nov 17, 2022
79915fd
Update snapshots and error schema for tests
Nov 17, 2022
155054f
Use `safe_add_gas` in `apply_delegate_action` function
Nov 18, 2022
81f4eb7
Add comments which explain what's going on
Nov 25, 2022
d0c3787
Rename protocol_feature_delegate_action with protocol_feature_nep366_…
Jan 12, 2023
e9e1f8f
Add comments and remove `new_delegate_actions`
Jan 12, 2023
d5cb1a0
Add "test_delegate_action_must_be_only_one" test
Jan 12, 2023
c9eb505
Add a test case for ACTION_DELEGATE_NUMBER
Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions chain/chain/src/tests/simple_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn build_chain() {
// cargo insta test --accept -p near-chain --features nightly -- tests::simple_chain::build_chain
let hash = chain.head().unwrap().last_block_hash;
if cfg!(feature = "nightly") {
insta::assert_display_snapshot!(hash, @"HTpETHnBkxcX1h3eD87uC5YP5nV66E6UYPrJGnQHuRqt");
insta::assert_display_snapshot!(hash, @"96KiRJdbMN8A9cFPXarZdaRQ8U2HvYcrGTGC8a4EgFzM");
} else {
insta::assert_display_snapshot!(hash, @"2iGtRFjF6BcqPF6tDcfLLojRaNax2PKDLxRqRc3RxRn7");
}
Expand Down Expand Up @@ -72,7 +72,7 @@ fn build_chain() {

let hash = chain.head().unwrap().last_block_hash;
if cfg!(feature = "nightly") {
insta::assert_display_snapshot!(hash, @"HyDYbjs5tgeEDf1N1XB4m312VdCeKjHqeGQ7dc7Lqwv8");
insta::assert_display_snapshot!(hash, @"4eW4jvyu1Ek6WmY3EuUoFFkrascC7svRww5UcZbNMkUf");
} else {
insta::assert_display_snapshot!(hash, @"7BkghFM7ZA8piYHAWYu4vTY6vE1pkTwy14bqQnS138qE");
}
Expand Down
56 changes: 54 additions & 2 deletions chain/jsonrpc/res/rpc_errors_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,13 @@
"FunctionCallError",
"NewReceiptValidationError",
"OnlyImplicitAccountCreationAllowed",
"DeleteAccountWithLargeState"
"DeleteAccountWithLargeState",
"DelegateActionInvalidSignature",
"DelegateActionSenderDoesNotMatchTxReceiver",
"DelegateActionExpired",
"DelegateActionAccessKeyError",
"DelegateActionInvalidNonce",
"DelegateActionNonceTooLarge"
],
"props": {
"index": ""
Expand All @@ -480,7 +486,9 @@
"FunctionCallMethodNameLengthExceeded",
"FunctionCallArgumentsLengthExceeded",
"UnsuitableStakingKey",
"FunctionCallZeroAttachedGas"
"FunctionCallZeroAttachedGas",
"DelegateActionCantContainNestedOne",
"DelegateActionMustBeOnlyOne"
],
"props": {}
},
Expand Down Expand Up @@ -556,6 +564,50 @@
"registrar_account_id": ""
}
},
"DelegateActionCantContainNestedOne": {
"name": "DelegateActionCantContainNestedOne",
"subtypes": [],
"props": {}
},
"DelegateActionExpired": {
"name": "DelegateActionExpired",
"subtypes": [],
"props": {}
},
"DelegateActionInvalidNonce": {
"name": "DelegateActionInvalidNonce",
"subtypes": [],
"props": {
"ak_nonce": "",
"delegate_nonce": ""
}
},
"DelegateActionInvalidSignature": {
"name": "DelegateActionInvalidSignature",
"subtypes": [],
"props": {}
},
"DelegateActionMustBeOnlyOne": {
"name": "DelegateActionMustBeOnlyOne",
"subtypes": [],
"props": {}
},
"DelegateActionNonceTooLarge": {
"name": "DelegateActionNonceTooLarge",
"subtypes": [],
"props": {
"delegate_nonce": "",
"upper_bound": ""
}
},
"DelegateActionSenderDoesNotMatchTxReceiver": {
"name": "DelegateActionSenderDoesNotMatchTxReceiver",
"subtypes": [],
"props": {
"receiver_id": "",
"sender_id": ""
}
},
"DeleteAccountStaking": {
"name": "DeleteAccountStaking",
"subtypes": [],
Expand Down
1 change: 1 addition & 0 deletions chain/rosetta-rpc/src/adapters/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ impl From<NearActions> for Vec<crate::models::Operation> {
);
operations.push(deploy_contract_operation);
}
near_primitives::transaction::Action::Delegate(_) => todo!(),
Copy link
Author

@e-uleyskiy e-uleyskiy Nov 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakmeier
Should this todo be fixed for commiting this patch or it can be done in another patch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this as a todo for the initial PR, this should not stop us from getting through the NEP approval.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it should be fixed before submitting (otherwise this code path could cause panic in the node)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we currently don't have a way to parse DelegateAction without the feature enabled. So it would only be possible to panic on nightly builds. But if you want to fix this before we merge, @firatNEAR told me they will take care of this one.

}
}
operations
Expand Down
4 changes: 4 additions & 0 deletions core/primitives-core/src/parameter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ pub enum Parameter {
ActionDeleteKeySendSir,
ActionDeleteKeySendNotSir,
ActionDeleteKeyExecution,
ActionDelegateSendSir,
ActionDelegateSendNotSir,
ActionDelegateExecution,

// Smart contract dynamic gas costs
WasmRegularOpCost,
Expand Down Expand Up @@ -205,6 +208,7 @@ pub enum FeeParameter {
ActionAddFunctionCallKey,
ActionAddFunctionCallKeyPerByte,
ActionDeleteKey,
ActionDelegate,
}

impl Parameter {
Expand Down
11 changes: 10 additions & 1 deletion core/primitives-core/src/runtime/fees.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ pub struct ActionCreationConfig {

/// Base cost of deleting an account.
pub delete_account_cost: Fee,

/// Base cost of a delegate action
pub delegate_cost: Fee,
}

/// Describes the cost of creating an access key.
Expand Down Expand Up @@ -219,6 +222,11 @@ impl RuntimeFeesConfig {
send_not_sir: 147489000000,
execution: 147489000000,
},
delegate_cost: Fee {
send_sir: 2319861500000,
send_not_sir: 2319861500000,
execution: 2319861500000,
},
},
storage_usage_config: StorageUsageConfig {
// See Account in core/primitives/src/account.rs for the data structure.
Expand Down Expand Up @@ -253,7 +261,8 @@ impl RuntimeFeesConfig {
function_call_cost_per_byte: free.clone(),
},
delete_key_cost: free.clone(),
delete_account_cost: free,
delete_account_cost: free.clone(),
delegate_cost: free,
},
storage_usage_config: StorageUsageConfig {
num_bytes_account: 0,
Expand Down
2 changes: 2 additions & 0 deletions core/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ protocol_feature_reject_blocks_with_outdated_protocol_version = []
protocol_feature_ed25519_verify = [
"near-primitives-core/protocol_feature_ed25519_verify"
]
protocol_feature_delegate_action = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small preference to add the nep_366 to the name (makes it a lot easier to find in the future).

nightly = [
"nightly_protocol",
"protocol_feature_fix_staking_threshold",
"protocol_feature_fix_contract_loading_cost",
"protocol_feature_reject_blocks_with_outdated_protocol_version",
"protocol_feature_ed25519_verify",
"protocol_feature_delegate_action",
]

nightly_protocol = []
Expand Down
3 changes: 3 additions & 0 deletions core/primitives/res/runtime_configs/parameters.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ action_add_function_call_key_per_byte_execution: 1_925_331
action_delete_key_send_sir: 94_946_625_000
action_delete_key_send_not_sir: 94_946_625_000
action_delete_key_execution: 94_946_625_000
action_delegate_send_sir: 2_319_861_500_000
e-uleyskiy marked this conversation as resolved.
Show resolved Hide resolved
action_delegate_send_not_sir: 2_319_861_500_000
action_delegate_execution: 2_319_861_500_000

# Smart contract dynamic gas costs
wasm_regular_op_cost: 3_856_371
Expand Down
3 changes: 3 additions & 0 deletions core/primitives/res/runtime_configs/parameters_testnet.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ action_add_function_call_key_per_byte_execution: 1_925_331
action_delete_key_send_sir: 94_946_625_000
action_delete_key_send_not_sir: 94_946_625_000
action_delete_key_execution: 94_946_625_000
action_delegate_send_sir: 2_319_861_500_000
e-uleyskiy marked this conversation as resolved.
Show resolved Hide resolved
action_delegate_send_not_sir: 2_319_861_500_000
action_delegate_execution: 2_319_861_500_000

# Smart contract dynamic gas costs
wasm_regular_op_cost: 3_856_371
Expand Down
30 changes: 30 additions & 0 deletions core/primitives/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ pub enum ActionsValidationError {
UnsuitableStakingKey { public_key: PublicKey },
/// The attached amount of gas in a FunctionCall action has to be a positive number.
FunctionCallZeroAttachedGas,
/// DelegateAction actions contain another DelegateAction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd mention "which is not allowed"

DelegateActionCantContainNestedOne,
/// There should be the only one DelegateAction
DelegateActionMustBeOnlyOne,
}

/// Describes the error for validating a receipt.
Expand Down Expand Up @@ -314,6 +318,14 @@ impl Display for ActionsValidationError {
f,
"The attached amount of gas in a FunctionCall action has to be a positive number",
),
ActionsValidationError::DelegateActionCantContainNestedOne => write!(
f,
"DelegateAction must not contain another DelegateAction"
),
ActionsValidationError::DelegateActionMustBeOnlyOne => write!(
f,
"The actions can contain the ony one DelegateAction"
)
}
}
}
Expand Down Expand Up @@ -397,6 +409,18 @@ pub enum ActionErrorKind {
OnlyImplicitAccountCreationAllowed { account_id: AccountId },
/// Delete account whose state is large is temporarily banned.
DeleteAccountWithLargeState { account_id: AccountId },
/// Signature does not match the provided actions and given signer public key.
DelegateActionInvalidSignature,
/// Receiver of the transaction doesn't match Sender of the delegate action
DelegateActionSenderDoesNotMatchTxReceiver { sender_id: AccountId, receiver_id: AccountId },
/// Delegate action has expired. `max_block_height` is less than actual block height.
DelegateActionExpired,
/// The given public key doesn't exist for Sender account
DelegateActionAccessKeyError(InvalidAccessKeyError),
/// DelegateAction nonce must be greater sender[public_key].nonce
DelegateActionInvalidNonce { delegate_nonce: Nonce, ak_nonce: Nonce },
/// DelegateAction nonce is larger than the upper bound given by the block height
DelegateActionNonceTooLarge { delegate_nonce: Nonce, upper_bound: Nonce },
}

impl From<ActionErrorKind> for ActionError {
Expand Down Expand Up @@ -707,6 +731,12 @@ impl Display for ActionErrorKind {
ActionErrorKind::InsufficientStake { account_id, stake, minimum_stake } => write!(f, "Account {} tries to stake {} but minimum required stake is {}", account_id, stake, minimum_stake),
ActionErrorKind::OnlyImplicitAccountCreationAllowed { account_id } => write!(f, "CreateAccount action is called on hex-characters account of length 64 {}", account_id),
ActionErrorKind::DeleteAccountWithLargeState { account_id } => write!(f, "The state of account {} is too large and therefore cannot be deleted", account_id),
ActionErrorKind::DelegateActionInvalidSignature => write!(f, "DelegateAction is not signed with the given public key"),
ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id, receiver_id } => write!(f, "Transaction reciever {} doesn't match DelegateAction sender {}", sender_id, receiver_id),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameters in wrong order in the error message.

Also - typo in receiver.

ActionErrorKind::DelegateActionExpired => write!(f, "DelegateAction has expired"),
ActionErrorKind::DelegateActionAccessKeyError(access_key_error) => Display::fmt(&access_key_error, f),
ActionErrorKind::DelegateActionInvalidNonce { delegate_nonce, ak_nonce } => write!(f, "DelegateAction nonce {} must be larger than nonce of the used access key {}", delegate_nonce, ak_nonce),
ActionErrorKind::DelegateActionNonceTooLarge { delegate_nonce, upper_bound } => write!(f, "DelegateAction nonce {} must be smaller than the access key nonce upper bound {}", delegate_nonce, upper_bound),
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions core/primitives/src/receipt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,30 @@ impl Receipt {
}),
}
}

pub fn new_delegate_actions(
jakmeier marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment - and explain why do we need a special function for this case.

signer_id: &AccountId,
predecessor_id: &AccountId,
receiver_id: &AccountId,
actions: &Vec<Action>,
public_key: &PublicKey,
gas_price: Balance,
) -> Self {
Receipt {
predecessor_id: predecessor_id.clone(),
receiver_id: receiver_id.clone(),
receipt_id: CryptoHash::default(),

receipt: ReceiptEnum::Action(ActionReceipt {
signer_id: signer_id.clone(),
signer_public_key: public_key.clone(),
gas_price: gas_price,
output_data_receivers: vec![],
input_data_ids: vec![],
actions: actions.clone(),
}),
}
}
}

/// Receipt could be either ActionReceipt or DataReceipt
Expand Down
1 change: 1 addition & 0 deletions core/primitives/src/runtime/parameter_table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ impl ParameterTable {
},
"delete_key_cost": self.fee_json(FeeParameter::ActionDeleteKey),
"delete_account_cost": self.fee_json(FeeParameter::ActionDeleteAccount),
"delegate_cost": self.fee_json(FeeParameter::ActionDelegate),
},
"storage_usage_config": {
"num_bytes_account": self.get(Parameter::StorageNumBytesAccount),
Expand Down
5 changes: 5 additions & 0 deletions ...imitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__0.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__42.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__48.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__49.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakmeier - are these changes expected? shouldn't the delegate cost be from the 'next' release onwards only?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, expected, due to Serde we need these values also in old version (maybe one day we can get away from the JSON representation all together, then we could avoid this)

"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__50.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__52.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__53.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions ...mitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__57.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
5 changes: 5 additions & 0 deletions .../src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_0.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ expression: store.get_config(*version)
"send_sir": 147489000000,
"send_not_sir": 147489000000,
"execution": 147489000000
},
"delegate_cost": {
"send_sir": 2319861500000,
"send_not_sir": 2319861500000,
"execution": 2319861500000
}
},
"storage_usage_config": {
Expand Down
Loading