Skip to content

Commit

Permalink
Incorporate changes from @movekevin (aptos-labs#8411)
Browse files Browse the repository at this point in the history
  • Loading branch information
alnoki committed Jun 5, 2023
1 parent bc1e58d commit ca4cc0f
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 43 deletions.
59 changes: 41 additions & 18 deletions aptos-move/framework/aptos-framework/doc/multisig_account.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ and implement the governance voting logic on top.
- [Function `reject_transaction`](#0x1_multisig_account_reject_transaction)
- [Function `vote_transanction`](#0x1_multisig_account_vote_transanction)
- [Function `execute_rejected_transaction`](#0x1_multisig_account_execute_rejected_transaction)
- [Function `flush_next_sequence_number_and_owner_schema`](#0x1_multisig_account_flush_next_sequence_number_and_owner_schema)
- [Function `flush_last_resolved_sequence_number_and_owner_schema`](#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema)
- [Function `validate_multisig_transaction`](#0x1_multisig_account_validate_multisig_transaction)
- [Function `validate_multisig_transaction_with_optional_sequence_number`](#0x1_multisig_account_validate_multisig_transaction_with_optional_sequence_number)
- [Function `flush_next_sequence_number_and_owner_schema_execution`](#0x1_multisig_account_flush_next_sequence_number_and_owner_schema_execution)
- [Function `flush_last_resolved_sequence_number_and_owner_schema_execution`](#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema_execution)
- [Function `successful_transaction_execution_cleanup`](#0x1_multisig_account_successful_transaction_execution_cleanup)
- [Function `failed_transaction_execution_cleanup`](#0x1_multisig_account_failed_transaction_execution_cleanup)
- [Function `remove_executed_transaction`](#0x1_multisig_account_remove_executed_transaction)
Expand Down Expand Up @@ -781,6 +781,16 @@ Owner list cannot contain the same address more than once.



<a name="0x1_multisig_account_EFLUSH_TXN_EXPIRED"></a>

Flush transaction proposal has expired.


<pre><code><b>const</b> <a href="multisig_account.md#0x1_multisig_account_EFLUSH_TXN_EXPIRED">EFLUSH_TXN_EXPIRED</a>: u64 = 19;
</code></pre>



<a name="0x1_multisig_account_EINVALID_PAYLOAD_HASH"></a>

Payload hash must be exactly 32 bytes (sha3-256).
Expand Down Expand Up @@ -2082,9 +2092,9 @@ Remove the next transaction if it has sufficient owner rejections.

</details>

<a name="0x1_multisig_account_flush_next_sequence_number_and_owner_schema"></a>
<a name="0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema"></a>

## Function `flush_next_sequence_number_and_owner_schema`
## Function `flush_last_resolved_sequence_number_and_owner_schema`

Prototype function signature used for flush proposal payload generation.

Expand All @@ -2111,17 +2121,20 @@ corresponds to a valid flush transaction: if the proposal stores the entire payl
on-chain then the payload is deserialized and verified, but if the proposal stores only
a payload hash, then the payload included in the Aptos API MultisigFlush transaction is
instead verified.
5. The VM calls <code>flush_next_sequence_number_and_owner_schema_execution</code>, applying owner and
required signature count updates, effectively bumping the proposal to the head of the
pending transaction queue such that all other pending transactions are disregarded.
5. The VM calls <code>flush_last_resolved_sequence_number_and_owner_schema_execution</code>, applying
owner and required signature count updates, effectively bumping the proposal to the head
of the pending transaction queue such that all other pending transactions are
disregarded.
6. The VM calls <code>successful_transaction_execution_cleanup</code>.

An optional new number of signatures must be passed as a singleton vector due to entry
function constraints: pass either an empty vector for no update, or a singleton vector with
one element specifying the new number of signatures required.

An expiry time in UNIX seconds is required to safeguard against untimely execution.


<pre><code>entry <b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_next_sequence_number_and_owner_schema">flush_next_sequence_number_and_owner_schema</a>(_new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, _owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, _optional_new_num_signatures_required_as_vector: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u64&gt;)
<pre><code>entry <b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema">flush_last_resolved_sequence_number_and_owner_schema</a>(_new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, _owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, _optional_new_num_signatures_required_as_vector: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u64&gt;, _expiry_in_unix_seconds: u64)
</code></pre>


Expand All @@ -2130,10 +2143,11 @@ one element specifying the new number of signatures required.
<summary>Implementation</summary>


<pre><code>entry <b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_next_sequence_number_and_owner_schema">flush_next_sequence_number_and_owner_schema</a>(
<pre><code>entry <b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema">flush_last_resolved_sequence_number_and_owner_schema</a>(
_new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;,
_owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;,
_optional_new_num_signatures_required_as_vector: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u64&gt;,
_expiry_in_unix_seconds: u64
) {}
</code></pre>

Expand Down Expand Up @@ -2246,18 +2260,18 @@ Check next sequence number to execute if none specified.

</details>

<a name="0x1_multisig_account_flush_next_sequence_number_and_owner_schema_execution"></a>
<a name="0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema_execution"></a>

## Function `flush_next_sequence_number_and_owner_schema_execution`
## Function `flush_last_resolved_sequence_number_and_owner_schema_execution`

Flush pending transactions and owner schema in reponse to a DoS attack.

Bumps a flush transaction proposal to the front of the pending transaction queue, then
updates the next transaction proposal sequence number such all other pending transactions
are ignored, to be overwritten by future transaction proposals.
Bumps a flush transaction proposal to the front of the pending transaction queue, by
updating the last resolved sequence number such that all pending transactions
are ignored.


<pre><code><b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_next_sequence_number_and_owner_schema_execution">flush_next_sequence_number_and_owner_schema_execution</a>(multisig_address: <b>address</b>, new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, optional_new_num_signatures_required: <a href="../../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option_Option">option::Option</a>&lt;u64&gt;, flush_transaction_proposal_sequence_number: u64)
<pre><code><b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema_execution">flush_last_resolved_sequence_number_and_owner_schema_execution</a>(multisig_address: <b>address</b>, new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;, optional_new_num_signatures_required: <a href="../../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option_Option">option::Option</a>&lt;u64&gt;, expiry_in_unix_seconds: u64, flush_transaction_proposal_sequence_number: u64)
</code></pre>


Expand All @@ -2266,20 +2280,26 @@ are ignored, to be overwritten by future transaction proposals.
<summary>Implementation</summary>


<pre><code><b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_next_sequence_number_and_owner_schema_execution">flush_next_sequence_number_and_owner_schema_execution</a>(
<pre><code><b>fun</b> <a href="multisig_account.md#0x1_multisig_account_flush_last_resolved_sequence_number_and_owner_schema_execution">flush_last_resolved_sequence_number_and_owner_schema_execution</a>(
multisig_address: <b>address</b>,
new_owners: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;,
owners_to_remove: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;<b>address</b>&gt;,
optional_new_num_signatures_required: Option&lt;u64&gt;,
flush_transaction_proposal_sequence_number: u64,
expiry_in_unix_seconds: u64,
flush_transaction_proposal_sequence_number: u64
) <b>acquires</b> <a href="multisig_account.md#0x1_multisig_account_MultisigAccount">MultisigAccount</a> {
// Verify that the proposal <b>has</b> not yet expired.
<b>assert</b>!(
expiry_in_unix_seconds &lt; now_seconds(),
<a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_invalid_state">error::invalid_state</a>(<a href="multisig_account.md#0x1_multisig_account_EFLUSH_TXN_EXPIRED">EFLUSH_TXN_EXPIRED</a>)
);
<b>let</b> multisig_account_ref_mut =
<b>borrow_global_mut</b>&lt;<a href="multisig_account.md#0x1_multisig_account_MultisigAccount">MultisigAccount</a>&gt;(multisig_address);
<b>let</b> transactions_table_ref_mut =
&<b>mut</b> multisig_account_ref_mut.transactions;
// Get new sequence number for flush transaction proposal.
<b>let</b> new_flush_transaction_proposal_sequence_number =
multisig_account_ref_mut.last_executed_sequence_number + 1;
multisig_account_ref_mut.next_sequence_number;
// Update flush transaction proposal sequence number.
<b>let</b> flush_transaction_proposal = <a href="../../aptos-stdlib/doc/table.md#0x1_table_remove">table::remove</a>(
transactions_table_ref_mut,
Expand All @@ -2293,6 +2313,9 @@ are ignored, to be overwritten by future transaction proposals.
// Update sequence number for next transaction proposal.
multisig_account_ref_mut.next_sequence_number =
new_flush_transaction_proposal_sequence_number + 1;
// Update sequence number for last resolved transaction proposal.
multisig_account_ref_mut.last_executed_sequence_number =
new_flush_transaction_proposal_sequence_number - 1;
<a href="multisig_account.md#0x1_multisig_account_flush_owner_schema">flush_owner_schema</a>(
multisig_address,
new_owners,
Expand Down
35 changes: 25 additions & 10 deletions aptos-move/framework/aptos-framework/sources/multisig_account.move
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ module aptos_framework::multisig_account {
const EINVALID_SEQUENCE_NUMBER: u64 = 17;
/// Provided owners to remove and new owners overlap.
const EOWNERS_TO_REMOVE_NEW_OWNERS_OVERLAP: u64 = 18;
/// Flush transaction proposal has expired.
const EFLUSH_TXN_EXPIRED: u64 = 19;

/// Represents a multisig account's configurations and transactions.
/// This will be stored in the multisig account (created as a resource account separate from any owner accounts).
Expand Down Expand Up @@ -802,18 +804,22 @@ module aptos_framework::multisig_account {
/// on-chain then the payload is deserialized and verified, but if the proposal stores only
/// a payload hash, then the payload included in the Aptos API MultisigFlush transaction is
/// instead verified.
/// 5. The VM calls `flush_next_sequence_number_and_owner_schema_execution`, applying owner and
/// required signature count updates, effectively bumping the proposal to the head of the
/// pending transaction queue such that all other pending transactions are disregarded.
/// 5. The VM calls `flush_last_resolved_sequence_number_and_owner_schema_execution`, applying
/// owner and required signature count updates, effectively bumping the proposal to the head
/// of the pending transaction queue such that all other pending transactions are
/// disregarded.
/// 6. The VM calls `successful_transaction_execution_cleanup`.
///
/// An optional new number of signatures must be passed as a singleton vector due to entry
/// function constraints: pass either an empty vector for no update, or a singleton vector with
/// one element specifying the new number of signatures required.
entry fun flush_next_sequence_number_and_owner_schema(
///
/// An expiry time in UNIX seconds is required to safeguard against untimely execution.
entry fun flush_last_resolved_sequence_number_and_owner_schema(
_new_owners: vector<address>,
_owners_to_remove: vector<address>,
_optional_new_num_signatures_required_as_vector: vector<u64>,
_expiry_in_unix_seconds: u64
) {}

////////////////////////// To be called by VM only ///////////////////////////////
Expand Down Expand Up @@ -885,23 +891,29 @@ module aptos_framework::multisig_account {

/// Flush pending transactions and owner schema in reponse to a DoS attack.
///
/// Bumps a flush transaction proposal to the front of the pending transaction queue, then
/// updates the next transaction proposal sequence number such all other pending transactions
/// are ignored, to be overwritten by future transaction proposals.
fun flush_next_sequence_number_and_owner_schema_execution(
/// Bumps a flush transaction proposal to the front of the pending transaction queue, by
/// updating the last resolved sequence number such that all pending transactions
/// are ignored.
fun flush_last_resolved_sequence_number_and_owner_schema_execution(
multisig_address: address,
new_owners: vector<address>,
owners_to_remove: vector<address>,
optional_new_num_signatures_required: Option<u64>,
flush_transaction_proposal_sequence_number: u64,
expiry_in_unix_seconds: u64,
flush_transaction_proposal_sequence_number: u64
) acquires MultisigAccount {
// Verify that the proposal has not yet expired.
assert!(
expiry_in_unix_seconds < now_seconds(),
error::invalid_state(EFLUSH_TXN_EXPIRED)
);
let multisig_account_ref_mut =
borrow_global_mut<MultisigAccount>(multisig_address);
let transactions_table_ref_mut =
&mut multisig_account_ref_mut.transactions;
// Get new sequence number for flush transaction proposal.
let new_flush_transaction_proposal_sequence_number =
multisig_account_ref_mut.last_executed_sequence_number + 1;
multisig_account_ref_mut.next_sequence_number;
// Update flush transaction proposal sequence number.
let flush_transaction_proposal = table::remove(
transactions_table_ref_mut,
Expand All @@ -915,6 +927,9 @@ module aptos_framework::multisig_account {
// Update sequence number for next transaction proposal.
multisig_account_ref_mut.next_sequence_number =
new_flush_transaction_proposal_sequence_number + 1;
// Update sequence number for last resolved transaction proposal.
multisig_account_ref_mut.last_executed_sequence_number =
new_flush_transaction_proposal_sequence_number - 1;
flush_owner_schema(
multisig_address,
new_owners,
Expand Down
Loading

0 comments on commit ca4cc0f

Please sign in to comment.