Repository navigation
Emit investment_made event and mint fraction tokens on invest - #677
Open
GoldenTuxbee wants to merge 1 commit into
Open
GoldenTuxbee wants to merge 1 commit into
GoldenTuxbee wants to merge 1 commit into
Conversation
Contributor
|
Closes StellarState#518 Closes StellarState#517 Both issues ask for functionality that was already substantially implemented under different names: invoice-escrow's invest/finalise_funding (StellarState#518's release_to_issuer)/refund (StellarState#518's refund_investors)/get_invoice (StellarState#518's get_escrow_balance) cover StellarState#518's asks, and invoice-token's mint/burn/transfer/balance/events already cover StellarState#517's asks as a standalone SEP-41-compliant contract (one deployed instance per invoice, so invoice_id is set once at initialize rather than passed per call). Two real gaps found and fixed: invest() on the multi-investor FundingInvoice flow (created via create_invoice, distinct from the legacy register_invoice/EscrowStatus path) emitted nothing at all. Added events::investment_made, published with the investor, invoice id, amount, and the investor's new total position; wired into invest(). Covered by test_invest_on_funding_invoice_emits_investment_made_event. them any fraction tokens representing their ownership share, because the two contracts were never wired together (invoice-token was only a dev-dependency of invoice-escrow, used in tests, not a real cross-contract call). The legacy create_escrow_with_tokens path already does this exact mint for its own flow ("Mint invoice tokens to the buyer to represent their ownership share"); invest() on FundingInvoice just never got the same treatment. Added an optional fraction_token field to FundingInvoice (Option<Address>, additive so it doesn't disturb the existing field layout) set via a new trailing create_invoice parameter; when set, invest() calls the configured invoice-token contract's mint via env.invoke_contract, same pattern as the legacy path. When fraction_token is None, behavior is unchanged (payment-only, no fraction tokens) for invoices that don't need one. Covered by test_invest_mints_fraction_tokens_when_fraction_token_configured, which deploys a real invoice-token instance, configures escrow as its minter, and asserts the investor's fraction balance after two invests. Scoped smaller than the full "wire every escrow lifecycle state to the token contract" version (burn on refund/partial_refund/finalise_funding is not wired here) per discussion: the minimal, lowest-risk integration is investor receives fraction tokens on invest, which is the literal ask in proportional to investment"); burn-on-settlement is a reasonable follow-up but adds more surface (which lifecycle transitions trigger it, whether partial refunds burn partially) that deserves its own review. Validation: cargo build -p invoice-escrow -p invoice-token builds clean (only pre-existing deprecation warnings). The crate's own test suite (contracts/invoice-escrow/src/test.rs) fails to compile with 152 pre-existing errors on a clean upstream/dev checkout (confirmed via git stash before/after) due to a soroban-sdk 27.0.2 -> 27.0.3 minor-version API drift (Storage::to_xdr removed, StorageKey visibility changed, etc.), unrelated to this change and well outside its scope to fix. Verified my two new tests compile cleanly in isolation (cargo check -p invoice-escrow --tests, confirmed zero errors in their line range) since the broken pre-existing tests in the same file prevent actually running the suite.
GoldenTuxbee
force-pushed
the
fix/wave-batch-517-518
branch
from
October 31, 2026 23:40
9a96791 to
afeb4cc
Compare
Contributor
✅ PR Ready for ReviewDue to a recent malware incident affecting our repositories, all PRs are required to pass automated security checks before being reviewed for merging. This PR has passed all automated checks:
A maintainer will review and merge this PR shortly. |
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
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.
Closes #518
Closes #517
Both issues ask for functionality that was already substantially implemented in this repo under different names: invoice-escrow's
invest/finalise_funding(#518'srelease_to_issuer)/refund(#518'srefund_investors)/get_invoice(#518'sget_escrow_balance) cover #518's asks, and invoice-token'smint/burn/transfer/balance/events already cover #517's asks as a standalone SEP-41-compliant contract (one deployed instance per invoice, soinvoice_idis set once atinitializerather than passed per call). Two real gaps found and fixed:#518: missing event on the FundingInvoice invest path
Every other state-changing call in
invoice-escrowemits an event, butinvest()on the multi-investorFundingInvoiceflow (created viacreate_invoice, distinct from the legacyregister_invoice/EscrowStatuspath) emitted nothing at all. Addedevents::investment_made, published with the investor, invoice id, amount, and the investor's new total position; wired intoinvest(). Covered bytest_invest_on_funding_invoice_emits_investment_made_event.#517: invest() never minted fraction tokens
invest()locked the investor's payment into escrow but never minted them any fraction tokens representing their ownership share, because the two contracts were never wired together (invoice-tokenwas only a dev-dependency ofinvoice-escrow, used in tests, not a real cross-contract call).The legacy
create_escrow_with_tokenspath already does this exact mint for its own flow (comment: "Mint invoice tokens to the buyer to represent their ownership share"),invest()onFundingInvoicejust never got the same treatment.Added an optional
fraction_tokenfield toFundingInvoice(Option<Address>, additive so it doesn't disturb the existing field layout) set via a new trailingcreate_invoiceparameter; when set,invest()calls the configured invoice-token contract'smintviaenv.invoke_contract, same pattern as the legacy path. Whenfraction_tokenisNone, behavior is unchanged (payment-only, no fraction tokens) for invoices that don't need one. Covered bytest_invest_mints_fraction_tokens_when_fraction_token_configured, which deploys a realinvoice-tokeninstance, configures escrow as its minter, and asserts the investor's fraction balance after two invests.Scoped smaller than the full "wire every escrow lifecycle state to the token contract" version (burn on refund/partial_refund/finalise_funding is not wired here): the minimal, lowest-risk integration is investor receives fraction tokens on invest, which is the literal ask in #517's acceptance criteria ("Mint issues correct fraction amount proportional to investment"). Burn-on-settlement is a reasonable follow-up but adds more surface (which lifecycle transitions trigger it, whether partial refunds burn partially) that deserves its own review rather than being bundled in here.
Validation
cargo build -p invoice-escrow -p invoice-tokenbuilds clean (only pre-existing deprecation warnings).The crate's own test suite (
contracts/invoice-escrow/src/test.rs) fails to compile with 152 pre-existing errors on a cleanupstream/devcheckout (confirmed viagit stashbefore/after) due to asoroban-sdk27.0.2 to 27.0.3 minor-version API drift (Storage::to_xdrremoved,StorageKeyvisibility changed, etc.), unrelated to this change and well outside its scope to fix.Verified my two new tests compile cleanly in isolation (
cargo check -p invoice-escrow --tests, confirmed zero errors in their line range) since the broken pre-existing tests in the same file prevent actually running the suite.