contract view derived its result without checking the slate. Reject an Unknown
slate state and an out-of-range num_participants (mirroring context::create), so
a malformed or tampered slate can't feed a bogus view to a caller/UI. Also fix
num_sigs, which counted participants still missing a signature rather than the
signatures present.
Addresses PR #754 review (view.rs).
revoke builds the self-spend via new(), which used a random slate id, so a crash
between new() and sign() left an orphaned self-spend and a retry created another
one. Derive the self-spend slate id deterministically from the revoked slate
(blake2b, already a dependency) and let contract::new take an optional slate id;
a retry then reuses the in-progress context via get_or_create instead of
orphaning a fresh self-spend. The other new() caller passes None (random id).
Adds a unit test that the derived id is deterministic and distinct.
revoke cancels the original tx (unlocking its inputs) before the replacement
self-spend is built, and the steps commit separately, so a crash in between left
the inputs Unspent with no self-spend while a retry silently did nothing.
- Find the contributed inputs by tx id whether they are still Locked (original tx
active) or already Unspent (a prior revoke cancelled them but the self-spend did
not finish), instead of Locked-only.
- Cancel only when the tx is still in a cancellable state, so a resumed or repeated
revoke skips the cancel and re-spends the Unspent inputs.
- Once the self-spend completes the inputs reference its tx id, so a later revoke
is a no-op.
Adds a regression test that interrupts a revoke after the cancel and confirms the
next revoke resumes and produces the self-spend.
Addresses PR #754 review (revoke.rs).
save_step wrote the tx file (store_tx) before the atomic batch commit, so a DB
failure could leave the stored tx out of sync with wallet state. Move store_tx
to after batch.commit() (and after the post-commit signing-context check),
matching the core convention in internal::selection. #755 keeps stored txs as
files for all flows, so this reorder closes the desync window without diverging
from core.
Addresses PR #754 review (utils.rs).
The serde deserialize helpers copied fixed-length byte arrays
(b.copy_from_slice(&bytes[0..N])) without checking the decoded length, so a
slatepack with a short pubkey/signature/uuid field panicked the wallet during
deserialization. The binary path was hardened in 25dab9f; do the same for the
JSON modules: reject any field whose decoded length is not exactly N. Adds a
regression test that exercised the panic.
De-panic the contract paths flagged in review plus several found alongside them:
- new: use unsigned_abs (abs() panics on i64::MIN).
- proofs: surface InvoiceProof serialization errors instead of expect(); bounds-
check the participant index before indexing participant_data.
- utils/slate: propagate the error when a context input is missing from the db
instead of unwrapping.
- revoke: return an error when a locked input has no cached commitment.
- selection: checked i64/u64 conversions so large amounts can't wrap into a
wrong (possibly negative) change.
- types: checked sum of output amounts.
Addresses PR #754 review (new.rs, proofs.rs, utils.rs, selection.rs).
OutputSelectionArgs.make_outputs was a comma-separated grin string parsed inside
libwallet via amount_from_hr_string. Move that parsing to the CLI boundary and
carry explicit nanogrin u64 amounts through the API and Context instead:
- OutputSelectionArgs.make_outputs: Option<String> -> Option<Vec<u64>>;
output_amounts() becomes a plain accessor, num_custom_outputs() uses len().
- The CLI (wallet_args) parses grin -> nanogrin per amount; ContractNewArgs and
ContractSetupArgs carry Vec<u64> and copy it through.
- Update the two tests that supplied make_outputs as a string.
Addresses PR #754 review (types.rs, wallet_args.rs).
Reconcile the contract feature with #755, which replaced the WalletBackend
trait with a concrete WalletBackend<C,K> struct (libwallet/src/backend.rs) and
removed impls/src/backends/lmdb.rs.
- Port the contract module and contract API entry points from the trait form
(T: WalletBackend<'a,C,K>) to the concrete struct.
- Re-home next_child_for and a parent_id+log_id get_tx_log_entry onto backend.rs.
- Re-apply the self-spend tx-log filters, cancel-loop, v5 payment-proof fields,
and Context contract fields onto staging's rewritten files.
- Adapt to fallible iter()/tx_log_iter() and self-consuming batch commit.
Full suite green (53 binaries / 191 tests).
* wallet: update lmdb from store crate, migrate single backend from trait to struct, update tests cargo deps and design doc
* backend: do not panic on iter and directory creation
* return iterator errors
* build: update to last node revision
* fix: height selection for scan, remove unused mutability for wallet
* wallet: remove unused mutability conversion
* tx: store slate state at database
* build: cargo fmt
* fix: do not return error when slate not found on tx slate state update
* lmdb: do not collect all tx log records at iterator, show read errors at log
* fix: save slate state, return error if slate not found
* tx: specify amount of bad txs on slate state update error
---------
Co-authored-by: Joerg <wiesche89@googlemail.com>
Comment-only follow-ups to the review:
- selection: a negative change output is already rejected; drop the stale TODO.
- utils: note the per-participant kernel fee is rounded up (overpay bounded to
under one unit per participant), and that add_outputs/parent_key_id are handled
by the sign step and the stored Context.
- types: drop the "custom outputs imply payjoin" TODO; make_outputs is valid
without a payjoin (e.g. a receiver splitting its output), so the check would be
incorrect.
Addresses PR #754 review (selection.rs, utils.rs, types.rs).
Comment/doc cleanups from review; the only behavior-adjacent change is the
get_net_change signature.
- get_net_change takes the slate id instead of the whole slate.
- replace the owner-API contract methods' `/// TODO` placeholders with real docs.
- narrow or remove now-resolved TODOs (slate proof fns, context, utils stored_tx,
a doc-comment that should be //, wallet_args parse notes); reword the rest as
concrete FUTURE notes.
- prune the contract actions README of items already fixed.
Addresses PR #754 review (comment hygiene).
revoke left the canceled slate's private context in place when the contract
had not been signed (signing already deletes it). Look up the revoked slate id
from its tx log entry and delete the context after cancelling, if one still
exists. Resolves the stale revoke TODOs about the cancelled context.
Addresses PR #754 review (revoke.rs).
contract view hardcoded agreed_net_change to None. When a local context exists
we've agreed on the net change, so read it from the context and report it
(None when there's no context); other errors propagate.
Addresses PR #754 review (view.rs).
The separate contract_setup step is superseded by contract_sign, which runs
setup internally. Remove the dead commented-out function (libwallet, controller,
api), its CLI subcommand (grin-wallet.yml), and its dispatch arm (wallet_args).
The contract_sign path and the parse_contract_setup_args parser are unaffected.
Addresses PR #754 review (command.rs, owner.rs).
ContractNewArgsAPI and ContractView carried a target_slate_version that was only
ever None and never read; the emitted slatepack version is chosen from the slate
content. Remove it. The legacy InitTxArgs/IssueInvoiceTxArgs fields are unrelated
and untouched.
Addresses PR #754 review (types.rs).
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).
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.
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.
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.
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>
* add all comsig code, update to master branch
* refactor code into libwallet, remove some redundancy and update type exports
* added api calls and unit test for comsig request creation
* Addition of JSON-RPC mwixnet function call
* attempt to debug invalid params
* additions to support json doctesting for comsig creation
* tweaks to testing and documentation updates
* dependencies for tests
* update srs test
* updating comsig creation, attempt to get working
* implementation of comsig given an output commit
* lock output on mwixnet creation, add further testing
* Simple contracts restructured v3 (#675)
* Add prototype contract implementation
Lacks:
- Payment proofs (add early payment proofs)
- Tests
- Better structure
* Separate contract utilities
* Simplify the main setup/sign code flow
* Remove commented initial_sec_key assignment
* Simplify sign::compute function
* Add logic for "removal" of secret keys from the context
* Further simplify a bit sign and setup
* No need for mutable context when adding outputs
* Refactor the commented code (lol)
* Refactor a bit
* tmp
---------
Co-authored-by: oryhp <gtrphyro@gmail.com>
* contract test fixes (#676)
* [Contracts] Experimental Slate v5 (#677)
* beginning to add and modify slate version 5
* completion of conversions from V5 to V4 Slate
* timestamp and memo fields
* upgrade/downgrade serialization of v5 slates
* add v5 binary slate versions + start of tests
* add bin slate ser/deser to tests
* ensure serialization of timestamp always excludes milliseconds
* start to update v5 tests, update v5 documentation
* add fn to generate populated internal slate for conversion testing
* add basic tests to convert all slate versions
* update
* commit cargo.lock
* [WIP] [Contracts] Early payment proofs (#681)
* add types and beginnings of signature utils
* add proof serialization
* serialisation of proof data + signature operation
* add serialization type for invoice proof + separate bin wrapper version
* add witness data + serializion to invoice payment proof, insert verfication functions in place in order to begin verification testing
* tests and infrastructure in place for validation
* verification of promise sig
* added verification of promise signature, infrastructure up to the point where a signature must be subtracted
* attempting to figure out differences between recipient nonce that's getting stored and calculated recipient nonce
* implementation of witness verification function, retrieve relevant values and re-validate derived recipient partial signature
* move stored portion of invoice proof into core types for storage, need to rename invoice proof
* define/refine the stored portion of payment proofs type 2?
* Folding all proof data into tx log entry storage
* back to importing master
* remove cargo files from diffs
* remove a lot of extra debug output
* return proof witness as part of proof retrieval, define json serialization of invoice proof + witness fields
* finish adding verification steps to foreign API
* remove redundant promise sig field
* move lcation of sign/verify calls
* Replace Azure Pipelines with Github Actions (#688)
* Update CI Badge on README.MD (#690)
* Trigger CI on push and pull request (#693)
* Update versioning to 5.2.0-beta.1 against grin 5.2.0-beta.3 (#691)
* update versioning to 5.2.0-beta.1 against grin 5.2.0-beta.3
* tweak for CI trigger
---------
Co-authored-by: Quentin Le Sceller <q.lesceller@gmail.com>
---------
Co-authored-by: Quentin Le Sceller <q.lesceller@gmail.com>
* fix Cargo.lock
* Contracts - Test fixes + warning cleanups (#694)
* clean up warnings in libwallet crate
* clean up warnings in controller crate
* update all contract tests with awareness of new proof structure
* added explicit payment proof tests for both rsr and srs (#695)
* Basic mwmixnet send (#696)
* integrating onion library
* updates and changes to support newly included mwmixnet types
* add (incorrect) owner api function
* turn off test for now
* switch working grin branch to master
* fix doctests for build
* update cargo lock in attempt to fix croaring build on CI server
* update cargo lock with upstream thiserror crate
* update test dependency for croaring
* [Contracts] Add self spend transaction state (#697)
* Add self spend transaction state
* subtle errors with output states and tx lookups - fixes
* [Contracts] Slatepack v5 Deserialization fix (#698)
* add V5 deserialization test + fixes
* clarify comment
* upwrap fix during v4 deserialization
* further unwrap removal
* update cargo lock, use latest grin packages
* addition of self-spend cancel + tests (#699)
* add tests + legacy self send cancel (#702)
* [Contracts] Cancel self-spend TX Pt. 2 (#703)
* add tests + legacy self send cancel
* add missing file
* Update grin to latest master, change versioning label to include contracts (#712)
* Add owner_api_listen_interface as hidden configuration field (#714)
* updates for rust 1.80
* updates for rust 1.80
---------
Co-authored-by: oryhp <gtrphyro@gmail.com>
Co-authored-by: Quentin Le Sceller <q.lesceller@gmail.com>
jwinterm