Multisig and accounts hard fork *draft*#2892
Draft
theodorebugnet wants to merge 33 commits into
Draft
Conversation
theodorebugnet
force-pushed
the
theodore/multisig-upgrade
branch
from
ca25d8f to
c1b91e2
Compare
citizen-stig
force-pushed
the
theodore/multisig-upgrade
branch
from
3ea3787 to
2e71f00
Compare
ross-weir
force-pushed
the
theodore/multisig-upgrade
branch
from
8e46ba6 to
35f6d27
Compare
* Unify versioned tx generics and simplify serialization * clippy * Rename to_unsigned methods to as_unsigned because they clone rather than consuming
) * Adding account -> [credential_id] mapping * Review: part 1 * Updating docs and more simplification * Reverting dual-write * Updating documentation * Update json schemas * PR feedback fixes part 1 * Revert and print receipt on error * Explicitly authorize embedded credential in sov-hyperlane-solana-register As it was done before * Rollback fluff part 1 * Revert accidental switcheroo * Clean up * Reformat readme for readability * More info in genesis bail * Simplify part n * Initial migrations implementation * Clean up * Fix migrations and add custom REST API for accounts module * Remove legacy account map reading from custom REST API. Extract common migration logic * Fixes * Address README.md comments
* Accounts Refactor PR 2: Adding target_account * Updating typescript to match new transaction * Fixing typescript * Continue fixing typescript * Fixing lint * Updating README tests part 1 * Documentation formatting * Addressing the feedback * Fixes after rebase * Fix after rebase * Simlify and fix * Rename target_address to address_override * Address comments * Keep fixing * More cleaning up * More cleaning up * Reformat comments * Regenrate rollup_resync and start updating readme tests * Bring back account_2 * Addressing feedback * Update EIP-712 tests * More typescript clean ups and comments * Fixes * More fixes * update hyperlane image so tests are suppose to pass * Lint! * use lander based image * Update crates/module-system/sov-modules-api/src/transaction/types/v0.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Update crates/module-system/sov-modules-api/src/transaction/types/v1.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Update crates/module-system/sov-solana-offchain-auth/src/authentication/payload.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Update crates/module-system/sov-solana-offchain-auth/src/authentication/payload.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Update crates/module-system/sov-modules-api/src/transaction/unsigned/v0.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Update crates/module-system/sov-modules-api/src/transaction/unsigned/v1.rs Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com> * Addressing feedback --------- Co-authored-by: Theodore Bugnet <24320578+theodorebugnet@users.noreply.github.com>
…#2773) * PR 3: Adding more call messages to accounts module * Adding swap credential id * Simplify round 1 * Remove new `TryDecodeCredentialId` trait * Address PR feedback * capabilities: trust authenticator default address on None override Description When `address_override` is `None`, `resolve_authorized_sender()` was re-checking `auth_data.default_address` through `Accounts::is_authorized_for()`. That works when the authenticator's default address matches `credential_id.into()`, but it breaks authenticators whose default address uses a different address form. For `sov-evm` with `MultiAddressEvm`, the authenticator sets `default_address` to `Vm(eth_addr)`, while the canonical fallback in `is_authorized_for()` is `Standard(credential_id.into())`. Ordinary EVM transactions were therefore skipped with "not authorized for resolved address" unless an explicit `(vm_address, credential_id)` mapping had already been inserted. Fix the shared `address_override = None` path to trust the authenticator-selected default address unless that exact `(address, credential_id)` pair is explicitly denied in `account_owners`. Keep `Some(address_override)` strict, and preserve revocation semantics for explicit `false` entries. This restores the default EVM sender path without changing EVM auth semantics or requiring pre-populated account mappings. * More fixes * Fixing typescript feedback
…ey (#2850) * accounts: wire sov-chain-state module dependency PR 4 will use the visible DA slot hash to derive unknown addresses, so add `sov-chain-state` as a workspace dependency and embed it as a `#[module]` field on `Accounts<S>`. Drop the `cfg_attr(feature = "arbitrary", derive(Debug))` because `ChainState<S>` does not implement `Debug`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * accounts: add CreateUnknownAddress call message Adds a fifth `CallMessage` variant that deterministically derives a fresh "unknown" address — an address with no naturally-corresponding private key — from the visible DA slot hash, sender address, sender credential, and a caller-supplied salt, and auto-authorizes the caller's current credential for it. Re-issuing the same `(caller, salt)` in the same visible slot is an idempotent no-op; revocation goes through the existing `RemoveCredentialFromAddress` path. The handler supports the V0 single-signer and V1 multisig credential types; other credential types are rejected. Also lifts `sov_accounts::Module::Event` from `()` to a real `Event<S>` enum with a single `UnknownAddressCreated` variant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * accounts: integration tests for CreateUnknownAddress Adds six integration tests covering the happy path, idempotent re-issuance in the same slot, distinct addresses across salts and across callers, the long-form multisig flow that creates an unknown address, funds it via bank transfer, swaps a credential, and rotates to a fresh multisig, and the `enable_custom_account_mappings = false` guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * accounts: document unknown-address support Adds a sixth bullet to the sov-accounts README describing `CreateUnknownAddress` and the derivation formula, and a breaking-change entry to the MULTISIG UPGRADE section of the CHANGELOG. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fixes and review from codex * Update guest locks * Clean up 1 * Rename and clean up * Updated cargo locks finally * Updates after rename * Fixing typescript CI * More CI fixes and changelog fix * Address feedback --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2853) * Fix typo in oprating_mode * Adding one more explicit test for revoked addresses `Some(false)` * Adding events * Disable non-synthetic addresses on genesis * Fixing REST API * Remove confusing message * Addressing the items * Update CHANGELOG.md entry * Update schema * Address feedback about events and CHANGELOG.md * Rewrite test description * Added comment
Add version to chain_state and constants.toml, cross-check on startup
UnsignedTransaction is now a versioned enum (V0/V1) and the V0 variant
gained an address_override field. The test JSON for the
window-uniqueness case was missing both the `{"V0": {...}}` envelope
and `"address_override": null`, causing schema parse to fail. The
sibling `some_gas_limit` and `none_gas_limit` tests were already
updated; this catches up the third.
* Charge borsh deserialization gas per byte actually read
Adds MeteredReader, a Read adapter that charges gas on every read/read_exact;
auto-applied to all `borsh::BorshDeserialize` types via a blanket impl on
MeteredBorshDeserialize. Cost now tracks actual decode work (length prefixes,
tag bytes, field bodies) rather than `bias + per_byte × buf.len()`, which
couldn't fairly price `Transaction<R, S, C>` because per-byte cost depends on
the encoded shape, not the byte count.
Collapses four shape-specific `(bias, per_byte)` constant pairs into one global
pair (`BORSH_PER_BYTE_READ`, `BORSH_PER_READ_BIAS`, inert at [0, 0] pending
calibration) plus the retained common `BIAS_BORSH_DESERIALIZATION`. Blob-storage
and state-codec sites that decode opaque payloads outside the trait keep
upfront-byte charging via a dedicated `GAS_TO_CHARGE_PER_BYTE_BLOB_DECODE_UPFRONT`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Wrap gas errors in io::Error instead of stashing on MeteredReader
`MeteredReader::charge` now returns `io::Error::other(gas_err)`; the reader
no longer carries a `stashed` field or `take_stashed_error()` method.
`deserialize_from_slice` recovers the typed `GasMeteringError` via
`io::Error::downcast`. Reader is now purely functional.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Group borsh-related gas constants together
Move BORSH_PER_BYTE_READ, BORSH_PER_READ_BIAS, and
GAS_TO_CHARGE_PER_BYTE_BLOB_DECODE_UPFRONT next to BIAS_BORSH_DESERIALIZATION
in constants.toml / constants.testing.toml and in the GasSpec trait + default
impls. JSON deserialization constants get their own section below.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Test work-based metering via env overrides; drop charge panic
Two tests in metered_utils.rs override BORSH_PER_BYTE_READ /
BORSH_PER_READ_BIAS via SOV_TEST_CONST_OVERRIDE_* env vars and exercise
the real deserialize_from_slice path. MeteredReader::charge no longer
panics on u32 overflow — returns io::Error::other instead.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Recover gas error in deserialize_reader, not just deserialize_from_slice
Push the io::Error → GasError downcast into `deserialize_reader` so direct
callers see the typed error too. `deserialize_from_slice` no longer redoes it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Test deserialize_from_slice advances buf by bytes consumed
Decodes a BorshTestStruct preceded by trailing bytes and asserts the
post-decode slice equals exactly the trailing bytes — catches off-by-one
in the cursor.position()-to-slice advance.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Collapse blob-decode-upfront constant into BORSH_PER_BYTE_READ
Drop GAS_TO_CHARGE_PER_BYTE_BLOB_DECODE_UPFRONT and route blob-storage,
state-codec, and reward-burn sites through BORSH_PER_BYTE_READ instead.
Same unit cost (per-byte borsh decode); the two constants only differed
in charge schedule (upfront vs metered).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Simplify MeteredReader and its tests
Use as_u32_or_panic for the usize→u32 conversion in MeteredReader::charge
matching the precedent in metered_utils.rs. Narrow new_with_prices to
pub(crate) since only tests use it. Unify the two budget helpers across
gas::tests into a shared budget_for_reader_calls. Trim per-constant doc
comments and the read_exact override comment to one short line.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip charging gas for zero-byte reads
A 0-byte `read_exact` previously charged the per-read bias even though
no work was done. `MeteredReader::charge` now no-ops on n=0, making
`read` and `read_exact` consistent for empty buffers and EOF cases.
This eliminates a small over-charge on zero-length collection decodes
(e.g. `Vec<u8>` with length=0 reads the 4-byte prefix then a 0-byte body).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Correct read_exact override rationale
The previous comment claimed the override was needed because
`<&[u8] as Read>::read_exact` bypasses `Read::read` and would skip
metering. That's wrong: dispatch goes through `MeteredReader::read_exact`
either way, and the default impl calls `self.read` which does meter. The
real reason for the override is consistent charging: one read_exact call
should produce one per-read-bias charge regardless of how the inner
reader chunks. Also rename the regression test to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Remove dead borsh env var from reset_constants
DEFAULT_GAS_TO_CHARGE_PER_BYTE_BORSH_DESERIALIZATION was removed when
the borsh constants were grouped; the override in reset_constants was
left dangling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Remove MeteredBorshDeserializeString newtype
The wrapper added nothing functionally — `String: BorshDeserialize`
implies `String: MeteredBorshDeserialize` via the blanket impl in
`sov-modules-api`. Call sites and the transaction generator now use
`String` directly with the same wire format.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Charge for partial-consumed bytes on read_exact failure
`Read::read_exact` is allowed to consume bytes before returning an
error. Previously we delegated to `inner.read_exact` and only charged
on full success, leaving any bytes consumed before an error unmetered
— an attacker controlling the inner source could exploit this for free
prover work. Now we loop on `inner.read` ourselves, accumulate consumed
bytes, and charge at every exit point. On error paths the charge result
is discarded (`let _ =`) so the original io::Error propagates instead
of being clobbered by a gas error.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* linting
* Migrate sequencer_run_out_of_gas to new borsh constant
The test overrode `DEFAULT_GAS_TO_CHARGE_PER_BYTE_BORSH_DESERIALIZATION`
which was removed when borsh constants were grouped. Use
`BORSH_PER_BYTE_READ` instead — same effect (inflates per-byte
deserialization cost to exhaust sequencer gas).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Use io::Error::other in partial-fail test
clippy::io_other_error flagged the `io::Error::new(ErrorKind::Other, _)`
construction; `io::Error::other` is the preferred shorthand.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* add changelog entry
* Update crates/module-system/sov-modules-api/src/gas/metered_reader.rs
Co-authored-by: Nikolai Golub <nikolai@sovlabs.io>
* fix lint
* De-flake test_rollup_resync: poll for state value
check_value queried the state once at slot height+4, assuming the node
had committed the value by then. Under CI load the node lags the slot
subscription, so the read hit {"value":null} and the client panicked
deserializing null into a sequence. Poll with a 30s deadline until the
value populates instead. Data is unchanged (verified by regeneration);
this is purely the test's timing assumption.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* clean
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Nikolai Golub <nikolai@sovlabs.io>
* Add SP1 microbenches for borsh deserialization gas calibration Three sub-commands isolate each constant: `borsh reader-bytes` sweeps payload size to calibrate `BORSH_PER_BYTE_READ`, `borsh reader-count` sweeps read count to calibrate `BORSH_PER_READ_BIAS`, and `borsh decode-vec` measures end-to-end `deserialize_from_slice::<Vec<u8>>` to calibrate `BIAS_BORSH_DESERIALIZATION` (using the prior two values to back out the per-call bias). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Apply setup-noise methodology fixes to borsh microbench Guest now uses constant-size source/output buffers (MAX_BYTES, MAX_READS) regardless of sweep point, making per-execution setup N-independent and cleaning the per-byte slope. Host now runs each sweep twice (high iter + baseline iter) and computes per-iter prover gas via subtraction, cancelling iteration-independent SP1 setup and cleaning the per-call bias intercept. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Document microbench methodology choice; round constants up to 1 README now explains when to use single-pass vs two-iter differencing (precompile-heavy vs cheap-per-cycle work). Borsh suggested-constant output now rounds positive sub-1 values up to 1 instead of flooring to 0, which would have effectively unmetered the operation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Factor sweep helper in cmd/borsh.rs and tighten README The three subcommands shared the same validate/load-ELF/double-loop/ differential/fit/print scaffolding. Extract `run_sweep` taking a closure for the per-sweep-point stdin payload; each subcommand now reduces to the call plus its own `print_fit` and suggested-constant block. README methodology section trimmed to the heuristic plus one explanatory paragraph. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add `borsh all` subcommand to chain calibration steps Single command runs reader-bytes, reader-count, and decode-vec in sequence, plumbing the fitted slope and per-read-bias between steps so no manual value copying is needed. Prints a final summary with the three calibrated constants in raw form and as X/2-split 2D values ready for constants.toml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * bump lockfile * clean * linting * only insert into 0-index constant * tidy * fix --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Clean up `EVM_RECEIPT_ACTUAL_FEE_HEIGHT` * Apply suggestion from @citizen-stig
Refactor core transaction signing types to separate the consumer-facing unsigned payload from the canonical signing payload.
* What used to be called UnsignedTransaction{, V0, V1} is now called TransactionSigningPayload{, V0, V1}. This is the type that is serialized for signing.
* Serialization logic is centralised to this Payload, in particular the chain_hash is now a field on the Payload rather than manually apended.
* A new type, called UnsignedTransaction, is added to encapsulate the actual user-defined parts of the transaction.
This removes the old ambiguity where “unsigned transaction” sometimes meant “user payload” and sometimes meant “exact signed preimage”.
Add `version: 0` field to V0 solana offchain transactions
…g surprised by a mismatch error (#2938)
* sov-2: fix unchecked fields during deserialization * remove type def
* Make migrations idempotent * Update CHANGELOG
* Add NOMT storage proof-verification microbench Sweeps verify_multi_proof / verify_multi_proof_update over trie depth and prices each access at depth 64. Calibrates GAS_TO_CHARGE_PER_STORAGE_ACCESS and BIAS_STORAGE_UPDATE in prover gas. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Resolve storage bench hasher from the storage spec Read the NOMT hasher off the spec's Storage type (the NomtVerifierStorage being benchmarked) instead of re-deriving it through CryptoSpec, so the bench can't drift from production's BinaryHasher<S::Hasher>. Same type today (sha256). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * lint * bump ver --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Point sp1-microbench borsh/storage cmds at the relocated sov_gas_tools::fit module, and pick up the regenerated rollup-config.json (RollupHeight and the gas-limit-height field dropped with CHANGE_GAS_LIMIT_AFTER_HEIGHT). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Schema (.bin/.json + TS fixture), chain/metadata hashes (schema.test.ts, wallet breaking-change test), README chain_hash/tx_hash, and the resync mock_da.sqlite all reflect the combined chain hash (0x10c4d33e...). mock_da.sqlite regenerated after the gas changes landed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- evm pruning test: TransactionAuthorizer impl now takes `&mut impl StateReader<User>` for resolve_context/resolve_unregistered_context/ check_uniqueness, matching the updated trait (#2908). - auth_eip712 duplicate_tx_is_rejected: pass the value arg (0, matching dev's original encode_message body) now that encode_message is parameterized. - schema.test.ts: update eip712 salt + signing hash for the new chain hash. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Built from the hyperlane-monorepo branch with sov-universal-wallet bumped to the rebased SDK tip, so the agent can decode the current rollup schema (FromSiblingFieldWithOverride + multisig). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The duplicate_tx_is_rejected test (from dev) asserted 148 trailing bytes; the rebased versioned/multisig tx format is 1 byte larger, so it's 149. Also point the hyperlane image comment at the theodore/multisig-upgrade branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
theodorebugnet
force-pushed
the
theodore/multisig-upgrade
branch
from
6c2d33c to
d256a83
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Draft PR to visualise the diff and run CI. Do not merge!
CHANGELOG.mdwith a new entry if my PR makes any breaking changes or fixes a bug. If my PR removes a feature or changes its behavior, I provide help for users on how to migrate to the new behavior.Cargo.tomlchanges before opening the PRs. (Are all new dependencies necessary? Is any module dependency leaked into the full-node (hint: it shouldn't)?)Linked Issues
Testing
Describe how these changes were tested. If you've added new features, have you added unit tests?
Docs
Describe where this code is documented. If it changes a documented interface, have the docs been updated?
Rendered docs are available at
https://sovlabs-ci-rustdoc-artifacts.us-east-1.amazonaws.com/<BRANCH_NAME>/index.html