S-157 Allow dynamic image hash for wallet as sapient signer #87
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.
This was raised during the C4 audit.
S-157
BaseAuth.recoverSapientSignaturereturns a constant instead of signer image hash, breaking sapient signer flowsIssue: Wallets always return
bytes32(1)as theirimageHashwhen responding torecoverSapientSignature. This goes against theISapientinterface expectation.Analysis: The decision to return
bytes32(1)as the wallet image hash was intentional. Otherwise, a parent wallet using a child wallet will have trouble using them since we expect the image hash changes. However, it may be desirable for a parent to only support a child wallet with a fixed configuration.Fix: The adds a new signature flag byte is prepended when using
recoverSapientSignature. This has a bool on whether to return the recovered image hash orbytes32(1), allowing a parent to support both changing and static image hashes. When the flag is not supplied (i.e. we are returningbytes32(1)), we ban the use of chained signatures to prevent unpersisted image hash updates returning the currently accepted image hash.Caveat: When using a static signature, the image hash cannot always be recovered. In this case we return
bytes32(bytes1(0x80))(the static signature flag) as the image hash.Enhancement: Update the ISapient spec to define
bytes(1)(or similar) as a constant that represents "any image hash", so that other ISapient integrators can follow the same pattern.Alternative: Remove the new sapient signature flag byte, always return the derived image hash. A parent wallet should use the child wallet as a signer under the 1271 interface when supporting dynamic image hashes. The caveat with this approach is that we lose the ability for the child to validate the payload contents without the ISapient interface.