✨ New Staking Contract #453

BertrandD · 2024-12-13T15:34:18Z

No description provided.

codecov · 2024-12-13T16:04:30Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
158	1	157	0

View the top 1 failed tests by shortest run time

hyle::consensus_tests e2e_consensus::can_rejoin

Stack Traces | 32.4s run time

Error: deadline has elapsed

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

src/consensus.rs

maxgttph · 2024-12-16T09:54:40Z

src/tools/transactions_builder.rs

+    hydentity_cf: Vec<(IdentityAction, Password, BlobIndex)>,
+    hyllar_cf: Vec<(ERC20Action, ContractName, BlobIndex)>,
+    staking_cf: Vec<(StakingAction, BlobIndex)>,


c'est pour quoi les _cf ? je vois pas 😄

c'est un résidu de hyrun où j'avais appelé ces XXXActions des "call_functions" abbrégés en "cf"

https://github.com/Hyle-org/hyle/blob/main/crates/hyrun/src/lib.rs#L183

src/tools/transactions_builder.rs

maxgttph · 2024-12-16T10:00:37Z

src/tools/transactions_builder.rs

+            let mut contract = StakingContract::new(
+                ExecutionContext {
+                    callees_blobs: vec![].into(),
+                    caller: self.identity.clone(),


OPT: je pense que c'est pas nécessaire de mettre un caller non?

je comprends pas, tu voudrais que je mette quoi ?

Rien !
Par défaut si on met rien on vérifie que pour l'identité id.<nom_de_contract_d'identité> il y a un blob pour le contract <nom_de_contract_d'identité>

Ici, c'est une "copie" de ce qui ce passe dans le guest. C'est un petit tricks pour réussir à générer le même state que si on passait par le prover. Je passe pas par les blobs, j'execute le code du contrat en "natif", il n'y a pas de verification d'identité là, je cherche juste à re-générer le même state, car on-chain on a que le hash. J'ai besoin de build le vrai "next_state" pour pouvoir itérer sur mon builder....

FYI: ExecutionContext est défini comme ça:

pub struct ExecutionContext { pub callees_blobs: RefCell<Vec<Blob>>, pub caller: Identity, }

j'ai pas le choix de passer un caller, et c'est tout à fait normal :)

pas certain de la viabilité du truc, mais on peut pas passer par les indexers ?

mais on peut pas passer par les indexers ?

Non, ici on est pré-genesis quand on s'en sert, donc les indexers ne nous sont d'aucune utilité

le builder il sert à construire une transaction pouvant contenir autant de blobs qu'on veut dedans (pour le même contract_name), il permet de build les blobs+proofs de manière cohérente en connaissant chaque state intermédiaire au sein de la blobTx.

l'indexer ne peut pas aider pour ça

En soit la logique de "process le state d'une TX pour obtenir le state intermédiaire" c'est la logique des indexer, ce que je me demandais c'est s'il y avait pas un peu d'overlap fonctionnel entre les deux. Mais ça mérite clairement plus de réflexion.

oui clairement il y a un overlap entre le TransactionBuilder et le ContractStateIndexer oui, j'aimerais réussir à rassembler les deux un jour

src/tools/transactions_builder.rs

wraitii · 2024-12-20T09:02:02Z

contracts/staking/src/state.rs

+        })
+    }
+
+    pub fn stake(&mut self, staker: Identity, amount: u128) -> Result<String, String> {


les Result<String, String> dans le code rust de la node c'est pas idéal, idéalement on trouverait une solution plus élégante

ici on est dans le code du contract

C'est appelé aussi depuis la node non ?

wraitii · 2024-12-20T09:08:08Z

src/data_availability/node_state.rs

        self.unsettled_transactions.add(UnsettledBlobTransaction {
            identity: tx.identity.clone(),
            hash: blob_tx_hash.clone(),
            blobs_hash,
            blobs,
        });

+        for blob in tx.blobs.clone() {


On a besoin du clone là ?

à cause de l'argument "blobs" de:

pub fn parse_structured_blob<Parameters>(blobs: &[Blob], index: &BlobIndex) -> StructuredBlob<Parameters>

oui

wraitii · 2024-12-20T09:13:54Z

src/mempool.rs

+                        let TransactionData::RegisterContract(register_contract_transaction) = contract.transaction_data else {
+                            continue;
+                        };


Remarque: dans ce cas particulier je pense le if let fait bien le café mais no need to change

le if let ajoute un niveau de tabulation de plus, c'est pour ça que j'ai fait ça comme ça ^^

oui en général je suis pas contre mais je sais pas si on gagne vraiment en lisibilité là :P

wraitii · 2024-12-20T09:20:47Z

src/tools/transactions_builder.rs

+    pub hyllar: HyllarToken,
+    pub hydentity: Hydentity,
+    pub staking: Staking,
+}


RQ générale sur ce fichier: c'est très clean, mais c'est assez hardcodé, je pense que faire un système un poil plus générique serait cool dans le futur (mais clairement ça demande un peu de boulot de design pour faire quelque chose qui a du sens)

wraitii · 2024-12-20T09:23:48Z

src/tools/transactions_builder.rs

+    pub fn iter_prove<'a>(
+        &'a self,
+    ) -> impl Iterator<
+        Item = (
+            Pin<Box<dyn std::future::Future<Output = Result<ProofData>> + Send + 'a>>,
+            ContractName,
+        ),
+    > + 'a {
+        self.runners.iter().map(|runner| {
+            let future = runner.prove();
+            (
+                Box::pin(future)
+                    as Pin<Box<dyn std::future::Future<Output = Result<ProofData>> + Send + 'a>>,
+                runner.contract_name.clone(),
+            )
+        })
+    }
+}


