Skip to content

FP: Double withdrawal#290

Open
elRaulito wants to merge 1 commit into
mainfrom
fp-double-withdraw
Open

FP: Double withdrawal#290
elRaulito wants to merge 1 commit into
mainfrom
fp-double-withdraw

Conversation

@elRaulito

Copy link
Copy Markdown
Collaborator

No description provided.

@keyan-m keyan-m 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.

Not a full review, but just pointing out one thing I spotted :)

Comment on lines +175 to +181
pub fn get_withdrawals_root(
tx: Transaction,
state_queue_node_ref_input_index: Int,
) -> ByteArray {
// Reference input with block_hash
expect Some(state_queue_node_input) =
list.at(tx.reference_inputs, state_queue_node_ref_input_index)

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.

Validation on the NFT seems missing. We do have a helper:

/// For beacon UTxOs with specific policy IDs. Note that the address is not
/// checked.
pub fn get_authentic_input_with_policy_at(
inputs: List<Input>,
nft_policy_id: PolicyId,
input_index: Int,
) -> Input {
expect Some(
Input { output: Output { value: input_value, .. }, .. } as found_input,
) = list.at(inputs, input_index)
expect (input_nft_policy_id, _, 1) =
get_single_asset_from_value_apart_from_ada(input_value)
expect input_nft_policy_id == nft_policy_id
found_input
}

Suggested change
pub fn get_withdrawals_root(
tx: Transaction,
state_queue_node_ref_input_index: Int,
) -> ByteArray {
// Reference input with block_hash
expect Some(state_queue_node_input) =
list.at(tx.reference_inputs, state_queue_node_ref_input_index)
pub fn get_withdrawals_root(
reference_inputs: List<Input>,
state_queue_policy: PolicyId,
state_queue_node_ref_input_index: Int,
) -> ByteArray {
// Reference input with block_hash
let state_queue_node_input =
utils.get_authentic_input_with_policy_at(
reference_inputs,
state_queue_policy,
state_queue_node_ref_input_index,
)

It might make sense to also define a get_datum in /lib/midgard/state-queue.ak (similar to the one we have in hub-oracle.ak) and use it here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hey @keyan-m

isn't this code doing the nft beacon verification?

let ct_token_asset_name = get_singleton_ct_token(own_input, ct_token_policy_id)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh nevermind I see

I get the asset name but I never use it

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.

That line seems fine and is validating authenticity of the input (own_input). For the withdrawals root, we're reading the datum off of a reference input, so its state token must also be validated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Isn't that missing also here then?

midgard\fraud-proof\common\utils.ak

pub fn get_tx_root_validate_block_hash(
  tx: Transaction,
  state_queue_node_ref_input_index: Int,
  ct_token_asset_name: ByteArray,
) -> ByteArray {
  // Reference input with block_hash 
  expect Some(state_queue_node_input) =
    list.at(tx.reference_inputs, state_queue_node_ref_input_index)

  // Extract tx_root via the state queue block hash
  expect InlineDatum(state_queue_datum_data) =
    state_queue_node_input.output.datum
  expect state_queue_node_datum: unordered.NodeDatum = state_queue_datum_data
  expect parsed_state_queue_datum: Header = state_queue_node_datum.data
  expect unordered.Key { key: block_hash } = state_queue_node_datum.key
  let extracted_tx_root = parsed_state_queue_datum.transactions_root

  // Get CT token and check match with the block hash
  expect drop(ct_token_asset_name, n: 4) == block_hash

  extracted_tx_root
}

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.

It seems to be, yeah. I would suggest defining get_confirmed_datum and get_block_datum helpers under /lib/midgard/state-queue.ak so that they can be reused for such validations.

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.

2 participants