Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contract audit fixes #145

Closed
wants to merge 7 commits into from
Closed

contract audit fixes #145

wants to merge 7 commits into from

Conversation

tgrecojs
Copy link
Contributor

@tgrecojs tgrecojs commented Dec 8, 2024

Refs: #141, #142

@tgrecojs tgrecojs added the enhancement New feature or request label Dec 8, 2024
@tgrecojs tgrecojs self-assigned this Dec 8, 2024
@tgrecojs tgrecojs requested a review from Jovonni December 9, 2024 18:24
Comment on lines 26 to 36
const ProofDataShape = harden({
hash: M.string(),
direction: M.string(),
});

const OfferArgsShape = harden({
tier: M.number(),
address: M.string(),
key: M.string(),
proof: M.arrayOf(ProofDataShape),
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good. Maybe we can also specify max length as (perhaps limits parameter) for string and arrayOf. WDYT?

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that sounds great! Great suggestion @amessbee.

Quick note: #141 is going to eliminate the address argument as the work done for that issue computes the address from the pubkey rather than relying on the user to supply the address.

This comment was marked as outdated.

});

const OfferArgsShape = harden({
tier: M.number(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

tier seems to be defined as bigInt in the contract and not as a number - not sure if that matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ATPIT the tier argument is working correctly within the contract. That said, the tier value is going to be concatenated onto the end of the pubkey hex string rather than being its own argument.

…ng handled correctly when processed through updated offerHandler code
…itedness as generic interface for creating AsyncGenerator subscribtions (#150)

This commit contains test for:
- Handling bobs offer
- Subscribing to bobs wallet
- Verifying Alice's inability to make a second claim attempt
@tgrecojs tgrecojs changed the title feat: added code for enforcing correct offerArgs shape contract audit fixes Dec 17, 2024
@tgrecojs
Copy link
Contributor Author

tgrecojs commented Jan 7, 2025

closing this PR as these fixes were merged in PR #149.

@tgrecojs tgrecojs closed this Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants