feat: support contract upgrades in batches by DSharifi · Pull Request #3361 · near/mpc

DSharifi · 2026-05-26T11:55:28Z

closes #2987

DSharifi · 2026-05-26T11:58:05Z

@claude review

…atches

DSharifi · 2026-05-31T23:24:24Z

@claude review

claude · 2026-05-31T23:29:23Z

Pull request overview

Adds a chunked-upload flow (start_contract_upload → upload_contract_chunk → finalize_contract_upload, with clear_staged_contract to abandon) so contract binaries above the ~1.5 MiB single-tx RPC limit can be proposed. Existing propose_update is narrowed to config-only and its borsh wire format becomes { config: Config } (no more Option<Vec<u8>> code field). Sandbox helpers and devnet tooling are migrated to the new flow; a test-large-contract crate is added that pads a contract to ~2 MiB so the chunked path is exercised end-to-end.

Changes:

New endpoints start_contract_upload, upload_contract_chunk, finalize_contract_upload, clear_staged_contract plus per-account StagedContractUpload metadata in IterableMap/LookupMap
ProposeUpdateArgs reshaped to config-only; TryFrom → From
v3.10→3.11 state migration initializes new staged-upload maps empty
Sandbox test infra: propose_and_vote_contract_binary switches to chunked; legacy inline helper kept for upgrades from 3.10.1 (which lacks chunked endpoints)
crates/test-large-contract: ~2 MiB padded contract built via build.rs to exercise the >1.5 MiB path
Devnet MpcProposeUpdateContractCmd rewritten to drive the chunked flow

Reviewed changes

Per-file summary

File	Description
`crates/contract/src/lib.rs`	Adds four new endpoints, two new state fields, wires them into all constructors/test fixtures
`crates/contract/src/update.rs`	`ProposeUpdateArgs` shape change, new `StagedContractUpload{Args}`/`UploadContractChunkArgs` and `StagedContractUpload::{record_chunk,is_complete,required_deposit_for_bytes}`
`crates/contract/src/storage_keys.rs`	Appends `StagedContractUploads`, `StagedContractChunks`
`crates/contract/src/v3_10_state.rs`	Default-initializes the two new maps during migration
`crates/contract/tests/sandbox/common.rs`	New `chunked_upload_contract` helper, legacy `propose_and_vote_contract_binary_inline` kept for production-binary tests
`crates/contract/tests/sandbox/upgrade_from_current_contract.rs`	Existing tests ported to chunked flow + 5 new chunked-upload integration tests
`crates/contract/tests/sandbox/upgrade_to_current_contract.rs`	Uses inline helper to upgrade from 3.10.1
`crates/contract/tests/sandbox/contract_configuration.rs`, `update_votes_cleanup_after_resharing.rs`	Ported to chunked flow
`crates/contract/tests/sandbox/utils/contract_build.rs`	Adds `large_contract()` builder
`crates/devnet/src/mpc.rs`	Rewrites the propose-update CLI for the chunked flow
`crates/e2e-tests/src/cluster.rs`	Documents the local borsh mirror as the legacy shape
`crates/near-mpc-contract-interface/src/method_names.rs`	New method name constants
`crates/test-large-contract/*`	New ~2 MiB padded contract
`Cargo.{toml,lock}`, `crates/contract/{README.md,snapshots/,tests/snapshots/}`	Workspace registration, README endpoint docs, snapshot updates

Findings

Blocking (must fix before merge):

crates/contract/src/lib.rs:216 — finalize_contract_upload registers an entry in proposed_updates.entries without validating that accumulated deposits cover the entry's required storage. The old propose_update enforced attached >= ProposedUpdates::required_deposit(&update) and refunded the surplus (lib.rs:1198-1220). In the new flow:
- Per-chunk deposit only covers storage_byte_cost * chunk_len, so the sum across chunks covers storage_byte_cost * code.len() — but bytes_used(&Update::Contract(_)) is sizeof(UpdateEntry) + 128 * sizeof(AccountId) + code.len() (update.rs:278-295), i.e. ~8 KiB / ~0.08 NEAR of overhead per proposal that no caller ever pays for.
- Any per-chunk overpayment is silently retained — clear_staged_contract refunds staged.deposited, but finalize_contract_upload simply drops it.
Fix: in finalize_contract_upload, compare staged.deposited against ProposedUpdates::required_deposit(&Update::Contract(code)), fail if short, and Promise::transfer the surplus back to caller (mirror lib.rs:1216-1220). The StagedContractUpload::deposited doc-comment already claims this is what happens ("consumed by the proposal entry's storage cost on finalize_contract_upload") — the code doesn't match the contract.
crates/contract/src/lib.rs:272 — clear_staged_contract requires voter_or_panic(), so a voter who calls start_contract_upload and then is removed during resharing has no way to clean up: their staged_uploads/staged_chunks entries and their deposit are stranded forever. remove_non_participant_update_votes (lib.rs:1608) already exists for the analogous vote-cleanup case; there's no equivalent for staged uploads. Either (a) drop the voter_or_panic requirement in clear_staged_contract — self-cleanup of one's own upload is harmless to other voters — or (b) wire staged-upload cleanup into the resharing path alongside remove_non_participant_votes. (a) is the cheaper fix.

Non-blocking (nits, follow-ups, suggestions):

crates/contract/src/lib.rs:124 — start_contract_upload accepts any total_size: u64. Declaring 10 GiB doesn't immediately allocate but the metadata entry persists until cleared, and Vec::with_capacity(total_size as usize) in finalize_contract_upload:248 truncates on wasm32 if total_size > u32::MAX. A reasonable cap (e.g. 5 MiB or whatever protocol RT enforces for deploy_contract) would catch obviously-malformed inputs at start time rather than letting the upload accumulate state first.
crates/contract/src/lib.rs:142 — start_contract_upload doesn't validate that attached_deposit() covers the storage cost of the new staged_uploads entry itself. Small (~hundreds of bytes), but every other entry-creation site in this contract validates.
crates/devnet/src/mpc.rs:493 — deposit_per_chunk = (self.deposit_near * ONE_NEAR) / (num_chunks as u128) uses integer division. For e.g. deposit_near=1, num_chunks=3, each chunk gets 0 yocto and upload_contract_chunk rejects. Either round up (div_ceil) or compute per-chunk from storage_byte_cost * chunk_len directly and ignore deposit_near.
crates/contract/src/update.rs:466 — StagedContractUpload::deposited uses saturating_add on every chunk — a u128 overflow here is unreachable in practice but, if it ever did saturate, the user would silently lose funds on clear. checked_add + error matches the style of record_chunk.
Borsh wire-format break of ProposeUpdateArgs (code field removed) is intentional and called out in the README, but worth being explicit in release notes — any in-flight client still sending { code: Some(...), config: None } will get a borsh-deserialize failure post-upgrade, and tools must adopt the chunked flow before clients can propose code again.

⚠️ Issues found

…atches

netrome

Mostly looks good to me. Nothing strictly blocking, but a few things I would like to change:

I'd like to rename the existing propose_update method to propose_config_update as it no longer is used for anything else than config updates. This should be fine since it's a method we call.
I don't see the point of emitting logs in the new methods. I suggest skipping it (YAGNI).
There are some as conversion. You know my preference on these.

gilcu3

Thank you very much! Looks like it should work!

I have a few small blockers:

contract size check (the bump seems no longer needed)
the contract state migration assumes 3.10 (needs to be fixed by another PR)
the lack of update_gas constants, which serve as an upper bound in tests.

I am also a bit concerned about the deposit logic, which does not seem trivial. Would be good to get that sorted out

I hoped we could do this change so that we could revert back to a single chunk easily when we get our contract size down, but the current implementation does not seem trivial to revert.

gilcu3 · 2026-06-05T08:26:40Z

small blocker: This file cannot be modified until #3473 lands

gilcu3 · 2026-06-05T08:50:43Z

+    uploader
+        .call(contract.id(), method_names::START_CONTRACT_UPLOAD)
+        .args_borsh(StartContractUploadArgs {
+            total_size: std::num::NonZeroU64::new(chunk.len() as u64).unwrap(),
+        })
+        .max_gas()
+        .deposit(NearToken::from_yoctonear(1))
+        .transact()
+        .await?
+        .into_result()
+        .map_err(|e| anyhow::anyhow!("start_contract_upload failed: {e}"))?;


we should probably measure and set a gas constant here instead of max_gas

gilcu3 · 2026-06-05T08:55:28Z

+    // This vote crosses the threshold and triggers deploy_contract + migrate
+    // on a multi-MiB binary; use max_gas so chunk reassembly + deploy fit.
    let execution = mpc_signer_accounts[1]
        .call(contract.id(), method_names::VOTE_UPDATE)
        .args_json(serde_json::json!({
            "id": proposal_b,
        }))
