Skip to content

H2: Replace (List<Coin>, List<Key>) tuple with SigningCoin record#913

Open
DavidGershony wants to merge 1 commit into
mainfrom
security/h2-signing-coin-record
Open

H2: Replace (List<Coin>, List<Key>) tuple with SigningCoin record#913
DavidGershony wants to merge 1 commit into
mainfrom
security/h2-signing-coin-record

Conversation

@DavidGershony

Copy link
Copy Markdown
Collaborator

Summary

Replace the unsafe (List<Coin>?, List<Key>) tuple return type with a strongly-typed SigningCoin record that pairs each coin with its signing key, eliminating the possibility of index-based mismatches.

Problem

GetUnspentOutputsForTransaction returned two parallel lists — coins and keys — indexed in lockstep. If the lists ever got out of sync (e.g., by a future refactor that filters one but not the other), the wrong key would sign the wrong input, producing an invalid transaction with no compile-time protection.

Solution

csharp public record SigningCoin(Coin Coin, Key Key);

  • Each coin is permanently paired with its derived key at construction time
  • Signing loops iterate a single list, accessing sc.Coin and sc.Key
  • Error messages now include the outpoint instead of an opaque index
  • All test mocks updated to return List<SigningCoin>

Files Changed

File Change
IWalletOperations.cs Add SigningCoin record; change return type
WalletOperations.cs Refactor all signing loops to use SigningCoin
WalletOperationsTest.cs Update mock setup
FounderTransactionActionTest.cs Update mock setup
InvestmentIntegrationsTests.cs Update mock setup
InvestmentOperations.cs (DataBuilder) Update mock setup
InvestmentOperationsTest.cs Update mock setup

Testing

All 154 shared tests pass.

Introduce a strongly-typed SigningCoin record that pairs each Coin with
its corresponding signing Key, eliminating the unsafe parallel-list
pattern where coins and keys could become mismatched by index.

- Add SigningCoin record to IWalletOperations
- Change GetUnspentOutputsForTransaction return type to List<SigningCoin>
- Refactor all callers in WalletOperations to use sc.Coin / sc.Key
- Remove index-based iteration in favor of direct sc.Key access
- Add outpoint to error messages (instead of opaque index)
- Update all test mocks to return List<SigningCoin>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant