Skip to content

Fix inconsistency in SDK - #186

Merged
Arvolear merged 5 commits into
devfrom
fix/issues
Oct 23, 2026
Merged

Arvolear merged 5 commits into
devfrom
fix/issues

Conversation

@ikripaka

@ikripaka ikripaka commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator
  • This PR suggests a bug fix and I've added the necessary tests.
  • This PR introduces a new feature and I've discussed the update in an Issue or with the team.
  • This PR is just a minor change like a typo fix.

This PR resolves some inconsistencies in the SDK.
ddcf727 - resolves an unblinding issue when TxOutSecret is missing. Added additional error ot handle this. On UTXO discovery, they're ignored, and balance can be returned, but without unblinded UTXOs.
6584157 - Fixed inconsistent final sequence injection. After the update, we're inserting ENABLE_LOCKTIME_NO_RBF when locktime is non-zero and non-final, or zero. In the previous case, we could create a transaction with locktime and forget to insert the sequence.
d0ed063 - Fixed issue with a possible tricky hash collision on local imports build. External paths in folders outside the project are banned. Explicit Imports using a file system path to a library must be in the same folder to reduce faulty dependency injection. Tags and branches are now cloned explicitly (we can easily clone a branch instead of a tag when they have the same name).

Commits can be easily cherry-picked.

PS
Regarding other issues:

  • The issue with RequiredSignature::None should be mulled over beforehand.
  • Fee upper cap, to my mind, seems to be overkill for us- the user can check a transaction before broadcasting it.
  • Exposing of electrsd cookies is ok for us- that's how our regtest works.

@ikripaka
ikripaka force-pushed the fix/issues branch 6 times, most recently from ff183af to 001f229 Compare October 21, 2026 19:18
@ikripaka
ikripaka marked this pull request as ready for review October 22, 2026 07:13
@ikripaka
ikripaka requested a review from Arvolear as a code owner October 22, 2026 07:13

@LesterEvSe LesterEvSe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also needs rebase

Comment thread crates/cli/src/commands/install.rs Outdated
Comment thread crates/build/src/resolver.rs Outdated
* add errors instead of panic
* add additional tests to catch this behaviour

@LesterEvSe LesterEvSe left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK d0ed063 with the fix in 3c5d86e. Tested the fixtures with simplex test and ran the unit tests in crates/cli/src/commands/install.rs. Other commits were not reviewed

@ikripaka

Copy link
Copy Markdown
Collaborator Author

Rebased onto current dev dc7b26d.

fix:
* hash collisions on deps installation
* overlapping cloning behaviour (tag and branch)
* dependency escaping issue
Comment thread crates/sdk/src/signer/core.rs
@ikripaka

Copy link
Copy Markdown
Collaborator Author

Dropped 6584157 and applied lints in 1b0829e.

@Arvolear
Arvolear merged commit a0a994c into dev Oct 23, 2026
9 checks passed
@Arvolear
Arvolear deleted the fix/issues branch October 23, 2026 15:26
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.

3 participants