check wallet id when receiving a transaction from the network#1743
check wallet id when receiving a transaction from the network#1743HayimShaul wants to merge 51 commits into
Conversation
HayimShaul
force-pushed
the
1631_semantic_security
branch
from
66719d8 to
686ab72
Compare
HayimShaul
force-pushed
the
1631_semantic_security
branch
from
75916c9 to
098c54c
Compare
The wallet validation added in PR #1743 is causing HTLC locked tokens to not be tracked correctly in both fabtoken and dlog implementations. The validation logic itself is correct (using EnrollmentID() instead of GetRecipientIdentity() to avoid side effects), but there appears to be a deeper issue with how the validation interacts with HTLC token storage. Temporarily disabling the validation until the root cause can be identified. The validation can be re-enabled by uncommenting the function call. Diagnostic logging has been added to help identify the issue in future debugging. Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
| // Notice that, this step would not be required if the issuer knew already which | ||
| // identity the recipient wants to use. | ||
| recipient, err := ttx.RequestRecipientIdentity(context, p.Recipient, ServiceOpts(p.TMSID, ttx.WithRecipientWalletID(p.RecipientWalletID))...) | ||
| assert.NoError(err, "failed getting recipient identity") |
There was a problem hiding this comment.
Why you replaced the assert with return?
|
|
||
| walletIDs := []string{provider.BaseID()} | ||
| if sender { | ||
| walletID = senderWallet(ctx, w.wallet) |
There was a problem hiding this comment.
why we replaced senderWallet with append?
There was a problem hiding this comment.
The original code was returning immediately after finding the first valid iterator (typically for the base wallet ID like 'bob'), without querying the second wallet ID (like 'htlc.recipientbob') where HTLC locked tokens are actually stored.
The Problem
In HTLC (Hash Time Locked Contract) scenarios, tokens can be stored under multiple wallet IDs:
Base wallet ID (e.g., "bob") - the user's main wallet
HTLC-specific wallet ID (e.g., "htlc.recipientbob" or "htlc.senderbob") - where locked tokens are stored
The old code would query only the first wallet ID and return, missing all tokens in the HTLC-specific wallet.
The Solution
The PR (commit d6c7fd8) changed the logic to:
Build an array of wallet IDs to query:
Always includes the base wallet ID
Adds sender-specific or recipient-specific HTLC wallet ID based on the sender flag
Query ALL wallet IDs and collect valid iterators from each
Chain iterators together using a new chainedIterator type that sequentially iterates through all of them
Graceful fallback: If one wallet ID fails (e.g., doesn't exist yet), it continues with others instead of failing completely
This ensures that HTLC queries return tokens from both the base wallet AND the HTLC-specific wallet, fixing the broken HTLC functionality in both fabtoken and dlog implementations.
The test TestOwnerWalletFilterIteratorFallsBackToBaseWalletID demonstrates this fallback behavior where the HTLC wallet ID fails but tokens are still found in the base wallet.
| walletIDs = append(walletIDs, provider.SenderID(ctx)) | ||
| } else { | ||
| walletID = recipientWallet(ctx, w.wallet) | ||
| walletIDs = append(walletIDs, provider.RecipientID(ctx)) |
There was a problem hiding this comment.
why we replaced recipientWallet with append?
There was a problem hiding this comment.
same answer as above
| if len(txEIDs) == 0 { | ||
| logger.DebugfContext(ctx, "no enrollment IDs to validate in transaction") | ||
|
|
||
| return nil |
There was a problem hiding this comment.
this function validates all enrollment ids. nil means there's no error. If there are no enrollment ids to validate no need to check anything
| } else { | ||
| // EID is not in our local set - could be: | ||
| // 1. A remote party's EID (valid - we don't validate remote parties) | ||
| // 2. A malformed local EID (invalid - but we can't distinguish without more context) |
There was a problem hiding this comment.
Ok, so if it is invalid what is the action item that we need to do? only to have a message in the log file?
There was a problem hiding this comment.
We can't distinguish between a remote wallet and an invalid local wallet.
The issue this PR addresses requested to validate that all wallets in database are valid.
Since you can't distinguish between a malformed local and a remote we can only log what we found
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
The wallet validation added in PR #1743 is causing HTLC locked tokens to not be tracked correctly in both fabtoken and dlog implementations. The validation logic itself is correct (using EnrollmentID() instead of GetRecipientIdentity() to avoid side effects), but there appears to be a deeper issue with how the validation interacts with HTLC token storage. Temporarily disabling the validation until the root cause can be identified. The validation can be re-enabled by uncommenting the function call. Diagnostic logging has been added to help identify the issue in future debugging. Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
- Enable debug log level for fabtoken tests (was info) - Add verbose test progress logging to track execution flow - Add detailed logging to RegisterAuditor, IssueCash, CheckBalanceWithLocked - Add logging to Restart and HTLCLock functions - Add step-by-step logging in TestHTLCSingleNetwork This will help diagnose the timeout issue in interop-fabtoken-t1 tests. All logging uses [TEST] prefix for easy filtering. Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Root cause: The filterIterator function was returning after finding the first
valid iterator (for base wallet ID 'bob'), without querying the second wallet
ID ('htlc.recipientbob') where HTLC locked tokens are actually stored.
This caused all HTLC locked token queries to return 0 tokens, breaking HTLC
functionality in both fabtoken and dlog implementations.
Solution: Collect iterators from ALL wallet IDs and chain them together using
a new chainedIterator type that sequentially iterates through all of them.
The bug was NOT related to the wallet validation changes - it was a pre-existing
issue in the HTLC wallet filter logic that only became apparent when tests
checked for locked tokens after node restart.
Fixes: interop-dlog-t1, interop-fabtoken-t1, and related HTLC tests
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Now that the HTLC iterator bug is fixed, we can safely re-enable the wallet validation that was temporarily disabled during debugging. Also removed excessive debug logging that was added during diagnosis: - Removed [TEST] prefix logging from integration tests - Removed verbose HTLC filterIterator debug logs - Kept only essential logging for multi-iterator chaining Changes: - token/services/ttx/transaction.go: Re-enabled validationFunc() call - token/services/interop/htlc/wallet.go: Cleaned up debug logging - integration/token/interop/tests.go: Removed verbose test logging - integration/token/interop/support.go: Removed verbose helper logging Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
HayimShaul
force-pushed
the
1631_semantic_security
branch
from
b07bed1 to
dfb4f9f
Compare
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
3 participants
check wallet id when receiving a transaction from the network
Addresses one check mentioned in #1631