Il te renvoie pas les preuves dans l'ordre de complétion si? Ou il y a de la magie que je ne suis pas. Tel que je lis le code là si on a 3 preuves et qu'elle finisse dans l'ordre on y gagne, mais sinon c'est équivalent à juste attendre les 3.

Tokio a des JoinSet qui exposent une API similaire: https://docs.rs/tokio/latest/tokio/task/struct.JoinSet.html

cette fonction retourne juste un itérateur, donc si à chaque itération de la boucle tu .await sur la future, en effet tu gagnes rien de plus qu'attendre les 3. Tu gagnes juste que tu peux send les proof au fure et à mesure, plutôt que toutes d'un coup.

Mais, l'utilisateur de cette fonction peut tout à fait itérer là dessus et await toutes les futures en parallèle

wraitii · 2024-12-20T09:25:09Z

tests/fixtures/ctx.rs

@@ -84,6 +84,8 @@ impl E2ECtx {
    }

    pub async fn new_single(slot_duration: u64) -> Result<E2ECtx> {
+        std::env::set_var("RISC0_DEV_MODE", "1");


Pas obvious pour moi pourquoi on a besoin de set ça ici, on le set dans proof_gen et dans TestProcess

le TransactionBuilder n'utilise pas les proof_gen, et peut être utilisé dans n'importe quel test

wraitii

Globalement je pense que c'est good to go, j'ai quelques remarques qui me paraissent utile à lire mais en soit rien qui nécessite un changement.

Le transaction_builder est une bonne base mais à étendre avec un boulot de design d'API pour faire un truc flexible et puissant.

✨ New Staking Contract

fe9e5e1

maxgttph reviewed Dec 16, 2024

View reviewed changes

src/consensus.rs Outdated Show resolved Hide resolved

maxgttph reviewed Dec 16, 2024

View reviewed changes

src/tools/transactions_builder.rs Show resolved Hide resolved

maxgttph reviewed Dec 16, 2024

View reviewed changes

BertrandD added 4 commits December 16, 2024 11:02

✅ Fix consensus it test

472889c

typo

20e4847

✅ Fix genesis test

e2f6eba

✅ Fix amm e2e test

ee6672f

wraitii reviewed Dec 16, 2024

View reviewed changes

src/tools/transactions_builder.rs Outdated Show resolved Hide resolved

BertrandD added 10 commits December 16, 2024 18:01

✅ Fix some e2e

30aa0e4

✅ rejoin flow (very flakky)

03efbcc

🚧

e2d21a1

Merge branch 'main' into new_staking

9431b2b

Merge branch 'main' into new_staking

d479d24

Merge branch 'main' into new_staking

e9311c9

Merge branch 'main' into new_staking

c10074a

✅ Fix rejoin

21f5b10

Merge branch 'main' into new_staking

28ff148

clean & rebuild contract

04c7647

BertrandD marked this pull request as ready for review December 19, 2024 15:48

BertrandD added 2 commits December 19, 2024 16:48

Merge branch 'main' into new_staking

81e1b36

clean

738467e

wraitii reviewed Dec 20, 2024

View reviewed changes

wraitii approved these changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ New Staking Contract #453

✨ New Staking Contract #453

BertrandD commented Dec 13, 2024

codecov bot commented Dec 13, 2024 •

edited

Loading

maxgttph Dec 16, 2024

BertrandD Dec 16, 2024

maxgttph Dec 16, 2024

BertrandD Dec 16, 2024

maxgttph Dec 16, 2024

BertrandD Dec 16, 2024

wraitii Dec 16, 2024

BertrandD Dec 16, 2024

BertrandD Dec 16, 2024

BertrandD Dec 16, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024

BertrandD Dec 20, 2024

wraitii Dec 20, 2024 •

edited

Loading

BertrandD Dec 20, 2024

wraitii left a comment

✨ New Staking Contract #453

Are you sure you want to change the base?

✨ New Staking Contract #453

Conversation

BertrandD commented Dec 13, 2024

codecov bot commented Dec 13, 2024 • edited Loading

❌ 1 Tests Failed:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wraitii Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wraitii left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2024 •

edited

Loading

wraitii Dec 20, 2024 •

edited

Loading