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

Add some validity checks for offer messages and oracle announcements #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tibo-lg
Copy link
Member

@Tibo-lg Tibo-lg commented Apr 22, 2022

Title says it all

Protocol.md Outdated
@@ -130,7 +132,9 @@ The sending node MUST:
- set `cet_locktime` to be less than `refund_locktime`.
- use a unique `input_serial_id` for each input
- set `change_serial_id` and `fund_output_serial_id` to different values
- use valid [contract descriptor(s)](./Messaging.md#The-contract_descriptor-Type) within `contract_info`.
- use valid [contract descriptor(s)](./Messaging.md#The-contract_descriptor-Type) within `contract_info`
- set `cet_locktime` to the earliest `event_maturity_epoch` amongst all `oracle_event` used included in the `contract_info`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this strictly necessary? I feel like any time before the earliest oracle announcement is fine (including current block height) so as to avoid any UX issues? (I agree that refund_locktime should be about a week after latest contract execution option)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point thanks! Updated to "less than or equal"

Protocol.md Outdated
@@ -166,6 +172,9 @@ The receiving node MUST reject the contract if:
- The `fund_output_serial_id` and `change_serial_id` are not set to different value
- Any input in `funding_inputs` is not a BIP141 (Segregated Witness) input.
- invalid [contract descriptor(s)](./Messaging.md#The-contract_descriptor-Type) are used within `contract_info`.
- `cet_locktime` is not set to the earliest maturity time of all included oracle events.
Copy link
Contributor

Choose a reason for hiding this comment

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

as I said above, I feel like cet_locktime doesn't need to be so strict (and refund locktime should be far enough away from the latest possible execution)

@Tibo-lg Tibo-lg force-pushed the more-validity-checks branch from f072b0e to bcb97ba Compare April 24, 2022 11:29
@@ -139,6 +143,7 @@ The sending node SHOULD:
- set `refund_locktime` sufficiently long after the latest possible release of oracle signatures added to all other delays to closing the contract.
- set `payout_spk` to a previously unused script public key.
- set `change_spk` to a previously unused script public key.
- set `refund_locktime` to a value not too big that both party can be expected to be refunded in a reasonable time-frame (recommended not more than latest contract maturity + 86400 * 14 meaning 2 weeks after latest contract maturity).
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it'd be good to have a recommendation for minimum as well (i.e. recommended no less than 2 days after the latest contract maturity) to give the oracle time to attest in case there are any infrastructure issues

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that cover here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep you're right, looks good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants