add-carbon-offset-to-claim-and-tests

[sBaki92]

On Ainur's advice, I modified the claim function and split it into two parts: the first is confirm_for_merkle_tree, which confirms the offset on the Merkle tree side by validating the proof provided by the user; the second is confirm_offset, which handles business logic checks (e.g., ensuring the claim hasn't already been made, verifying that there is sufficient pending retirement, etc.), calls confirm_for_merkle_tree for Merkle tree checks, and performs the offset action with _offset_carbon_credit.

Consequently, the tests for the claim function in test_merkle_tree.cairo have been converted to tests for the confirm_for_merkle_tree function.

I added tests for the confirm_claim function in test_offseter.cairo.

I also added two getters, get_allocation_id and get_retirement, in the offsetter code as they were needed for my tests.

Remarks:

I considered checking the vintage status in the confirm_offset function, but since this is already done in retire_carbon_credits, I concluded it was unnecessary.

Since the claim_for_merkle_tree function is internal to confirm_offset, we needed to determine how to call it. It is not callable by the user (or anyone other than the contract itself). Therefore, I added a from parameter representing the claimer's address to avoid issues related to calling the function, ensuring it is only invoked by the contract.

We realized when attempting to call _offset_carbon_credit that there is no mapping from allocation_id to allocation. This will need to be addressed in a future PR: adding the token_id field to the allocation struct (which will involve modifying aspects of the Merkle tree, especially the intermediate_hash in the confirm_for_merkle_tree method).

For now, in this PR, we assume that only vintages with token_id = 1 are offset, which is why we set self._offset_carbon_credit(claimee, 1, amount.into()) in confirm_offset.

-> In the confirm_offset tests, we perform the deposit and the entire process with token_id set to 1.

In confirm_offset, in the [Mark as claimed] section, would it be better to use a getter for the allocation instead of recreating one (with the exact same fields) each time?

[tekkac]

look good to me, minor changes requested.

[tekkac]

In confirm_offset, in the [Mark as claimed] section, would it be better to use a getter for the allocation instead of recreating one (with the exact same fields) each time?

Yes, and probably you just need to add a claimed: bool field in Allocation

[sBaki92]

In confirm_offset, in the [Mark as claimed] section, would it be better to use a getter for the allocation instead of recreating one (with the exact same fields) each time?

Yes, and probably you just need to add a claimed: bool field in Allocation

Could we add this field in the next PR? Because, similar to the allocation_id --> allocation mapping issue, adding a field to the allocation struct will require changes to the Merkle tree and the intermediate hashes in the confirm_for_merkle_tree method.

[tekkac]

lgtm

[julienbrs]

code: @all-contributors please add @sBaki92 for his first great test PR 🔥

[allcontributors]

@julienbrs

I've put up a pull request to add @sBaki92! 🎉