Adds the two regression tests requested in review:
- verify_selection_consistency: same/default args are accepted; changed
non-default args after setup are rejected.
- my_fee_contribution: a single participant pays the whole kernel cost, two
participants each pay ceil(kernel/2), and the shares cover the full cost.
Also resolves the related TODOs (selection.rs "test that this works",
utils.rs fee-encoding compatibility note).
Addresses PR #754 review (selection.rs, utils.rs).
add_keys added our participant info without checking the slate wasn't already
full, so a tampered slate could exceed num_participants. Block adding when the
participant set is already full and we aren't already in it (re-adding our own
info stays idempotent).
Addresses PR #754 review (slate.rs).
generate_invoice_signature derived the receiver's proof address from the active
account's parent key. Like the output-derivation fix, use context.parent_key_id
so the proof address belongs to the contract's account, not whatever account is
active.
Addresses PR #754 review (proofs.rs, utils.rs).
The committed-fee-exceeded check returned a GenericError. Use the existing
Error::Fee variant and fold the "cancel and retry" hint into the message.
Addresses PR #754 review (selection.rs).
A large u64 amount could wrap when cast `as i64`. Convert with i64::try_from in
the CLI net-change derivation and in contract view, returning an error instead.
get_net_change / to_api_args now return Result (the two arg panics become
errors too), and the to_api_args call moves out of the owner-session closure.
Addresses PR #754 review (command.rs, view.rs).
num_participants is divided into the kernel cost, so 0 caused a divide-by-zero
(inf -> garbage fee). Validate it in context::create, the common setup path for
every party: the initiator's value comes from args and the signer's from the
slate, so this also rejects a tampered slate. Only 1 (self-spend) and 2
(standard) are supported today; lift the upper bound when multi-party contracts
are implemented.
Addresses PR #754 review (wallet_args.rs:1039).
The foreign contract_sign API unwrapped the optional net_change field, so an
incomplete request could panic the wallet. Return an error instead, matching
the foreign contract_new path.
Addresses PR #754 review (foreign.rs).
revoke() cancelled the original tx and built the self-spend using the wallet's
active account, but the locked inputs belong to the account that created the
contract. Revoking while a different account was active would fail to find and
cancel the original tx and would recover funds into the wrong account.
- derive the account from the locked input's root_key_id; use it for cancel_tx
and as the self-spend's src_acct_name.
- test: revoke a contract while a different account is active.
Addresses PR #754 review (revoke.rs).
create_slatepack always serialized to V5, so every slatepack went out V5 and
V4-only peers could not read it. Emit V4 unless the slate carries a payment
proof: only the (opt-in) proof needs the V5-only fields (timestamp/memo) that
V4 cannot represent. The slate's declared version is set to match the emitted
structure so readers select the right variant.
- packer: V5 when payment_proof is present, else V4.
- test: a proof slate round-trips as V5 (memo preserved); a no-proof slate as V4.
Addresses PR #754 review (packer.rs).
contract `new` and the foreign `contract_new` API unwrapped the optional
net_change field, so a request omitting it panicked the wallet. Return an
error instead.
- new::compute and foreign::contract_new: net_change.ok_or_else(..).
- test: contract_new with no net change errors rather than panicking.
Addresses PR #754 review (new.rs, foreign.rs).
get_private_context() flattened "missing context" and real store/keychain
errors into one error, so the contract code created a fresh context on ANY
error (e.g. a decryption failure from a wrong keychain mask), masking the
real problem.
- add Error::NotFoundErr; the grin_store conversion maps NotFoundErr through
faithfully (other store errors stay Backend).
- contract get_or_create / get_net_change / sign create-or-skip only on
NotFoundErr and propagate every other error.
Addresses PR #754 review (context.rs, utils.rs, sign.rs).
Custom output amounts (--make-outputs) and the CLI amount parser converted
grin to nanogrin with `(g * GRIN_BASE as f64) as u64`, which can't represent
all values exactly. Use core::amount_from_hr_string — the same exact integer
parser already used by `send` and `invoice`.
- contract OutputSelectionArgs::output_amounts (covers selection.rs).
- wallet_args::parse_grin_amount.
Addresses PR #754 review (types.rs, selection.rs, wallet_args.rs).
New contract outputs were derived with next_child(), which uses the wallet's
active account. During a late-lock the active account can differ from the one the
contract context was created under, so an output could be created in the wrong
account. Derive under context.parent_key_id instead.
- WalletBackend: add next_child_for(parent_key_id, mask); next_child delegates to it.
- add_outputs_to_ctx derives via next_child_for(&context.parent_key_id, ..).
- parent_key_for returns Result and errors (UnknownAccountLabel) on an unknown
account name instead of silently falling back to the active account.
- test: a contract referencing an unknown account now errors.
Addresses PR #754 review (context.rs, utils.rs).
--encrypt-for is no longer required. `contract new` without it produces an
unencrypted slatepack; `contract sign` without it replies to the incoming slate's
sender when known, otherwise unencrypted. Passing --encrypt-for is unchanged. The
slatepack layer already produced plaintext for an empty recipient list — this only
removes the CLI-side requirement.
Bring the branch current with upstream staging (= master + #748). The earlier
"Merge branch master into contracts" used a stale local master snapshot, which
left the branch ~41 commits behind; this merges the current tip, including the
grin 5.4.0 dependency bump (#745) and the tx-delete / retrieve_txs fixes.
Only Cargo.lock conflicted (regenerated). Full cargo test --workspace green
(52 binaries / 185 tests).
OutputSelectionArgs::output_amounts() unwrap()'d the float parse of each
make-outputs value, panicking on bad input via the API path (the CLI now
validates earlier). Return Result instead; sum_output_amounts() propagates.
num_custom_outputs() now counts CSV entries without parsing, so it stays
infallible and the debug! macros that use it are unaffected.
Bad input to the contract CLI could abort the process:
- a non-numeric --make-outputs value reached output_amounts(), which
unwrap()s the float parse;
- `contract new` with neither --send nor --receive hit a panic in
get_net_change;
- `contract sign` panicked ("No address to encrypt for") when given no
--encrypt-for and an incoming slate with no embedded sender.
Validate --make-outputs and require exactly one of --send/--receive at
parse time (ParseError, before reaching the panicking paths), and return a
typed error instead of panicking when there is no address to encrypt for.
Verified at runtime: each now yields a clean "Invalid Arguments" error.
Remove the broken, commented-out SEC_KEY_FAKE key-scrubbing: a zero secret
key is rejected by secp, and the scrubbing was redundant. The signing
context holding sec_key/sec_nonce is forgotten by deleting the private
context in save_step once signed (retained only at step2 for safe cancel),
and contexts are keyed by slate.id so a nonce is never reused across
messages.
Make that guarantee enforced rather than assumed: after a signed,
non-step2 save_step, verify the private context was actually removed and
error otherwise. Document the invariant at the sign site.
Contract setup/sign and output selection used panic! on
protocol-reachable invariant violations: receiver fee exceeding
net_change, net_change / num_participants mismatch between the setup and
sign phases, a negative change output, conflicting selection args, and a
failed kernel-excess computation. A crafted or inconsistent counterparty
slate could abort the wallet. Return Error::GenericError (or propagate the
underlying error) instead; build_output_amount_list now returns Result.
The contract new/sign/revoke parsers panicked or unwrapped on user input:
conflicting --no-payjoin/--use-inputs, and unparseable receive / send /
num-participants / tx-id values would abort the process. Return
ParseError instead, and add a parse_grin_amount helper for the
Grin->nanogram conversion.
ProofWrap::read unwrapped on the sender/receiver address, promise
signature, and memo_type while parsing untrusted slatepack bytes, so a
malformed payment proof would panic the wallet. Propagate CorruptedData
instead, matching the timestamp path already in this function.
Default ProofArgs.suppress_proof to true. With it false, the receiver's
`contract sign` unconditionally built an invoice promise, but the sender
address is never plumbed through, so a bare send/receive failed with
NoSenderAddressProvided (#729). The CLI never sets this field, so every
default `contract sign --receive` hit the broken path; the basic tests
passed only because they set suppress_proof = true by hand.
Drop those workarounds from contract_srs/contract_rsr so they now exercise
the default path, and opt the proof-using tests (contract_early_proofs_srs,
contract_early_proofs_rsr, contract_accounts) into proofs explicitly.
master's mwixnet (#726) keeps the `types` submodule private and re-exports
its items, so use the public `mwixnet::MixnetReqCreationParams` path. The
merge missed this because `cargo check --workspace` does not build test
targets.
Bring the contracts feature branch up to current master, which has since
landed rust 1.80 support (#722), the canonical mwixnet onion code (#726),
and node-client proxy support (#738).
Conflict resolution:
- libwallet/src/mwixnet/*: take master's (#726), dropping the contracts
branch's older divergent copy. owner.rs::create_mwixnet_req updated to
master's API (use_test_rng) and master's flat re-export imports.
- Cargo manifests/lock/.cargo: master's deps & version; re-add serde_with
for Slate v5; keep contracts' .cargo/config.toml (Windows static-CRT).
- lib.rs: keep serde_derive AND serde_with macro_use; single mwixnet decl.
- error.rs: union of master + contracts variants; flat mwixnet re-exports.
- api/libwallet owner.rs: master's #738 + create_mwixnet_req plus the
contract_* entry points; drop duplicate older create_mwixnet_req method.
- slatepack/armor.rs, node_clients/resp_types.rs: take master's.
Workspace builds; libwallet lib tests pass (34/34). The Slate version()
bug, issue #729 fix, and de-panicking follow in subsequent commits.
* scan: save last scanned block info for initial wallet scanning
* scan: save start height at scanned block info on outputs collecting
* scan: 10k blocks batches
* fix: include last height into batch on scan
* scan: save hash of last block
* scan: add larger window (2880 blocks) to scan outputs when updating wallet state
* add test batched scan and resume, add scan doc
* rename corrupted restore test
---------
Co-authored-by: Joerg <wiesche89@googlemail.com>
jwinterm