-        .gas(GAS_FOR_VOTE_UPDATE)
+        .max_gas()


as I said in some comments above, we should still track how much gas is that, by keeping the constant

gilcu3 · 2026-06-05T08:56:26Z

+        .await;
+
+    let code = current_contract();
+    assert!(code.len() > 1024, "contract binary should be non-trivial");


this is a strange restriction. Why would the test care about that?

gilcu3 · 2026-06-05T09:08:02Z

+
+        for (account, num_chunks, deposited) in orphaned {
+            for i in 0..num_chunks {
+                self.staged_chunks.remove(&(account.clone(), i));
+            }
+            self.staged_uploads.remove(&account);
+            if deposited > NearToken::from_yoctonear(0) {
+                Promise::new(account).transfer(deposited).detach();
+            }
+        }
+


how are we ensuring that this does not consume more gas than we are paying for it?

gilcu3 · 2026-06-05T09:09:09Z

    /// Cleans update votes from non-participants after resharing.
    /// Can be called by any participant or triggered automatically via promise.
    #[handle_result]
    pub fn remove_non_participant_update_votes(&mut self) -> Result<(), Error> {
        log!(
            "remove_non_participant_update_votes: signer={}",
            env::signer_account_id()
        );


isn't this comment a bit misleading? It mentions online participants can call it, but it seems anyone can actually call it

…atches

DSharifi added 2 commits May 26, 2026 13:54

.

109f6ab

fix: update abi

db9e211

DSharifi marked this pull request as ready for review May 26, 2026 12:45

DSharifi added 11 commits May 27, 2026 14:52

Merge remote-tracking branch 'origin/main' into dsharifi/upgrade-in-b…

2a09d73

…atches

part of merge

905548f

fix ci

3e8b452

merge from main

c9d42e7

.

be0c04e

test: top up the account during chunked upload

b9bc3fa

cargot fmt

2f20744

remove old upload way

9527cb6

remove comments

9d71892

keep proposal agnostic of chunks

a3172db

refactor: uploading chunked should just be a transport

ff5c144

DSharifi added 5 commits June 1, 2026 01:42

fix blocking comments

bf78fad

Merge remote-tracking branch 'origin/main' into dsharifi/upgrade-in-b…

c5dd7bf

…atches

bump hard limit, still within 1,5 MiB

bd3f16d

remove claude comments

6e550bd

update insta

c429ba6

netrome mentioned this pull request Jun 1, 2026

refactor: Replace launcher vote with generic votes struct #2935

Draft

netrome previously approved these changes Jun 4, 2026

View reviewed changes

Comment thread crates/contract/src/update.rs Outdated

Comment thread crates/contract/src/lib.rs Outdated

Comment thread crates/contract/src/lib.rs Outdated

Comment thread crates/contract/src/lib.rs Outdated

Comment thread crates/contract/src/lib.rs Outdated

gilcu3 requested changes Jun 5, 2026

View reviewed changes

DSharifi added 2 commits June 5, 2026 11:55

chore: remove logs

f860197

chore!: rename propose_update to propose_config_update

9af8912

DSharifi dismissed netrome’s stale review via 9af8912 June 5, 2026 10:25

DSharifi added 2 commits June 5, 2026 13:41

fix: store data lengths and chunk sizes as usize to avoid conversions

baa80a6

remove comment mentioning contract version for legacy upgrade path

e4036a2

DSharifi added 11 commits June 5, 2026 14:02

chore: remove contract check limit

500eb46

fix tests to use atomicusize

51b5bc5

use rand for uncompressable bytes

9b23ea9

simplify comment, nothing compressed embedded data

affcdf4

use NonZeroUsize in sandbox tests as well

c831035

use legacy struct in common and fix usize

2336972

Merge remote-tracking branch 'origin/main' into dsharifi/upgrade-in-b…

d6fd524

…atches

fix: update the abi

7814180

refactor: needles as conversion

3dfa88e

test(fix): code field comes before config

05ed6a4

test: use legacy types in e2e tests and also move type to interface

f91211b

Conversation

DSharifi commented May 26, 2026 • edited by netrome Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DSharifi commented May 26, 2026

Uh oh!

DSharifi commented May 31, 2026

Uh oh!

claude Bot commented May 31, 2026

Pull request overview

Reviewed changes

Findings

Uh oh!

netrome left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gilcu3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

gilcu3 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DSharifi commented May 26, 2026 •

edited by netrome

Loading

gilcu3 left a comment •

edited

Loading